Simplify the PHAR build script #656

theofidry · 2018-04-08T16:43:51Z

I originally wanted to try if I was hitting a too many files open limit when compressing the PHAR. I cant' seem to reproduce it on my machine so I was looking for a real project where this was happening.

I did not hit the issue and simplified the build script:

Leverage PhpScoper from inside Box instead: it's faster
Simplify the Box config
Instead of relying on global dependencies (for example you won't be able to install php-scoper & box on mine), I prefer to leverage bamarni composer bin plugin. You can find a bit more of it's usage in its doc or the article I wrote.

The last step, running the psalm.phar PHAR does not work: I think I'm missing something obvious but can't figure out what.

Also not if you wish to see the files added to the PHAR, e.g. to debug the PHP-Scoper effect, you can:

use an IDE like PhpStorm which is able to index PHAR files and inspect them
use the --debug mode of compile which is going to dump the files added to the PHAR a directory .box

coveralls · 2018-04-08T17:05:37Z

Coverage remained the same at 84.715% when pulling 2df9d2d on theofidry:master into 3119a1f on vimeo:master.

muglug · 2018-04-09T19:34:57Z

I can't get the Phar to build at all on my work machine (4-year-old Macbook Pro). I get OOM errors.

muglug · 2018-04-09T19:41:09Z

I'll try to get it building on my home machine too, maybe a bad config somewhere.

theofidry · 2018-04-09T20:56:23Z

OOM errors? If possible it would be nice to get a details on them. I’m trying to make so that box handle the config for you, for example setting phar.readonly=0. So let’s not rush that PR I would like as much as possible fix the real underlying issues :) as you already have a working PHAR and build process, we can take our time for this.

…

On Mon 9 Apr 2018 at 20:41, Matthew Brown ***@***.***> wrote: I'll try to get it building on my home machine too, maybe a bad config somewhere. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#656 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE76gSvS6N4F4s0r8wQnXb5alCm597acks5tm7lWgaJpZM4TLm5H> .

theofidry · 2018-05-01T19:22:59Z

bin/build-phar.sh

-[ -d build ] || mkdir build
-[ -d build/psalm ] || mkdir build/psalm
-# increase FD limit, or Phar compression will fail
-ulimit -Sn 4096


did you encounter that failure or it was here for good measure?

I've seen that, when building psalm.phar including dev dependencies.

And I can reproduce it:

$ cd psalm $ git checkout c13fda4034b33eb859e8810b10b0322fca17b858 $ patch -p1 <<EOM diff --git a/bin/build-phar.sh b/bin/build-phar.sh index 1e05c796..7843dd05 100755 --- a/bin/build-phar.sh +++ b/bin/build-phar.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash composer global require humbug/box:dev-master -composer install --no-dev +composer install # --no-dev [ -d build ] || mkdir build -box build +php -dphar.readonly=0 `which box` build EOM patching file bin/build-phar.sh Hunk #1 succeeded at 1 with fuzz 2. $ bin/build-phar.sh [......snip......] Box (repo) // Loading the configuration file "/home/weirdan/src/psalm/box.json". Building the PHAR "/home/weirdan/src/psalm/build/psalm.phar" ? Registering compactors + KevinGH\Box\Compactor\Php ? Adding main file: /home/weirdan/src/psalm/psalm ? Adding binary files > No file found ? Adding files > 4496 file(s) ? Generating new stub - Using shebang line: #!/usr/bin/env php - Using banner: > Generated by Humbug Box. > > @link https://github.com/humbug/box ? Compressing with the algorithm "BZ2" In Compile.php line 415: unable to create temporary file build [-c|--config CONFIG] [--debug] [-d|--working-dir WORKING-DIR] [......snip......]

The box version is 3.0.0-alpha.0 ( f5eec71df2df3a2a1f39a50ba77e8bb4312fef51 )
ulimit -n is set to 1024
Phar version is 2.0.2
PHP version is 7.2.4-1

Fixed in Box now

theofidry

Bumped box and added some comments, ready for a first review :)

theofidry · 2018-05-05T00:46:31Z

bin/build-phar.sh

@@ -1,37 +1,6 @@
 #!/usr/bin/env bash
-if ! php -r 'extension_loaded("bz2") or exit(1);' ; then


this is now handled in Box

theofidry · 2018-05-05T00:47:23Z

bin/build-phar.sh


-composer global require 'humbug/php-scoper:^1.0@dev'


this has been replaced by bamarni/composer-bin-plugin, up to you if you're cool with that or you prefer the composer global or maybe switch to a wget the Box PHAR

theofidry · 2018-05-05T00:47:33Z

bin/build-phar.sh

-[ -d build ] || mkdir build
-[ -d build/psalm ] || mkdir build/psalm
-# increase FD limit, or Phar compression will fail
-ulimit -Sn 4096


