Skip to content

fix pgsql and other polyfill libs leaking into configure #1236

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.

Sign up for GitHub

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

Merged
henderkes merged 3 commits into from
Aug 22, 2026
Merged

fix pgsql and other polyfill libs leaking into configure #1236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4c01e0
fix pgsql and other polyfill libs leaking into configure
henderkes Aug 21, 2026
31b43bd
fix openssl dir detection
henderkes Aug 21, 2026
8eba75d
fix unusable go pkg roots if a new version downloads over an old one
henderkes Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/Package/Library/openssl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public function build(LibraryPackage $lib): void
'--with-zlib-lib=' . BUILD_LIB_PATH . ' ';

$openssl_dir = getenv('OPENSSLDIR') ?: null;
$openssl_dir ??= LinuxUtil::getOSRelease()['dist'] === 'redhat' ? '/etc/pki/tls' : '/etc/ssl';
$openssl_dir ??= self::detectOpenSSLDir();
$ex_lib = trim($ex_lib);

// anything we want included (PGO -fprofile-*, LTO, custom hardening)
Expand Down Expand Up @@ -117,6 +117,19 @@ public function build(LibraryPackage $lib): void
$this->patchPkgConfig($lib);
}

private static function detectOpenSSLDir(): string
{
// /etc/centos-release exists on AlmaLinux and Rocky, so getOSRelease() reports
// 'centos' there and the name check missed the RHEL layout
foreach (['/etc/ssl', '/etc/pki/tls'] as $dir) {
if (file_exists("{$dir}/cert.pem") || glob("{$dir}/certs/*.0")) {
return $dir;
}
}

return LinuxUtil::getOSRelease()['dist'] === 'redhat' ? '/etc/pki/tls' : '/etc/ssl';
}

private function patchPkgConfig(LibraryPackage $pkg): void
{
$pkg->patchPkgconfPrefix(['libssl.pc', 'openssl.pc', 'libcrypto.pc']);
Expand Down
5 changes: 1 addition & 4 deletions src/Package/Target/php/unix.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,12 @@ public function configureForUnix(TargetPackage $package, PackageInstaller $insta
$configure_str = str_replace('--with-pic', '--enable-pic', $configure_str);
}

// reuse the same make vars so configure conftest links use the same LIBS (incl. -framework flags)
$vars = $this->makeVars($installer);

// run ./configure with args
$this->seekPhpSrcLogFileOnException(fn () => shell()->cd($package->getSourceDir())->setEnv([
'CFLAGS' => getenv('SPC_CMD_VAR_PHP_MAKE_EXTRA_CFLAGS'),
'CPPFLAGS' => "-I{$package->getIncludeDir()}",
'LDFLAGS' => "-L{$package->getLibDir()} " . getenv('SPC_CMD_VAR_PHP_MAKE_EXTRA_LDFLAGS'),
'LIBS' => $vars['EXTRA_LIBS'] ?? '',
'LIBS' => SystemTarget::getRuntimeLibs(),

@crazywhalecc crazywhalecc Aug 22, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@henderkes

The LIBS configuration in PHP's ./configure was downgraded from makeVars()['EXTRA_LIBS'] (the complete library, including -lstdc++/-lc++, -lssl -lcrypto, and frameworks) to SystemTarget::getRuntimeLibs() (only the basic runtime library).

Then, the conftest linker test in configure failed due to missing C++ runtime and OpenSSL symbols, causing some jobs crash with the error configure: error.

This leads to a regression problem.

@henderkes henderkes Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

SystemTarget::getRuntimeLibs() should be returning -lstdc++ if we have any C++ extensions.

The problem with passing $vars['EXTRA_LIBS'] is passing all the polyfill libraries which will make php get wrong HAVE_XXX defines which will lead to make fail.

Are frameworks missing from getRuntimeLibs()? Then they need to be added. The implementation we have in main branch is correct.

@crazywhalecc crazywhalecc Aug 22, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Fixed in my PR

])->exec($configure_str), $package->getSourceDir());
}

Expand Down
15 changes: 11 additions & 4 deletions src/StaticPHP/Artifact/ArtifactExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ protected function extractWithType(string $cache_type, string $source_file, stri
*
* Supports: tar, tar.gz, tgz, tar.bz2, tar.xz, txz, zip, exe
*
* @param bool $merge when true, merge zip contents into existing target dir instead of wiping it
* @param bool $merge when true, merge contents into existing target dir instead of wiping it
*/
protected function extractArchive(string $filename, string $target, bool $merge = false): void
{
Expand All @@ -532,10 +532,17 @@ protected function extractArchive(string $filename, string $target, bool $merge

$extname = FileSystem::extname($filename);

if ($extname !== 'exe' && !is_dir($target)) {
FileSystem::createDir($target);
if ($extname !== 'exe') {
// tar extraction merges into whatever is already there, so a re-extract would
// leave files from the previous version behind
if (!$merge && is_dir($target)) {
FileSystem::removeDir($target);
}
if (!is_dir($target)) {
throw new FileSystemException("Failed to create target directory: {$target}");
FileSystem::createDir($target);
if (!is_dir($target)) {
throw new FileSystemException("Failed to create target directory: {$target}");
}
}
}
match (SystemTarget::getTargetOS()) {
Expand Down
Loading