Read php.ini from the dependency directory in the sessions extension - #1313
Open
ocean90 wants to merge 1 commit into
Open
Read php.ini from the dependency directory in the sessions extension#1313ocean90 wants to merge 1 commit into
ocean90 wants to merge 1 commit into
Conversation
The sessions extension resolves php.ini through PHPConfigHelper, which builds its paths from ctx.BuildDir. That field is populated in NewContext from a BUILD_DIR environment variable that the lifecycle never exports, so it is empty and the helper opens the relative path php/etc/php.ini. Any app with a bound Redis or Memcached session service therefore fails staging with: Extension compilation failed: extension sessions compile failed: failed to load PHP config: failed to load php.ini: failed to read config file php/etc/php.ini The supply phase records the real location under the DEPS_DIR key, and that is also where PHP is installed: php.ini lives in <deps>/<idx>/php/etc, never under the build directory. The unit test passed only because it assigned ctx.BuildDir directly, bypassing the code path production uses, and laid out php/etc under the build directory. It now models the dependency directory instead.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Staging fails for any app with a bound Redis or Memcached session service:
This is the failure reported in #1291.
Cause
NewPHPConfigHelperbuilds its paths fromctx.BuildDir:NewContextfills that field fromos.Getenv("BUILD_DIR"), which the lifecycle never exports, so it is empty and the helper opens the relative pathphp/etc/php.ini.createExtensionContextdoes callctx.Set("BUILD_DIR", s.Stager.BuildDir()), butSetwrites into theDatamap and does not reach the struct field. Every other extension readsctx.GetString("BUILD_DIR")and is unaffected, which is why only the sessions path breaks.The build directory is also the wrong base.
installPHPusesfilepath.Join(s.Stager.DepDir(), "php"), andphp.iniis written to<deps>/<idx>/php/etc/php.ini. Nothing writes to<build>/php.Fix
Resolve
php.iniandphp-fpm.conffromctx.GetString("DEPS_DIR"), which the supply phase sets tos.Stager.DepDir().Test
sessions_test.gopassed only because it assignedctx.BuildDirdirectly, with a comment noting thatSet()would not work, and createdphp/etcunder the build directory. That layout does not exist at staging time, so the test could not catch this. It now models the dependency directory. All unit tests pass;src/php/bratsfails identically before and after this change, on aDeferCleanupmisuse in the vendoredbratshelper.