Skip to content
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

Jump to bottom

Simplify the PHAR build script #656

Merged
merged 2 commits into from
May 15, 2018
Merged

Simplify the PHAR build script #656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ecbaab
Simplify the PHAR build script
theofidry Apr 8, 2018
2df9d2d
Update Box
theofidry May 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 11 additions & 9 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
vendor/
.DS_Store
phpunit.xml
composer.lock
.php_cs.cache
.php_cs
.*.swp
.*.swo
/build/logs/
/build/
/vendor/
/.DS_Store
/.php_cs.cache
/.php_cs
/.*.swp
/.*.swo
/composer.lock
/phpunit.xml
/vendor-bin/*/composer.lock
/vendor-bin/*/vendor/
37 changes: 3 additions & 34 deletions bin/build-phar.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,6 @@
#!/usr/bin/env bash
if ! php -r 'extension_loaded("bz2") or exit(1);' ; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is now handled in Box

echo "You need to install (or enable) bz2 php extension"
exit 1
fi
composer bin box install

composer global require 'humbug/php-scoper:^1.0@dev'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

composer global require 'humbug/box:3.0.0-alpha.0'
composer install --no-dev
[ -d build ] || mkdir build
[ -d build/psalm ] || mkdir build/psalm
# increase FD limit, or Phar compression will fail
ulimit -Sn 4096
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@weirdan weirdan May 1, 2018 •
edited

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in Box now

vendor/bin/box compile

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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn 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.


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

Choose a reason for hiding this comment

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

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


chmod 755 build/psalm/psalm

cp bin/phar.psalm.xml build/psalm/psalm.xml

./build/psalm/psalm --config=build/psalm/psalm.xml --root=build/psalm

php -d memory_limit=-1 -d phar.readonly=0 `which box` compile
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the readonly flag is taken care of by Box


# clean up build
rm -Rf build/psalm

# reinstall deps (to regenerate autoloader and bring back dev deps)
rm -Rf vendor/*
composer install
build/psalm.phar --config=bin/phar.psalm.xml
29 changes: 11 additions & 18 deletions box.json.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,16 @@
{
"alias": "psalm.phar",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there shouldn't be a need for an alias unless you really want to refer to phar://psalm.phar in the codebase which I didn't see

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this was just added for renaming in #595, but you fixed later.

"base-path": "build/psalm",
"main" : "./psalm",
"output" : "../../build/psalm.phar",
"directories" : [
"assets",
"vendor"
],
"finder": [
{
"in" : "src"
}
],
"files" : [
"config.xsd"
"output" : "build/psalm.phar",
"files": [
"src/command_functions.php",
"src/psalm.php",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there any reason why those files are not included in the autoload?

Copy link
Collaborator

Choose a reason for hiding this comment

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

They're part of Psalm's command-line startup, and themselves handle autoloading

"src/psalter.php"
],
"files-bin": ["config.xsd"],
"directories-bin" : ["assets"],
"intercept" : false,
"compactors" : [],
"compactors" : [
"KevinGH\\Box\\Compactor\\PhpScoper"
],
"chmod" : "0755",
"shebang" : "#!/usr/bin/env php",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is already the default shebang

"stub" : true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the default value already

"compression": "GZ"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

}
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
"phpunit/phpunit": "^5.7.4",
"friendsofphp/php-cs-fixer": "^2.3|^2.4|^2.5|^2.6|^2.7|^2.8|^2.9",
"squizlabs/php_codesniffer": "^3.0",
"php-coveralls/php-coveralls": "^2.0"
"php-coveralls/php-coveralls": "^2.0",
"bamarni/composer-bin-plugin": "^1.2"
},
"suggest": {
"ext-igbinary": "^2.0.5"
Expand Down
38 changes: 10 additions & 28 deletions scoper.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,6 @@
<?php

use Isolated\Symfony\Component\Finder\Finder;

return [
'finders' => [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this part (finding the files) is handled by Box

Finder::create()->files()->in('src'),
Finder::create()->files()->in('assets'),
Finder::create()
->files()
->ignoreVCS(true)
->notName('/LICENSE|.*\\.md|.*\\.dist|Makefile|composer\\.json|composer\\.lock/')
->exclude([
'doc',
'test',
'test_old',
'tests',
'Tests',
'vendor-bin',
])
->in('vendor'),
Finder::create()->append([
'composer.json',
'composer.lock',
'config.xsd',
'psalm'
]),
],
'whitelist' => [

],
'patchers' => [
function ($filePath, $prefix, $contents) {
//
Expand All @@ -46,6 +18,13 @@ function ($filePath, $prefix, $contents) {

return $contents;
},
function ($filePath, $prefix, $contents) {
return str_replace(
'\\'.$prefix.'\Composer\Autoload\ClassLoader',
'\Composer\Autoload\ClassLoader',
$contents
);
},
function ($filePath, $prefix, $contents) {
if ($filePath === realpath(__DIR__ . '/src/Psalm/Config.php')) {
return str_replace(
Expand Down Expand Up @@ -136,4 +115,7 @@ function ($filePath, $prefix, $contents) {
return $contents;
},
],
'whitelist' => [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this whitelist is useless so could be removed

\Composer\Autoload\ClassLoader::class,
]
];
7 changes: 7 additions & 0 deletions vendor-bin/box/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"humbug/box": "dev-master"
}
}