Skip to content

fix: FrankenPHP build args#1057

Merged
henderkes merged 1 commit intocrazywhalecc:mainfrom
dunglas:patch-2
Mar 10, 2026
Merged

fix: FrankenPHP build args#1057
henderkes merged 1 commit intocrazywhalecc:mainfrom
dunglas:patch-2

Conversation

@dunglas
Copy link
Contributor

@dunglas dunglas commented Mar 10, 2026

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

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php or *.json, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:sort-config
  • If it's an extension or dependency update, please ensure the following:
    • Add your test combination to src/globals/test-extensions.php.
    • If adding new or fixing bugs, add commit message containing extension test or test extensions to trigger full test suite.

Copilot AI review requested due to automatic review settings March 10, 2026 10:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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=frankenphp into its own -X ldflag entry instead of being concatenated into the CustomVersion value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 457 to 463
'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}",
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@henderkes henderkes merged commit e31f648 into crazywhalecc:main Mar 10, 2026
11 of 12 checks passed
@henderkes
Copy link
Collaborator

Thanks, silly oversight. I thought we tested frankenphp build in CI too, but it was turned off.

crazywhalecc added a commit that referenced this pull request Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants