phar: deterministic file order

[drupol]

Hello,

I recently discovered the project https://stagex.tools/ and out of curiosity I checked how the PHP package (https://codeberg.org/stagex/stagex/src/branch/staging/packages/core/php-zts) was built. This is how I discovered those two potential interesting patches (credits to @zeroware) that could easily be merged in the PHP distribution.

I proposed the adoption of those patches to the Nix community as well at NixOS/nixpkgs#463814

I hope the branch PHP-8.3 is the correct one, let me know if there's anything to change.

This PR:

• Make sure files are listed in deterministic order when building the PHAR

[petk]

I'm not that familiar with all the phar options. This -l 5 works everywhere? For example, in case of /home/user/php-src/ or in case of /home/user/projects/opensource/subfolder/php/php-src also?

[thg2k]

That will not work, I will show you a patch that does it, you also have to consider out-of-tree builds

[thg2k]

This one is for PHP-8.2 branch but should be straightforward to port it for PHP-8.3:

--- ext/phar/Makefile.frag
+++ ext/phar/Makefile.frag
@@ -44,11 +44,13 @@
 $(builddir)/phar.phar: $(builddir)/phar.php $(builddir)/phar/phar.inc $(srcdir)/phar/*.inc $(srcdir)/phar/*.php $(SAPI_CLI_PATH)
        -@(echo "Generating phar.phar"; \
        if [ "$(TEST_PHP_EXECUTABLE_RES)" != 1 ]; then \
                rm -f $(builddir)/phar.phar; \
                rm -f $(srcdir)/phar.phar; \
-               $(PHP_PHARCMD_EXECUTABLE) $(PHP_PHARCMD_SETTINGS) $(builddir)/phar.php pack -f $(builddir)/phar.phar -a pharcommand -c auto -x \\.svn -p 0 -s $(srcdir)/phar/phar.php -h sha1 -b "$(PHP_PHARCMD_BANG)"  $(srcdir)/phar/; \
+               rm -rf $(builddir)/phar.tmp; \
+               cp -a $(srcdir)/phar $(builddir)/phar.tmp; \
+               (cd $(builddir); $(PHP_PHARCMD_EXECUTABLE) $(PHP_PHARCMD_SETTINGS) phar.php pack -f phar.phar -a pharcommand -c auto -x \\.svn -p 0 -l 1 -s $(srcdir)/phar/phar.php -h sha1 -b "$(PHP_PHARCMD_BANG)" phar.tmp/*.inc); \
                chmod +x $(builddir)/phar.phar; \
        else \
                echo "Skipping phar.phar generating during cross compilation"; \
        fi)

[petk]

This one is for PHP-8.2 branch but should be straightforward to port it for PHP-8.3

I should check this more thoroughly but I trust your patch. Thanks for the info. Here perhaps also distclean Make target should be adjusted a bit further (in that build/Makefile.global file) by removing the phar.tmp directory. And as Autotools build system "still" supports in-source builds also .gitignore should be updated with ext/phar/phar.tmp. I'm just adding this as a note for the future...

[thg2k]

Patch i posted is tested on 8.2, hence the disclaimer, but it should work as is in 8.3.

The phar tmp is needed for out of tree builds, and to ensure exactly one level, to match the "-l 1" parameter to the phar command.

A cleaner solution would be to modify phar.phar itself to sort files, but I didn't get to it, just wanted a quick solution to make it stable across builds.

But yes, distclean should be adjusted as well, or maybe it can just be removed right away..

[petk]

A cleaner solution would be to modify phar.phar itself to sort files, but I didn't get to it, just wanted a quick solution to make it stable across builds.

Can't blame you. The PHP code generating the phar.phar file looks terrible. A book-alike definition of messy unreadable code. Methods can't be searched as they are dynamically created based on the command argument, etc. I'm also not going to dive too deep into that phar.php file and its clicommand.inc file :D

So, most likely shell bypass is the way to go here.

Edit: Otherwise, if someone is willing to rewrite the phar_add method in the pharcommand.inc file, it's probably there where sorting should be added for the RecursiveDirectoryIterator.

[petk]

And another place where such ordering might need to be done is also in the win32/build/mkdist.php file at the make_phar_dot_phar() function.

[github-actions]

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

[petk]

I've removed the "Waiting for author" GitHub label as it seems that this triggers bot to close the pull request after two weeks of inactivity.