Upgrade to Box 3 for phar generation. #691
Conversation
jwage
force-pushed
the
upgrade-box
branch
6 times, most recently
from
c248830 to
6d043b8
Compare
box.json
Outdated
| @@ -1,23 +1,11 @@ | |||
| { | |||
| "alias": "doctrine-migrations.phar", | |||
| "compactors": [ | |||
There was a problem hiding this comment.
I don't think it's gonna make a big difference, but you can keep the compactors, I would replace Composer with Php though
build-phar.sh
Outdated
| @@ -8,10 +8,4 @@ fi | |||
|
|
|||
| composer install --no-dev --optimize-autoloader | |||
There was a problem hiding this comment.
you shouldn't need that line: Box dump the optimized autoloader and already skips the dev deps
|
@theofidry I committed the box.phar since the tests rely on it. I guess the test could download it each time but I am not sure what is worse, relying on the internet for a test or checking in box.phar :) |
jwage
force-pushed
the
upgrade-box
branch
2 times, most recently
from
b85b9af to
bdc4f0d
Compare
| ] | ||
| ], | ||
| "platform": { | ||
| "php": "7.1.x" |
There was a problem hiding this comment.
@jwage Why exactly was this added?
a) it's invalid here, it must be in "config" node (has no effect here)
b) using 7.1.x is invalid, it must be a specific version (i.e. 7.1.3)
c) having this here will likely cause issues with SF4 since Migrations require ^7.1 and platform override therefore should be 7.1.0
There was a problem hiding this comment.
It was suggested by @theofidry but i must have gotten it wrong. Can you record an issue and I will fix today?
| @@ -5,7 +5,6 @@ reports/ | |||
| dist/ | |||
| tests/phpunit.xml | |||
| vendor/ | |||
| box.phar | |||
There was a problem hiding this comment.
If you commit it in the repo, you should at least ignore it from the archive (using .gitattributes) so that users of the package don't get the box phar downloaded in their projects.
| { | ||
| $boxPharPath = realpath(__DIR__ . '/../../../../box.phar'); | ||
|
|
||
| $compilePharCommand = sprintf('php %s compile -vvv', $boxPharPath); |
There was a problem hiding this comment.
this will break in case the path has spaces in them, as it misses proper escaping.
you should use new Process(array('php', $boxPharPath, 'compile', '-vvv')) to let the Process component escape each part properly.
| self::assertTrue($process->isSuccessful()); | ||
| self::assertTrue(file_exists($doctrinePharPath)); | ||
|
|
||
| $runDoctrinePharCommand = sprintf('php %s', $doctrinePharPath); |
|
|
||
| $process = new Process($runDoctrinePharCommand); | ||
|
|
||
| $process->start(function ($type, $buffer) use (&$output, &$successful) : void { |
Summary
Upgrade to Box 3 for PHAR generation.