fix: FrankenPHP build args#1057
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes FrankenPHP’s xcaddy/Caddy build ldflags by correcting how CustomBinaryName is passed, addressing a regression introduced in #1056 that breaks FrankenPHP CI.
Changes:
- Separates
CustomBinaryName=frankenphpinto its own-Xldflag entry instead of being concatenated into theCustomVersionvalue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'XCADDY_GO_BUILD_FLAGS' => '-buildmode=pie ' . | ||
| '-ldflags \"-linkmode=external ' . $extLdFlags . ' ' . | ||
| '-X \'github.com/caddyserver/caddy/v2/modules/caddyhttp.ServerHeader=FrankenPHP Caddy\' ' . | ||
| '-X \'github.com/caddyserver/caddy/v2.CustomBinaryName=frankenphp\' ' . | ||
| '-X \'github.com/caddyserver/caddy/v2.CustomVersion=FrankenPHP ' . | ||
| '-X \'github.com/caddyserver/caddy/v2.CustomBinaryName=frankenphp ' . | ||
| "v{$frankenPhpVersion} PHP {$libphpVersion} Caddy'\\\" " . | ||
| "-tags={$muslTags}nobadger,nomysql,nopgx{$nobrotli}{$nowatcher}", |
There was a problem hiding this comment.
Optional: there doesn’t appear to be an automated test asserting the exact formatting of the XCADDY_GO_BUILD_FLAGS string for FrankenPHP (e.g., that CustomBinaryName and CustomVersion are separate -X entries and properly quoted). This area regressed once (#1056); extracting the ldflags construction into a helper and adding a unit test for the resulting env string would help prevent future quoting/concatenation mistakes.
|
Thanks, silly oversight. I thought we tested frankenphp build in CI too, but it was turned off. |
What does this PR do?
Fix a bug in Caddy build arguments introduced in #1056.
This breaks FrankenPHP's CI: https://github.com/php/frankenphp/actions/runs/22896611170/job/66432268615
cc @henderkes
Checklist before merging
*.phpor*.json, run them locally to ensure your changes are valid:composer cs-fixcomposer analysecomposer testbin/spc dev:sort-configsrc/globals/test-extensions.php.extension testortest extensionsto trigger full test suite.