Fixed in Box now

theofidry · 2018-05-05T00:48:24Z

bin/build-phar.sh

-rm -f bin/psalm.phar
-
-# Prefixes the code to be bundled
-php -d memory_limit=-1 `which php-scoper` add-prefix --prefix='PsalmPhar' --output-dir=build/psalm --force


the scoping is done via Box now, the only issue is humbug/php-scoper#178 so right now the prefix is really random

the memory is also bumped by Box although the number is quite arbitrary so it may need more

The deterministic prefix was on purpose to enable better cache re-use, but I don't really mind caches being busted between new phars.

theofidry · 2018-05-05T00:49:17Z

bin/build-phar.sh

-
-# Re-dump the loader to account for the prefixing
-# and optimize the loader
-composer dump-autoload --working-dir=build/psalm --classmap-authoritative --no-dev


including only the non-dev files (and avoiding to scope the dev files) is done by Box

theofidry · 2018-05-05T00:51:38Z

box.json.dist

    "chmod" : "0755",
-    "shebang" : "#!/usr/bin/env php",
-    "stub" : true


this is the default value already

theofidry · 2018-05-05T00:52:38Z

box.json.dist

    "chmod" : "0755",
-    "shebang" : "#!/usr/bin/env php",
-    "stub" : true
+    "compression": "GZ"


there was no compression, not sure if that was voluntary because you wanted to avoid your users to have the zlib extension

Yes, definitely on purpose (because bzip was causing issues for users without it). If 99% of PHP installs have zlib, happy to use that.

theofidry · 2018-05-05T00:53:00Z

scoper.inc.php

 return [
-    'finders' => [


this part (finding the files) is handled by Box

theofidry · 2018-05-05T00:53:22Z

scoper.inc.php

@@ -136,4 +115,7 @@ function ($filePath, $prefix, $contents) {
            return $contents;
        },
    ],
+    'whitelist' => [


this whitelist is useless so could be removed

theofidry · 2018-05-05T00:54:13Z

src/psalm.php

@@ -127,7 +127,7 @@
    $options['r'] = $options['root'];
 }

-$current_dir = (string)getcwd() . DIRECTORY_SEPARATOR;
+$current_dir = (string)getcwd();


I'm not quite sure why we have the trailing /. This was making fail the find autoloader since we ended up with // in the path

theofidry · 2018-05-05T17:57:10Z

@muglug in theory this should be ready. I think however some code changed and maybe the PHP-Scoper config needs to be updated.

theofidry · 2018-05-14T22:56:20Z

Updated and the builds are green.

I may have missed them, but if they is no e2e tests for the PHAR I would recommend to add one (in another PR)

muglug · 2018-05-15T18:30:24Z

Thanks a lot!

theofidry mentioned this pull request Apr 26, 2018

Box adoption in the wild box-project/box#19

Closed

34 tasks

theofidry commented May 1, 2018

View reviewed changes

theofidry mentioned this pull request May 1, 2018

Check the ulimit box-project/box#187

Closed

theofidry force-pushed the master branch from 632c7c6 to 9ebb904 Compare May 5, 2018 00:46

theofidry commented May 5, 2018

View reviewed changes

theofidry added 2 commits May 14, 2018 23:48

Simplify the PHAR build script

9ecbaab

Update Box

2df9d2d

theofidry force-pushed the master branch from 9ebb904 to 2df9d2d Compare May 14, 2018 22:52

muglug merged commit 1ecf1c4 into vimeo:master May 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify the PHAR build script #656

Simplify the PHAR build script #656

theofidry commented Apr 8, 2018

coveralls commented Apr 8, 2018 •

edited

Loading

muglug commented Apr 9, 2018

muglug commented Apr 9, 2018

theofidry commented Apr 9, 2018 via email

theofidry May 1, 2018

weirdan May 1, 2018

weirdan May 1, 2018 •

edited

Loading

theofidry May 5, 2018

theofidry left a comment

theofidry May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

muglug May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

muglug May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

theofidry May 5, 2018

theofidry commented May 5, 2018

theofidry commented May 14, 2018

muglug commented May 15, 2018

		@@ -1,37 +1,6 @@
		#!/usr/bin/env bash
		if ! php -r 'extension_loaded("bz2") or exit(1);' ; then

Simplify the PHAR build script #656

Simplify the PHAR build script #656

Conversation

theofidry commented Apr 8, 2018

coveralls commented Apr 8, 2018 • edited Loading

muglug commented Apr 9, 2018

muglug commented Apr 9, 2018

theofidry commented Apr 9, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weirdan May 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theofidry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theofidry commented May 5, 2018

theofidry commented May 14, 2018

muglug commented May 15, 2018

coveralls commented Apr 8, 2018 •

edited

Loading

weirdan May 1, 2018 •

edited

Loading