-
-
Notifications
You must be signed in to change notification settings - Fork 326
fix building of shared extensions (grpc, simdjson, soap) #905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
meh, it looks like that's not enough. may have to remove -Wl,--as-needed instead (just for ImageMagick) |
… (stupid grpc build thirdparty hardcode)
closes #908 |
$original_ldflags = $this->builder->arch_ld_flags; | ||
if (str_contains($this->builder->arch_ld_flags, '-Wl,--as-needed')) { | ||
$this->builder->arch_ld_flags = str_replace('-Wl,--as-needed', '', $original_ldflags); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to change only for this scope rather than modifying builder property? I don't think modifying builder's value is the best idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a way, unfortunately, other than patching imagemagick's ./configure, but that would be much worse than temporarily changing the ldflags.
I'll work on it next week. On a business trip until Sunday. |
What does this PR do?
fixes grpc shared compilation - it just ignores CXXFLAGS and instead abuses CPPFLAGS for what should in CXXFLAGS...
closes #908
Checklist before merging
*.php
or*.json
, run them locally to ensure your changes are valid:composer cs-fix
composer analyse
composer test
bin/spc dev:sort-config
src/globals/test-extensions.php
.extension test
ortest extensions
to trigger full test suite.