Remove runtime rewrite binary and modernize build-time placeholder replacement#1226
Merged
Remove runtime rewrite binary and modernize build-time placeholder replacement#1226
Conversation
This commit removes the runtime rewrite binary and replaces it with build-time
placeholder replacement, fixing multiple issues with multi-buildpack deployments
and git URL buildpack usage. It also fixes a critical bug where php.ini.d
configs were processed with the wrong HOME context.
## Rewrite Binary Removal
The rewrite binary was originally copied from the v4.x Python buildpack and used
to replace template variables in configuration files at runtime. This approach
had several issues:
- Failed when buildpack was deployed via git URL
- Compilation errors in multi-buildpack scenarios
- Security concerns with runtime config rewriting
- Performance overhead at container startup
This commit completes the migration to build-time placeholder replacement that
provides better security, performance, and multi-buildpack compatibility.
### Changes:
- Remove bin/rewrite shell wrapper script
- Remove src/php/rewrite/cli/main.go (entire rewrite implementation)
- Remove rewrite binary compilation from bin/finalize
- Remove bin/rewrite from manifest.yml include_files
- Remove /bin/rewrite-compiled from .gitignore
- Update ARCHITECTURE.md to remove rewrite binary documentation
## Build-Time Placeholder Replacement
Add ProcessConfigs() method to finalize phase that replaces @{VAR} placeholders
with actual values during staging:
- @{HOME} - App or dependency directory path
- @{DEPS_DIR} - Dependencies directory (/home/vcap/deps)
- @{WEBDIR} - Web document root (default: htdocs)
- @{LIBDIR} - Library directory (default: lib)
- @{PHP_FPM_LISTEN} - PHP-FPM socket/TCP address
- @{TMPDIR} - Converted to ${TMPDIR} for runtime expansion
- @{PHP_EXTENSIONS} - Extension directives
- @{ZEND_EXTENSIONS} - Zend extension directives
## Placeholder Syntax Unification
Unify all template variables to use @{VAR} syntax consistently across all
config files (httpd, nginx, php-fpm, php.ini).
## Multi-Buildpack Fixes
- Fix PHP-FPM PID file path to use deps directory for multi-buildpack scenarios
- Fix nginx configuration for Unix socket and runtime variable expansion
- Update supply buildpack integration tests
## php.ini.d Context Bug Fix
Fix critical bug where php.ini.d directory was processed with deps context
(@{HOME} = /home/vcap/deps/{idx}) instead of app context (@{HOME} =
/home/vcap/app).
The php.ini.d directory contains user-provided PHP configurations that typically
reference application paths (include_path, open_basedir, etc.), similar to fpm.d
configs. Processing with deps context caused the buildpack-created
include-path.ini and user configs to reference incorrect paths.
### Changes:
- Process php.ini.d separately (like fpm.d) with app-context replacements
- Update supply.go comments to clarify php.ini.d context behavior
- Add fixture test for @{HOME} placeholder in php.ini.d configs
- Enhance modules integration test to verify placeholder replacement
Both fpm.d and php.ini.d now use app HOME context while other PHP configs
(php.ini, php-fpm.conf) use deps HOME context.
Fixes issues with:
- Buildpack-created include-path.ini referencing wrong directory
- User-provided php.ini.d configs using @{HOME} placeholders
- Include paths not resolving to application lib directory
This commit improves the integration test infrastructure with better cleanup, more robust regex matching, and proper handling of unsupported features. Changes: - Fix integration test regex patterns for more reliable matching - Skip custom extensions test (feature not yet supported in v5.x) - Cleanup buildpack files after integration tests to prevent disk space issues Test Infrastructure Improvements: - Add buildpack cleanup in init_test.go to remove uploaded buildpacks after tests - Refactor default_test.go regex patterns for better readability - Add explicit skip for custom extensions with clear explanation These changes improve test reliability and reduce CI/CD resource usage by properly cleaning up test artifacts.
Go 1.26 refuses to overwrite non-object files with 'go build -o'.
Add 'rm -f "${output}"' before each go build invocation so the
shell script stubs in bin/ are removed first.
This was referenced Feb 25, 2026
The generate*StartScript functions read webDir from options but never interpolated it into the script body. Unit tests for custom and default WEBDIR were failing because the variable was never written out. Add WEBDIR="%s" line to all three generators (HTTPD, Nginx, PHP-FPM) and pass webDir as the first fmt.Sprintf argument.
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
1 participant
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.
Summary
Removes the runtime rewrite binary and replaces it with build-time placeholder replacement during the finalize phase. Fixes critical issues with multi-buildpack deployments and git URL buildpack usage.
Problem
The rewrite binary (copied from v4.x Python buildpack) caused multiple issues:
Solution
Replace runtime rewriting with build-time placeholder replacement in the finalize phase.
Build-Time Placeholders:
@{HOME},@{DEPS_DIR},@{WEBDIR},@{LIBDIR},@{PHP_FPM_LISTEN},@{TMPDIR},@{PHP_EXTENSIONS},@{ZEND_EXTENSIONS}Changes
1. Remove Runtime Rewrite Binary
bin/rewriteshell wrapper scriptsrc/php/rewrite/cli/main.go(entire rewrite implementation)bin/finalizebin/rewritefrommanifest.ymlinclude_files2. Build-Time Placeholder Replacement
ProcessConfigs()in finalize phase replacing@{VAR}placeholders at staging time@{VAR}syntax consistently (httpd, nginx, php-fpm, php.ini)3. Fix php.ini.d Context Bug
php.ini.dwas processed with deps context instead of app contextphp.ini.dandfpm.dnow correctly use appHOMEcontext4. Improve Test Infrastructure
5. Fix Go 1.26 Build Compatibility
go build -orm -fbefore eachgo buildinvocation inscripts/build.shTesting
All integration tests pass. Changes are backward compatible for standard usage.
Breaking Changes
None for standard usage. Custom
@{PLACEHOLDER}variables no longer supported — use environment variables instead.