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.

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

Update dependencies for PHAR distribution of PHPUnit 9.6 #5712

Closed
nikic opened this issue Feb 21, 2024 · 23 comments
Closed

Update dependencies for PHAR distribution of PHPUnit 9.6 #5712

nikic opened this issue Feb 21, 2024 · 23 comments
Labels
installation/phar type/bug Something is broken version/9 Something affects PHPUnit 9

Comments

@nikic
Copy link

nikic commented Feb 21, 2024

Q A
PHPUnit version 9.6.16
PHP version 7.4.21
Installation Method PHAR

Summary

Based on the composer.lock file in the phar, it currently contains phpdocumentor/type-resolver version 1.6.1, which is quite old (predating the release date of PHPUnit 9.6.16 significantly). In particular, it does not contain the fix for PHP-Parser 5 compatibility in phpDocumentor/TypeResolver#197, which is part of 1.7.4. This caused a good bit of confusion over in nikic/PHP-Parser#981, because an error was only reproducible with the phpunit phar release, but not the composer dev dependency.

Is it possible to update this dependency? It's not really obvious to me how this version is determined, because the composer.lock in the phar doesn't seem to match https://github.com/sebastianbergmann/phpunit/blob/9.6/composer.lock at all (that one does not contain phpdocumentor/type-resolver in the first place).

@llaville
Copy link

Hello @sebastianbergmann

As author of nikic/PHP-Parser#981, I approve this request, especially because it has also an impact on GitHub Action shivammathur/setup-php@v2.

