Skip to content

patchelf and rename -release tagged extensions from php configure build#767

Merged
crazywhalecc merged 77 commits intomainfrom
docs
Jun 19, 2025
Merged

patchelf and rename -release tagged extensions from php configure build#767
crazywhalecc merged 77 commits intomainfrom
docs

Conversation

@henderkes
Copy link
Collaborator

@henderkes henderkes commented Jun 12, 2025

What does this PR do?

the release option also affects shared extensions, which is unwanted, patchelf their soname back and rename them

Closes #771

@crazywhalecc
Copy link
Owner

crazywhalecc commented Jun 13, 2025

I got build error when building shared extension:

bin/spc build "" --build-shared=swoole --debug --build-cli --no-strip
configure:4185: clang -I/Users/jerry/project/git-project/static-php-cli/buildroot/include -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/main -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/TSRM -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/Zend -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/ext  -L/Users/jerry/project/git-project/static-php-cli/buildroot/lib conftest.c -Wl,-Bstatic -Wl,--start-group -lcares -lbrotlidec -lbrotlienc -lbrotlicommon -lz -lssl -lcrypto -lnghttp2 -lphp -lcurl -Wl,--end-group -Wl,-Bdynamic -lc -lpthread -lnghttp2 -framework CoreFoundation -framework CoreServices -framework SystemConfiguration -lresolv >&5
ld: unknown options: -Bstatic --start-group --end-group -Bdynamic 
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@crazywhalecc crazywhalecc requested a review from Copilot June 13, 2025 05:06
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

This PR addresses issues caused by the release option affecting shared PHP extensions. It updates tool checklists, modifies shared extension build operations to rename and update soname via patchelf, and adjusts file modifications conditional on a release flag.

  • Added patchelf to various Linux tool lists.
  • Modified the build process for shared extensions in LinuxBuilder.
  • Adjusted file replacements in LibraryBase and updated logging in Extension.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/SPC/doctor/item/LinuxToolCheckList.php Added 'patchelf' to tool lists for various Linux distributions.
src/SPC/builder/linux/LinuxBuilder.php Renames shared extension files and uses patchelf to update soname.
src/SPC/builder/LibraryBase.php Conditionally replaces lines in a source file based on a release flag.
src/SPC/builder/Extension.php Adds shared extension build checks and logs warnings for build issues.
src/SPC/builder/BuilderBase.php Removes redundant shared extension checks.
Comments suppressed due to low confidence (1)

src/SPC/builder/Extension.php:338

  • [nitpick] Adjust the backtick formatting in the logged message so that the command is properly encapsulated (e.g. use a single pair: spc build).
logger()->warning('Try deleting your build and source folders and running `spc build`` again.');

@henderkes
Copy link
Collaborator Author

henderkes commented Jun 13, 2025

I got build error when building shared extension:

bin/spc build "" --build-shared=swoole --debug --build-cli --no-strip
configure:4185: clang -I/Users/jerry/project/git-project/static-php-cli/buildroot/include -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/main -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/TSRM -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/Zend -I/Users/jerry/project/git-project/static-php-cli/buildroot/include/php/ext  -L/Users/jerry/project/git-project/static-php-cli/buildroot/lib conftest.c -Wl,-Bstatic -Wl,--start-group -lcares -lbrotlidec -lbrotlienc -lbrotlicommon -lz -lssl -lcrypto -lnghttp2 -lphp -lcurl -Wl,--end-group -Wl,-Bdynamic -lc -lpthread -lnghttp2 -framework CoreFoundation -framework CoreServices -framework SystemConfiguration -lresolv >&5
ld: unknown options: -Bstatic --start-group --end-group -Bdynamic 
clang: error: linker command failed with exit code 1 (use -v to see invocation)

ld64 doesn't understand a lot of the linker flags that lld/gnu-ld support. MacOS shared extension support is very much barebones and we're expecting .so files (which curiously, also get generated on macos, when I expected .dylibs). I was able to build shared extensions on MacOS by toying with linker flags a bit, but I only have a shitty hacked together docker environment and hate the MacOS keyboard scheme, so it's not a priority for me.

If you want to properly support shared extensions on mac, there's a lot of work to be done. Start group, end group for one is crucial, because by default, the linker doesn't go over previously visited archives again to find new unresolved symbols. We'd need to pass the libraries in perfect order, so that every library is only specified once, at the very last time that it needs to be referenced.

