Fix custom PHP binary deleted when binary path is the app root

[simonhamp]

Summary

Fixes #115.

When NATIVEPHP_PHP_BINARY_PATH is set to a value that points at the project root (e.g. ./), native:build deletes the entire built app right before electron-builder packages it.

Root cause

In PrunesVendorDirectory::pruneVendorDirectory(), the configured binary package directory was interpolated naively into "app/{$binaryPackageDirectory}". When the value normalizes to a no-op (./, ., /, or a trailing /.), Path::join('/build', 'app/./') collapses to /build/app, so the prune step removes the whole app directory. The previous ! empty() guard did not catch this because strings like ./ are non-empty.

native:run was unaffected because it does not run the prune step.

Fix

Trim the configured path of surrounding dots, slashes, and whitespace, then skip removal entirely when nothing meaningful remains:

$binaryPackageDirectory = trim($this->binaryPackageDirectory(), "./\\ \t\n\r\0\x0B");
if ($binaryPackageDirectory !== '' && $filesystem->exists($this->buildPath("app/{$binaryPackageDirectory}"))) {
    $filesystem->remove($this->buildPath("app/{$binaryPackageDirectory}"));
}

Tests

Added tests/Build/PruneVendorDirectoryTest.php:

• A normal custom binary directory (php-bin/) is still pruned.
• A path that normalizes to the app root (./) no longer wipes the built app (regression test for Custom PHP binary gets deleted if in root of project #115).

Verified the regression test fails against the old code and passes with the fix. Full suite (114 tests) passes and Pint is clean.

🤖 Generated with Claude Code

[simonhamp]

@bmckay959 would you mind trying this fix out when you get a chance?

[bmckay959]

I'm a bit confused by this test....why do we want the custom binary removed? Feels like this test is introducing the bug the opposite direction? Basically, we don't want to delete the directory where the NATIVEPHP_PHP_BINARY points....or am I missing something?

[gwleuverink]

Understandable confusion. The binary is moved to a location in the app bundle itself during build. So leaving it in the source directory would include it twice 👍🏻

[PeteBishwhip]

LGTM. Happy to merge pending passing CI.

[PeteBishwhip]

Looks like we're not currently able to support the ->after() method here.

[bmckay959]

Based on @gwleuverink comment from above, do we actually want the /app/bin/win/x64/php.exe to be removed? So basically add the expect("$buildPath/app/bin")->not->toBeDirectory()?

I can confirm this solved the bug, but I believe this is now including ALL the PHP versions with the app this way(So my build goes from 127MB to 212MB). So, if the binary is getting copied to a 'special' place inside the electron app(?), then can we not have the /bin directory also removed?

[gwleuverink]

Good point. I may have misinterpreted the test. I'll have a another look at it to confirm

[gwleuverink]

Dug into this a bit more. The PR fixes the crash, but Ben's spotted a second thing worth handling before we merge: the binaries are still ending up in the bundle, which is the 127 to 212MB jump he measured.

What's happening is php.js already extracts the one binary we need into build/php during the build, so the php-*.zip files left in the source tree are redundant copies. Right now the prune skips them when the path is ./, because we're removing the whole binary directory and there's no way to do that at the app root without taking the app with it. So the test ends up asserting they survive, when really we'd want them gone.

The wrinkle on my side is that we don't own bin/ outright, only {os}/{arch}/php-*.zip inside it, so I'd rather not remove the whole directory in case someone keeps other things there. Targeting just our archives covers both the size and that:

$binDir = $this->buildPath('app/'.$this->binaryPackageDirectory().'bin');
$filesystem->remove(glob("{$binDir}/*/*/php-*.zip"));

For ./ that resolves to app/bin/*/*/php-*.zip, which never collapses to the app root, so the trim/guard isn't needed anymore either. Tested it against a bin/ with our zips plus some unrelated files and only the zips go.

@simonhamp happy to push this onto the branch and tweak the test to match if you're good with it. Or we iterate the os/arch matrix instead of the glob if you'd prefer, though php-*.zip feels specific enough.

[bmckay959]

Thanks for digging in more...as I mentioned in my initial issue, I'm still pretty new on the native php side of things, so I haven't fully wrapped my arms around the build process/order/when things get injected.

This feels like the right approach to me.

[gwleuverink]

Wouldn't have spotted it without your keen eye! @bmckay959 🙏🏻

Here's my suggested fix for review #121

This targets only the archives we manage, at their fixed bin/{os}/{arch}/php-*.zip location, never the containing directory. so if you happen to keep other files in ./bin those won't be wiped.

[gwleuverink]

The failing tests seem unrelated to this PR. Looks like the PendingRequest contract has broken backwards compat in a minor release. (Only happens with L13 - prefer-stable)

[SRWieZ]

Does it still work with absolute directory path ?