This action uses the PHAR version of PHPUnit (see https://github.com/shivammathur/setup-php/blob/v2/dist/index.js#L969)

So, when we run a workflow with such declaration :

      uses: shivammathur/setup-php@v2
        with:
          php-version: 7.4
          tools: phpunit:9.6

We've got trouble ! For example : https://github.com/composer-unused/symbol-parser/actions/runs/7990344120/job/21818955371#step:5:15

Workaround, as suggested Nikita, is to use the vendor/bin/phpunit (installed with Composer).

And it's works ! See https://github.com/composer-unused/symbol-parser/actions/runs/7996055440

@sebastianbergmann sebastianbergmann added installation/phar version/9 Something affects PHPUnit 9 labels Feb 22, 2024
@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Feb 22, 2024

It's not really obvious to me how this version is determined, because the composer.lock in the phar doesn't seem to match

With PHPUnit 9.6, the composer.lock file that is in the 9.6 branch of this repository is not used to build phpunit.phar. When building the PHAR, additional packages are installed, which results in a modified composer.lock file. It is this modified composer.lock file that you see when you introspect the PHAR like so:

$ wget https://phar.phpunit.de/phpunit-9.6.phar
$ php phpunit-9.6.phar --composer-lock

For backward compatibility, the PHAR build process for PHPUnit 9.6 installs phpspec/prophecy with version constraint ^1.12.1. This depends on phpdocumentor/reflection-docblock which in turn depends on phpdocumentor/type-resolver.

Is it possible to update this dependency?

I do not know how. Let me explain:

PHPUnit's composer.json file on the 9.6 branch uses

"config": {
    "platform": {
        "php": "7.3.0"

to make sure that no package which requires a version of PHP that is newer than PHP 7.3.0 can be installed regardless of the actual PHP version that is used to build the PHAR, for instance. This leads to version 1.6.1 of phpdocumentor/type-resolver to be installed. When I change "php": "7.3.0" to "php": "8.3.0" in the above, then the PHAR build process uses version 1.8.1 of phpdocumentor/type-resolver.

This is due to the fact that version 1.6.1 of phpdocumentor/type-resolver uses the "php": "^7.2 || ^8.0" version constraint where version 1.8.1 of phpdocumentor/type-resolver uses the "php": "^7.4 || ^8.0" version constraint.

The only solution for this problem that I can see is a version of phpdocumentor/type-resolver that has phpDocumentor/TypeResolver#197 and can be installed with PHP 7.3.

@jrfnl
Copy link
Contributor

jrfnl commented Feb 22, 2024

@jaapio Any chance of TypeResolver supporting PHP 7.3 ?

@nikic
Copy link
Author

nikic commented Feb 22, 2024

@sebastianbergmann Thank you the detailed explanation!

The easiest solution would probably be to release a 1.6.2 version of phpdocumentor/type-resolver with that patch cherry-picked. But maybe this isn't worth the bother, given that using phpunit as a dev dependency avoids the problem.

@jaapio
Copy link
Contributor

jaapio commented Feb 22, 2024

I think we can fix that quite easy, will investigate that tomorrow and create a new release 1.6.2

@jaapio
Copy link
Contributor

jaapio commented Feb 22, 2024

I managed to have a look today, what would be the easiest way for me to validate this issue is solved before I create a new release? The new release will be based on 1.8.1 and has support for php 7.3 I cannot go lower without to many changed.

@sebastianbergmann
Copy link
Owner

what would be the easiest way for me to validate this issue is solved before I create a new release?

I do not how this could be validated before the release. Once that release has been made, you can use ant phar on PHPUnit's 9.6 branch. This should then use the new release.

@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

Ok, will give it a try

@sebastianbergmann
Copy link
Owner

will investigate that tomorrow and create a new release 1.6.2

1.6.2 was already tagged in October 2022. We are talking about 1.6.3 as the new version here, right?

@nikic
Copy link
Author

nikic commented Feb 23, 2024

Oh, that's tricky. I didn't realize that the PHP 7.4 requirement was added in 1.6.2, rather than 1.7.0. In that case the version would have to be something like 1.6.1-p1 I think? (Based on looking at https://getcomposer.org/doc/04-schema.md#version, though it's not explicit about how the versions sort there).

@llaville
Copy link

@nikic If we follow real semantic perharps. But for an older version I think we may accept to downgrade PHP compatibility between 1.6.2 (php: ^7.4 || ^8.0) and 1.6.3 (php: ^7.3 || ^8.0)
Look at download stats https://packagist.org/packages/phpdocumentor/type-resolver/stats#1.6.2

@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

I was thinking about reintroducing php 7.3 support in our main branch and release 1.8.2 because that version is compatible with php-parser 5

@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

PR is ready, please let me know what you think, apart from 7.3 drop there is nothing breaking, yes we had to remove all typed properties. But that's about it

@llaville
Copy link

@sebastianbergmann Just a little question about PHPUnit 9.6 : I don't see any schema 9.6.xsd into https://github.com/sebastianbergmann/phpunit/tree/main/schema ; is it expected ?

@llaville
Copy link

@jaapio I 've checked your PR and check your new branch php73 locally, rebuilding a PHPUnit 9.6 phar locally too !

I've re-tested again the https://github.com/composer-unused/symbol-parser project on PHP 7.4 platform

PHP 7.4.33 (cli) (built: Nov 15 2022 06:05:55) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.33, Copyright (c), by Zend Technologies
    with Xdebug v3.1.6, Copyright (c) 2002-2022, by Derick Rethans

With nikic/php-parser v5.0.1 requires php (>=7.4) installed.

With a rebuild of PHPUnit 9.6 with code of https://github.com/phpDocumentor/TypeResolver/tree/php73 seems works fine !

devilbox@php-7.4.33 in /shared/backups/forks/symbol-parser $ /shared/backups/github/phpunit/phpunit.phar --bootstrap phpunit.bootstrap.php --no-configuration -v tests/
PHPUnit 9.6.16 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.33

........................................................          56 / 56 (100%)

Time: 00:00.101, Memory: 20.00 MB

OK (56 tests, 105 assertions)

@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

Thanks for confirming, I will merge this and tag a new version

@llaville
Copy link

Thanks for confirming, I will merge this and tag a new version

Hope I don't missed anything ;-)

@llaville
Copy link

@jaapio Is it not supposed to release a 1.6.3 rather than https://github.com/phpDocumentor/TypeResolver/releases/tag/1.8.2 ?

@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

No, because the base version was 1.8.1, the 1.6 series is not compatible with the new php parser, it would have resulted in the same errors.

@sebastianbergmann
Copy link
Owner

I just noticed that with phpdocumentor/type-resolver 1.8.2 two new dependencies are pulled in: doctrine/deprecations and phpstan/phpdoc-parser:

-phpunit/phpunit: 9.6.16
+phpunit/phpunit: 9.6@b59fa6534869a57ccfe0142d08aef3db59cd2680
+doctrine/deprecations: 1.1.3
 doctrine/instantiator: 1.5.0
 myclabs/deep-copy: 1.11.1
 nikic/php-parser: v4.18.0
@@ -6,8 +7,9 @@
 phar-io/version: 3.2.1
 phpdocumentor/reflection-common: 2.2.0
 phpdocumentor/reflection-docblock: 5.3.0
-phpdocumentor/type-resolver: 1.6.1
+phpdocumentor/type-resolver: 1.8.2
 phpspec/prophecy: v1.18.0
+phpstan/phpdoc-parser: 1.25.0
 phpunit/php-code-coverage: 9.2.30
 phpunit/php-file-iterator: 3.0.6
 phpunit/php-invoker: 3.1.1

@sebastianbergmann sebastianbergmann changed the title Update phpdocumentor/type-resolver version in PHPUnit phar Update dependencies for PHAR distribution of PHPUnit 9.6 Feb 23, 2024
sebastianbergmann added a commit that referenced this issue Feb 23, 2024
@jaapio
Copy link
Contributor

jaapio commented Feb 23, 2024

That's correct, we use the doctrine/deprecations and stan parser to process the new types like array shapes

@sebastianbergmann
Copy link
Owner

I have released PHPUnit 9.6.17. The PHAR for this release has the updated dependencies.

@llaville
Copy link

I confirmed that all still run fine with official PHAR release 9.6.17 on PHP 7.4 and 8.2 (my quick tests about contextual project https://github.com/composer-unused/symbol-parser/ that used nikic/php-parser 4.18 or 5.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation/phar type/bug Something is broken version/9 Something affects PHPUnit 9
Projects
None yet
Development

No branches or pull requests

5 participants