@henderkes
Copy link
Collaborator Author

closes #765

@crazywhalecc
Copy link
Owner

Removing LIBS=-Wl,-Bstatic ... seems to work fine on macOS so far.

$ ls -lah buildroot/modules/swoole.so
-rwxr-xr-x  1 jerry  staff    32M Jun 13 14:08 buildroot/modules/swoole.so

$ php -d "extension=buildroot/modules/swoole.so" --ri swoole

swoole

Swoole => enabled

$ otool -L buildroot/modules/swoole.so
buildroot/modules/swoole.so:
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1900.180.0)

@henderkes
Copy link
Collaborator Author

You're welcome to remove it, it was mostly because it made testing easier in combination with --whole-archive, to force full resolution of symbols in those libraries. I think I edged it out with the new logic to find dependent libraries and the --start-group and --end-group-

Copy link
Owner

@crazywhalecc crazywhalecc left a comment

Choose a reason for hiding this comment

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

It's a bit of a beast though. Next time it would be better to separate the dependencies to another PR - this would help reviewing and automatically generating changelogs, if it’s not too related.

In addition, it may be convenient for us to add a simple commit message detect workflow to trigger a single test function.

@henderkes
Copy link
Collaborator Author

there were a lot of issues introduced by the lock file changes, I think I'm getting behind them though

@henderkes
Copy link
Collaborator Author

found the segmentation fault bug in imagick, I think it forced symbols from libphp.a into imagick.so, which then conflicted when a program that already had the symbols from libphp.a tried to load the extension

@henderkes
Copy link
Collaborator Author

heh, we have a small problem with xcaddy - go install xcaddy links it against the systems libraries. when we install xcaddy on the host and then try to run bin/spc-alpine-docker, it fails with the misleading error that the xcaddy file doesn't exist

@crazywhalecc
Copy link
Owner

crazywhalecc commented Jun 19, 2025

heh, we have a small problem with xcaddy - go install xcaddy links it against the systems libraries. when we install xcaddy on the host and then try to run bin/spc-alpine-docker, it fails with the misleading error that the xcaddy file doesn't exist

Sounds like the pkgroot and buildroot reuse in different env error. I don't currently have any good way to get spc to recognize the correct pkgroot and buildroot, since we expect everyone to compile in only one environment at one time.

@henderkes
Copy link
Collaborator Author

yeah I think it's fair to ignore this for now.

@henderkes
Copy link
Collaborator Author

okay: mac failed 3x to rate limiting now. doesn't Downloader::curlExec automatically use the GITHUB_TOKEN?

@crazywhalecc
Copy link
Owner

Sorry, but I prefer that whoever made the changes should fix them. 🫣 Just now my local has been replaced globally, and the your commit pushed is missing the part in the docs and UnixBuilderBase.

@henderkes
Copy link
Collaborator Author

Oh, sorry! On the bright side, frankenphp built with musl now links fully statically again, meaning it can also run on glibc.

@henderkes
Copy link
Collaborator Author

Sorry, but I prefer that whoever made the changes should fix them. 🫣 Just now my local has been replaced globally, and the your commit pushed is missing the part in the docs and UnixBuilderBase.

That's by design, though. We keep php/frankenphp where it's possible, it's only for the xcaddy command where it must be called dunglas/frankenphp.

@henderkes
Copy link
Collaborator Author

I'll let a few more static extension tests run before I merge this

@henderkes
Copy link
Collaborator Author

everything builds correctly except the opcache jit error. I thought php-src already fixed that O.o

@crazywhalecc
Copy link
Owner

I don't think PHP will be fixed immediately in versions other than the main branch. Also, if necessary, I would use gh cache -R crazywhalecc/static-php-cli delete --all to clear the cache to confirm that the latest source is valid.

@henderkes
Copy link
Collaborator Author

henderkes commented Jun 19, 2025

oh you're right, it was tagged, but for 8.4.9 php/php-src@7a0beb4

Feel free to merge this then.

@crazywhalecc crazywhalecc merged commit a2f0640 into main Jun 19, 2025
146 of 147 checks passed
@crazywhalecc crazywhalecc deleted the docs branch June 19, 2025 07:57
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.

FrankenPHP is now an official php SAPI

3 participants