Update PHAR creation to more up-to-date version #1462
Conversation
That way we do not have to install box ourselves which simplifies the PHAR creation
This removes some defaults that are no longer necessary as well as the files config that stopped Box from applying it'S own logic of figuring out what is necessary. This should now produce a working PHAR file that is similar to the one based on box2 but now with the latest box-releases so that bugfixes are much easier to be applied. This still is - similar to the previous version - not a scoped build so that there might be circumstances where the executed code uses code from the PHAR file instead of from their own code. This though is a separate issue.
There was a problem hiding this comment.
There's one failing build, looks like a dependencies issue?
The actual changes look fine to me if that's possible to resolve.
How can we test it works, aside from manually? It'd be good to test the PHAR as well, similar to how phpspec does it but that could be a future PR if this one's been manually verified.
|
Scoping the phar would be really hard because the API exposed to Behat extensions is not isolated from the symfony/dependency-injection API. |
It will only check whether the PHAR file can be basically executed. It is not testing every functionality of the PHAR!
|
Any particular reason why you are using PHP7.2 to build the PHAR and not a recent version? Otherwise I'd change that so that the PHAR file can be created by box (which requires at least 8.0) |
.github/workflows/build.yml
Outdated
| @@ -11,6 +11,7 @@ on: | |||
| jobs: | |||
| tests: | |||
| runs-on: ${{ matrix.os }} | |||
| continue-on-error: ${{ matrix.experimental }} | |||
There was a problem hiding this comment.
this is not the right approach for making experimental builds: this would continue the job even if setup-php fails for instance.
There was a problem hiding this comment.
According to the GithubAction docs this setting allows a job to "pass" even though one step failed. The job will still be marked as failed but the whole workflow will not fgail due to that one failed step.
If you have a different way of achieving this I am all ears!
There was a problem hiding this comment.
oh, it looks like they have separated the continue-on-error at the job level and at the step level now.
|
@heiglandreas this is used as an easy way of ensuring that the dependencies inside the phar are resolved as compatible with PHP 7.2. But this could be achieved in a different way by using |
Which might make sense anyhow. |
.github/workflows/build.yml
Outdated
| # Get the existing 7.2 to publish the phar | ||
| - php: 7.2 | ||
| # Test the next upcomming release of PHP. This needs to be allowed to fail | ||
| - php: 8.4 |
There was a problem hiding this comment.
It might be better to add this in a separate PR
Previously that was the lowest possible PHP-version. But as the build-tools only work with a more recent version of PHP and the PHP-version used to create the PHAR has no impact on the PHAR itself it makes sense to use the latest (or at least a rather recent) PHP-Version
This commit moves building the PHAR file out of the test-suite and moves it into it's own step that requires all biulds to pass to be executed. That makes it for one thing easier to make sure that all tests in all versions of PHP work before creating the PHAR and it also allows to easier optimize the build of the PHAR without interfering with tests. The build-steps from the test-part were taken over into the build-phar part and appropriately extended. Also this makes sure that the PHAr is build based on the PHP7.2 version so taht it can be ran in all supported PHP-Versions.
heiglandreas
force-pushed
the
fix-box-setup
branch
from
0ea96a1
to
9077627
Compare
This PR updates the PHAR creation process to use a rather current version of box.
It moves
a) from the old box2 to the current box
b) from downloading and preparing the build-tools ourselves to using the build-tools provided by shivamatur
c) towards using more internal logic from box to determine which files are to be included into the PHAR
The config file has been cleaned up and the github-action files have been updated accordingly.
Note: This just replaces the old process with an updated one. The PHAR file is still unscoped which might cause problems when the application executes code that calls classes that are available in the PHAR as well as in the tested codes dependencies as the ones from the PHAR will be used. But that is a separate issue from the update