Skip to content

fix CustomPackage handling #897

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 8 commits into from
Sep 18, 2025
Merged

fix CustomPackage handling #897

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5c1194e
fix custompackage handling
henderkes Sep 9, 2025
893ce31
only install pkgs for crafting when necessary
henderkes Sep 10, 2025
0b0ae27
we lost a v somehow
henderkes Sep 10, 2025
8680e83
fix dba building shared
mhpcc Sep 16, 2025
4247883
Merge branch 'main' into fix-zig
henderkes Sep 18, 2025
808d224
build frankenphp before shared extensions again
henderkes Sep 18, 2025
311ee40
update LinuxToolCheckList.php to install perl-Time-Piece to make open…
henderkes Sep 18, 2025
191d345
update paths to only install once
henderkes Sep 18, 2025
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
4 changes: 2 additions & 2 deletions src/SPC/builder/BuilderBase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use SPC\store\Config;
use SPC\store\FileSystem;
use SPC\store\LockFile;
use SPC\store\pkg\GoXcaddy;
use SPC\store\SourceManager;
use SPC\store\SourcePatcher;
use SPC\util\AttributeMapper;
Expand Down Expand Up @@ -507,8 +508,7 @@ public function checkBeforeBuildPHP(int $rule): void
throw new WrongUsageException('FrankenPHP SAPI is only available on Linux and macOS!');
}
// frankenphp needs package go-xcaddy installed
$pkg_dir = PKG_ROOT_PATH . '/go-xcaddy-' . arch2gnu(php_uname('m')) . '-' . osfamily2shortname();
if (!file_exists("{$pkg_dir}/bin/go") || !file_exists("{$pkg_dir}/bin/xcaddy")) {
if (!GoXcaddy::isInstalled()) {
global $argv;
throw new WrongUsageException("FrankenPHP SAPI requires the go-xcaddy package, please install it first: {$argv[0]} install-pkg go-xcaddy");
}
Expand Down
11 changes: 3 additions & 8 deletions src/SPC/builder/unix/UnixBuilderBase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ protected function patchPhpScripts(): void

protected function buildFrankenphp(): void
{
GlobalEnvManager::addPathIfNotExists(GoXcaddy::getEnvironment()['PATH']);
GlobalEnvManager::addPathIfNotExists(GoXcaddy::getPath());
$nobrotli = $this->getLib('brotli') === null ? ',nobrotli' : '';
$nowatcher = $this->getLib('watcher') === null ? ',nowatcher' : '';
$xcaddyModules = getenv('SPC_CMD_VAR_FRANKENPHP_XCADDY_MODULES');
Expand Down Expand Up @@ -310,7 +310,7 @@ protected function buildFrankenphp(): void
$cflags .= ' -Wno-error=missing-profile';
$libs .= ' -lgcov';
}
$env = [
$env = [...[
'CGO_ENABLED' => '1',
'CGO_CFLAGS' => clean_spaces($cflags),
'CGO_LDFLAGS' => "{$this->arch_ld_flags} {$staticFlags} {$config['ldflags']} {$libs}",
Expand All @@ -320,12 +320,7 @@ protected function buildFrankenphp(): void
"{$frankenPhpVersion} PHP {$libphpVersion} Caddy'\\\" " .
"-tags={$muslTags}nobadger,nomysql,nopgx{$nobrotli}{$nowatcher}",
'LD_LIBRARY_PATH' => BUILD_LIB_PATH,
];
foreach (GoXcaddy::getEnvironment() as $key => $value) {
if ($key !== 'PATH') {
$env[$key] = $value;
}
}
], ...GoXcaddy::getEnvironment()];
shell()->cd(BUILD_BIN_PATH)
->setEnv($env)
->exec("xcaddy build --output frankenphp {$xcaddyModules}");
Expand Down
13 changes: 9 additions & 4 deletions src/SPC/store/FileSystem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public static function convertPath(string $path): string
public static function convertWinPathToMinGW(string $path): string
{
if (preg_match('/^[A-Za-z]:/', $path)) {
$path = '/' . strtolower(substr($path, 0, 1)) . '/' . str_replace('\\', '/', substr($path, 2));
$path = '/' . strtolower($path[0]) . '/' . str_replace('\\', '/', substr($path, 2));
}
return $path;
}
Expand Down Expand Up @@ -314,8 +314,13 @@ public static function scanDirFiles(string $dir, bool $recursive = true, bool|st
$sub_file = self::convertPath($dir . '/' . $v);
if (is_dir($sub_file) && $recursive) {
# 如果是 目录 且 递推 , 则递推添加下级文件
$list = array_merge($list, self::scanDirFiles($sub_file, $recursive, $relative));
} elseif (is_file($sub_file) || is_dir($sub_file) && !$recursive && $include_dir) {
$sub_list = self::scanDirFiles($sub_file, $recursive, $relative);
if (is_array($sub_list)) {
foreach ($sub_list as $item) {
$list[] = $item;
}
}
} elseif (is_file($sub_file) || (is_dir($sub_file) && !$recursive && $include_dir)) {
# 如果是 文件 或 (是 目录 且 不递推 且 包含目录)
if (is_string($relative) && mb_strpos($sub_file, $relative) === 0) {
$list[] = ltrim(mb_substr($sub_file, mb_strlen($relative)), '/\\');
Expand Down Expand Up @@ -440,7 +445,7 @@ public static function createDir(string $path): void
public static function writeFile(string $path, mixed $content, ...$args): bool|int|string
{
$dir = pathinfo(self::convertPath($path), PATHINFO_DIRNAME);
if (!is_dir($dir) && !mkdir($dir, 0755, true)) {
if (!is_dir($dir) && !mkdir($dir, 0755, true) && !is_dir($dir)) {
throw new FileSystemException('Write file failed, cannot create parent directory: ' . $dir);
}
return file_put_contents($path, $content, ...$args);
Expand Down
6 changes: 5 additions & 1 deletion src/SPC/store/pkg/CustomPackage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@ abstract public function fetch(string $name, bool $force = false, ?array $config

/**
* Get the environment variables this package needs to be usable.
* PATH needs to be appended, rather than replaced.
*/
abstract public static function getEnvironment(): array;

/**
* Get the PATH required to use this package.
*/
abstract public static function getPath(): ?string;

@crazywhalecc crazywhalecc Sep 9, 2025 •
edited
Loading

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.

I think here can return a default value PKG_ROOT_PATH . '/bin', rather than making an abstract function.

@henderkes henderkes Sep 9, 2025

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.

PKG_ROOT_PATH . '/bin' doesn't exist. PKG_ROOT_PATH . '/$name/bin' is not guaranteed to exist. Both go and zig use non-standard locations for the binaries.

@crazywhalecc crazywhalecc Sep 9, 2025

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.

If we only look at these two special packages, we don't seem to need to add this abstract method - these two getPath calls are both certain classes.

@henderkes henderkes Sep 9, 2025

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.

Yes, but I would expect more packages to need to add a path to the PATH. We already have the abstract getEnvironment method because of the same assumption, so I believe it should be made here too. In case we don't need one, the method can return null, but it's a hint to us to think about whether the package needs a path, so I make it abstract.

@henderkes henderkes Sep 17, 2025

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.

@crazywhalecc if this is a blocker for you, I'm okay to change it to a function that returns null by default. We should look into merging this PR, though, because at the moment spc craft doesn't work until this is merged.

@crazywhalecc crazywhalecc Sep 17, 2025

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.

Ah I just on business trip and have almost no time to deal with other things. Will check it soon.

@crazywhalecc crazywhalecc Sep 18, 2025

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.

I'm open to changes in this PR, but I might also remove or modify it in 3.0. I don't think it's a big deal since we haven't finalized vendor mode yet.


abstract public static function isInstalled(): bool;

/**
Expand Down
17 changes: 6 additions & 11 deletions src/SPC/store/pkg/GoXcaddy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,17 @@ public function extract(string $name): void

public static function getEnvironment(): array
{
$arch = arch2gnu(php_uname('m'));
$os = match (PHP_OS_FAMILY) {
'Windows' => 'win',
'Darwin' => 'macos',
'BSD' => 'freebsd',
default => 'linux',
};

$packageName = "go-xcaddy-{$arch}-{$os}";
$packageName = 'go-xcaddy';
$pkgroot = PKG_ROOT_PATH;

return [
'PATH' => "{$pkgroot}/{$packageName}/bin",
'GOROOT' => "{$pkgroot}/{$packageName}",
'GOBIN' => "{$pkgroot}/{$packageName}/bin",
'GOPATH' => "{$pkgroot}/go",
];
}

public static function getPath(): ?string
{
return PKG_ROOT_PATH . '/go-xcaddy/bin';
}
}
17 changes: 2 additions & 15 deletions src/SPC/store/pkg/Zig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,23 +129,10 @@ public function extract(string $name): void

public static function getEnvironment(): array
{
$arch = arch2gnu(php_uname('m'));
$os = match (PHP_OS_FAMILY) {
'Windows' => 'win',
'Darwin' => 'macos',
'BSD' => 'freebsd',
default => 'linux',
};

$packageName = "zig-{$arch}-{$os}";
$path = PKG_ROOT_PATH . "/{$packageName}";

return [
'PATH' => $path,
];
return [];
}

private static function getPath(): string
public static function getPath(): ?string
{
return PKG_ROOT_PATH . '/zig';
}
Expand Down
4 changes: 2 additions & 2 deletions src/SPC/toolchain/ZigToolchain.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ public function initEnv(): void

public function afterInit(): void
{
if (!is_dir(Zig::getEnvironment()['PATH'])) {
if (!Zig::isInstalled()) {
throw new EnvironmentException('You are building with zig, but zig is not installed, please install zig first. (You can use `doctor` command to install it)');
}
GlobalEnvManager::addPathIfNotExists(Zig::getEnvironment()['PATH']);
GlobalEnvManager::addPathIfNotExists(Zig::getPath());
f_passthru('ulimit -n 2048'); // zig opens extra file descriptors, so when a lot of extensions are built statically, 1024 is not enough
$cflags = getenv('SPC_DEFAULT_C_FLAGS') ?: '';
$cxxflags = getenv('SPC_DEFAULT_CXX_FLAGS') ?: '';
Expand Down