Proxy bin file regression in 2.2.0

[jenkoian]

Seems to be a regression in 2.2.0 (since RC-1 sorry for not testing earlier 😞 ) which can be summarised as a reintroduction of this bug: #10246 which I think was caused by this PR: #10137

Reproduction repo: https://github.com/jenkoian/composer-issue-10387

My composer.json:

{
    "name": "jenkoian/composer-issue-10387",
    "description": "Reproducer for composer issue #10387",
    "license": "MIT",
    "require": {
        "php": "^7.3"
    },
    "require-dev": {
        "phpunit/phpunit": "^9"
    }
}

Output of composer diagnose:

Checking composer.json: OK
Checking platform settings: OK
Checking git settings: OK
Checking http connectivity to packagist: OK
Checking https connectivity to packagist: OK
Checking github.com oauth access: OK
Checking disk free space: OK
Checking pubkeys:
Tags Public Key Fingerprint: 57815BA2 7E54DC31 7ECC7CC5 573090D0  87719BA6 8F3BB723 4E5D42D0 84A14642
Dev Public Key Fingerprint: 4AC45767 E5EC2265 2F0C1167 CBBB8A2B  0C708369 153E328C AD90147D AFE50952
OK
Checking composer version: OK
Composer version: 2.2.0
PHP version: 7.4.27
PHP binary path: /opt/homebrew/Cellar/php@7.4/7.4.27/bin/php
OpenSSL version: OpenSSL 1.1.1m  14 Dec 2021
cURL version: 7.80.0 libz 1.2.11 ssl (SecureTransport) OpenSSL/1.1.1m
zip: extension present, unzip present, 7-Zip not available

When I run this command:

vendor/bin/phpunit

I get the following output:

PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.27
Configuration: /Users/ianjenkins/Code/composer-issue/phpunit.xml.dist

E                                                                   1 / 1 (100%)

Time: 00:00.043, Memory: 6.00 MB

There was 1 error:

1) ExampleTest::test_example
PHPUnit\Framework\Exception: PHP Fatal error:  strict_types declaration must be the very first statement in the script in /Users/ianjenkins/Code/composer-issue/vendor/phpunit/phpunit/phpunit on line 2
PHP Stack trace:
PHP   1. {main}() Standard input code:0

Fatal error: strict_types declaration must be the very first statement in the script in /Users/ianjenkins/Code/composer-issue/vendor/phpunit/phpunit/phpunit on line 2

Call Stack:
    0.0004     548552   1. {main}() Standard input code:0

/Users/ianjenkins/Code/composer-issue/vendor/phpunit/phpunit/phpunit:91

And I expected this to happen:

PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 00:00.039, Memory: 6.00 MB

OK (1 test, 1 assertion)

[GrahamCampbell]

Seeing a similar issue trying to run psalm.

vendor/bin/psalm.phar:92:9: MissingFile: Cannot find file composer-bin-proxy:/home/runner/work/repo/vendor/psalm/phar/psalm.phar

[matks]

We do have a similar issue on PrestaShop CI

We have narrowed down the problem. PR #10137 is believed to be the root cause as mentioned by @jenkoian. When running PrestaShop phpunit suite test, we used vendor/bin/phpunit which was a symlink when using Composer 2.1 . vendor/bin/phpunit is now a proxy file, when using Composer 2.2, that use include to include the phpunit binary.

It seems a combination of phpunit declaring the use of strict type and the use of include inside the proxy file generates the error

PHP Fatal error:  strict_types declaration must be the very first statement in the script in /home/runner/work/PrestaShop/PrestaShop/vendor/phpunit/phpunit/phpunit on line 2

In the short term, we work around the issue by running our test suite using the real phpunit binary vendor/phpunit/phpunit/phpunit instead of the proxy vendor/bin/phpunit

[Seldaek]

This should not happen in the latest iteration of the bin proxy code unless perhaps if you have stream_wrapper_register in disable_functions? Or if there's a bug I suppose.

It'd be good if those able to repro could investigate why it doesn't do the include using stream wrapper inside the bin proxy file.

BTW another mitigation to this is upgrading to php 8..

[Seldaek]

Another point would be to make sure you have the latest proxy code by removing the proxy files and running composer install again, altho I doubt that's the problem if you first saw the regression after upgrading to composer 2.2.0

@GrahamCampbell that seems like an unrelated issue that I've never seen, maybe best report it separately.

[Seldaek]

Indeed if stream_wrapper_register is disabled you'll see this error. I don't know what else would trigger it, nor why you'd have stream wrappers disabled..

With the composer.json from OP I see this:

$ php7.4 -ddisable_functions=stream_wrapper_register vendor/bin/phpunit
PHP Fatal error:  strict_types declaration must be the very first statement in the script in /var/www/composer/testd/vendor/phpunit/phpunit/phpunit on line 2

$ php7.4 vendor/bin/phpunit
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.
[...]

[stof]

maybe the generated proxy should check for that case

[herndlm]

Just wanted chime in and add that indeed f12a5b8 / #10137 is the root cause, confirmed with the commands posted 2 posts above via git bisect. But that's no big news I guess.. :)
UPDATE: wait I just figured out now that this test was a bit stupid and the outcome was expected. sorry

[atomiix]

@Seldaek I have 2 different results when disabling stream_wrapper_register or not:

$ php -d date.timezone=UTC ./vendor/bin/phpunit -c tests/Integration/phpunit.xml
PHPUnit 8.5.16 by Sebastian Bergmann and contributors.

E

Time: 241 ms, Memory: 26.00 MB

There was 1 error:

1) Tests\Integration\Adapter\Cart\CartPresenterTest::testProductAttributesAreProperlyConverted with data set #0 ('Taille : S- Couleur : Noir', array('S', 'Noir'))
PHPUnit\Framework\Exception: PHP Fatal error:  strict_types declaration must be the very first statement in the script in /Users/atomiix/Sites/prestashop-dev/vendor/phpunit/phpunit/phpunit on line 2
Fatal error: strict_types declaration must be the very first statement in the script in /Users/atomiix/Sites/prestashop-dev/vendor/phpunit/phpunit/phpunit on line 2

/Users/atomiix/Sites/prestashop-dev/vendor/phpunit/phpunit/phpunit:60

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

Remaining deprecation notices (2)

$ php -ddisable_functions=stream_wrapper_register -d date.timezone=UTC ./vendor/bin/phpunit -c tests/Integration/phpunit.xml
PHP Fatal error:  strict_types declaration must be the very first statement in the script in /Users/atomiix/Sites/prestashop-dev/vendor/phpunit/phpunit/phpunit on line 2

Fatal error: strict_types declaration must be the very first statement in the script in /Users/atomiix/Sites/prestashop-dev/vendor/phpunit/phpunit/phpunit on line 2

[jenkoian]

Added a GH workflow to run the reproduction against various PHP versions: https://github.com/jenkoian/composer-issue-10387/runs/4616783070?check_suite_focus=true

It seems to fail on PHP8(.1) as well but for a different reason (times out).

[Seldaek]

OK it seems I can repro with the repository from @jenkoian - with php8 it just hangs forever but with php7 it indeed breaks due to processIsolation=true in phpunit.xml.dist. @matks @atomiix do you also use process isolation in phpunit?

[atomiix]

@Seldaek yes we do!

[jenkoian]

If it helps I added a functions_exists('stream_wrapper_register') line to the debugging step (https://github.com/jenkoian/composer-issue-10387/runs/4616842731?check_suite_focus=true#step:6:97)

[Seldaek]

Yeah this isn't related to stream_wrapper_register IMO, otherwise it would fail without even starting PHPUnit. It seems to break the PHPUnit process isolation but I am not entirely sure how/why yet.

[jenkoian]

@Seldaek good spot with the process isolation, I didn't even notice that, but did copy the phpunit config from the project I was seeing the issue. Can confirm it works just fine if I set that to false.

[jenkoian]

Ok padded out the workflow a bit, hope it helps https://github.com/jenkoian/composer-issue-10387/actions/runs/1615203291

tl;dr

[Seldaek]

It seems the global state collection/restore process of PHPUnit includes all included files again which includes phpunit itself then because we now include it instead of running it directly.

I got a "fix" here by adding $GLOBALS['__PHPUNIT_ISOLATION_EXCLUDE_LIST'] = [realpath(__DIR__ . '/..'.'/phpunit/phpunit/phpunit')]; to the vendor/bin/phpunit proxy. And I opened sebastianbergmann/phpunit#4846 which should fix it at the source..

Will keep digging a bit to see if I find a better way to solve this.

[Seldaek]

OK I think the workaround should work, feel free to give it a shot with composer self-update --snapshot. A proper fix in PHPUnit to handle our new proxies would be better in the long term, but that'll take a bit longer to sort out.

[jenkoian]

Confirmed! https://github.com/jenkoian/composer-issue-10387/actions/runs/1615405186 Many thanks 🙏

[Seldaek]

2.2.2 is now out with this fix 👍🏻

[jrfnl]

Thank you all for all your work on this.

Unfortunately, this still doesn't solve the problem for packages where PHPUnit is a non-dev dependency and a range of PHPUnit versions is supported.

Case in point: the BrainMonkey test utilities package.

While running the CI with --prefer-highest and Composer 2.2.2, the tests now run fine (except for PHP 5.6).
However, while running with --prefer-lowest and Composer 2.2.2, the test runs hang indefinitely (including on PHP 5.6 with highest).

For now, I'm "fixing" this for the BrainMonkey CI by using Composer 2.1 when running with --prefer-lowest or PHP 5.6, but as this is a test utility, I have a niggly feeling this may also affect consumers of this package when they use process isolation in the tests.

[Seldaek]

@jrfnl iirc my commit should fix it for phpunit 6.x (not sure from which minor/patch) and above, but earlier releases do not have the global var to exclude files from being included so for those I think projects would have to mark the vendor/phpunit/phpunit/phpunit file as excluded somewhere in the config, or to workaround the issue execute vendor/phpunit/phpunit/phpunit directly instead of using vendor/bin/phpunit.

I am hoping the amount of projects still using such old phpunit versions combined with proc isolation will be small enough that this is manageable on a case by case basis.

[jrfnl]

Thanks for your reply @Seldaek.

@jrfnl iirc my commit should fix it for phpunit 6.x (not sure from which minor/patch) and above, but earlier releases do not have the global var to exclude files from being included

Ah! That explains the outlier on PHP 5.6 I was seeing (using PHPUnit 5.x)

to workaround the issue execute vendor/phpunit/phpunit/phpunit directly instead of using vendor/bin/phpunit.

Would this work-around also work when using a Composer script ? I know of quite a few projects using scripts like the below to have things execute on the same PHP version as used by Composer (which may not be the system default), so the issue would not exist for any of those (which would be great!).

	"scripts": {
		"test": [
			"@php ./vendor/phpunit/phpunit/phpunit"
		]
	}

(For anyone wondering why this doesn't use "@php vendor/bin/phpunit" - that's to ensure compatibility with Windows)

I am hoping the amount of projects still using such old phpunit versions combined with proc isolation will be small enough that this is manageable on a case by case basis.

Well, considering WP is still insisting on a PHP 5.6 minimum (yeah, I know, I'm trying to change it, but there's lots of resistance), I fear that amount of projects may be larger than expected...

[Seldaek]

OK I dug up some more info:

• The fix in Composer 2.2.2 works for PHPUnit 6.5.0 and above, as they have support for __PHPUNIT_ISOLATION_BLACKLIST (and __PHPUNIT_ISOLATION_EXCLUDE_LIST in newer versions).
• For versions below that, perhaps executing this somewhere in the test bootstrap would workaround the issue PHPUnit_Util_Blacklist::$blacklistedClassNames['PHPUnit_Framework_TestCase'] = 3;
• I see an alternative hack to fix it in Composer, as the file excludes files with phpvfs:// (see https://github.com/sebastianbergmann/phpunit/blob/5.7.27/src/Util/GlobalState.php#L53-L78) I think I can work something up that'll work, but it's going to get a little ugly in BinaryInstaller :(

[Seldaek]

@jrfnl added 2a731ef with the extra hackery I mentioned above. This seems to resolve it for me on all PHP + PHPUnit versions combinations.

Interested if you can confirm though with composer self-update --snapshot.

[jrfnl]

@Seldaek That sounds awesome! I'm going to set up a complete test run for BrainMonkey now and am keeping my fingers crossed 🤞🏻

[jrfnl]

@Seldaek Test run finished, definitely an improvement, but there's still some tweaks needed.

Seeing the below on PHP 7.4 with --prefer-lowest:

PHP Parse error:  syntax error, unexpected ''You c' (T_ENCAPSED_AND_WHITESPACE) in phpvfs1:///home/runner/work/BrainMonkey/BrainMonkey/vendor/phpunit/phpunit/phpunit on line 43

And this on PHP 7.4 with --prefer-highest:

PHP Parse error:  syntax error, unexpected end of file in phpvfs1:///home/runner/work/BrainMonkey/BrainMonkey/vendor/phpunit/phpunit/phpunit on line 87

And it looks like the builds on PHP 5.6 (both) and PHP 7.0/7.1 with --prefer-lowest are still hanging (= all PHPUnit 5.x).

Doing a run with fail-fast: false now - output will be available here: https://github.com/Brain-WP/BrainMonkey/actions/runs/1638263233
(commit with the CI change: Brain-WP/BrainMonkey@d7ae7e5)

[stronk7]

Just to share that with 2.2.2 (without the extra hackery, I think) we have started to get a number of "old" branches (still using phpunit 7.5, that's the "oldest" we use) , only with php74, some parse errors:

https://github.com/stronk7/moodle/runs/4668195077?check_suite_focus=true

And, with current phpunit 9.5.x... also a good number of php warnings with php73:

PHP Warning:  is_file(): Unable to find the wrapper "phpvfs1" - did you forget to enable it when you configured PHP? in /home/runner/work/moodle/moodle/vendor/phpunit/phpunit/src/Util/Filter.php on line 92

https://github.com/paulholden/moodle/runs/4666343704?check_suite_focus=true

Both seem to be related to the changes here... and they weren't happening before yesterday (2.2.2 release).

Ciao :-)

[Seldaek]

OK 164a769 should fix the parse errors and 09d1330 the is_file warnings. This is getting really nasty IMO but I'm hoping we at least are getting close to a complete solution.

cc @johnstevenson if you also want to try your tests again

[jrfnl]

@Seldaek ❤️

I'm re-running the build for BrainMonkey now: https://github.com/Brain-WP/BrainMonkey/runs/4668635915

Looks like the PHP 7.4 failures no longer break the builds, but I'm seeing a lot warnings on all PHP 7.2, 7.3 and 7.4 builds:

PHP Warning:  stat(): stat failed for /home/runner/work/BrainMonkey/BrainMonkey/vendor/phpunit/phpunit/patchwork.json in /home/runner/work/BrainMonkey/BrainMonkey/vendor/bin/phpunit on line 94

Test runs against PHPUnit 5.x still hang indefinitely though (both PHP 5.6 builds, PHP 7.0/7.1 with --prefer-lowest).

[Seldaek]

Huh.. really curious where that is coming from but won't have time to dig in tonight I am afraid.

I can't repro the hanging on PHPUnit 5.7.26 though :/ May have to try to reproduce exactly your build steps from CI and hopefully that triggers it.

[johnstevenson]

@Seldaek I can confirm this fixes the parsing errors in PHP 7.4 (and upwards, though not relevant here). Nice trick with phpvfscomposer wrapper name.

[jrfnl]

Huh.. really curious where that is coming from but won't have time to dig in tonight I am afraid.

No worries. I really appreciate all you are trying to do to mitigate the fall-out of Composer 2.2.
I imagine this must be so frustrating for you! Shall we say for next year: let's not release a significant minor just before the holidays ?

I can't repro the hanging on PHPUnit 5.7.26 though :/ May have to try to reproduce exactly your build steps from CI and hopefully that triggers it.

Please let me know if there's anything I can do to help with this. The CI script is actually pretty straight-forward, but the repo is combining a number of test utilities (Patchwork, Mockery and BrainMonkey native), so that may be making things more complex.

Let me do a test run with WP Core with the current dev version of Composer. I heard they were having some issues with PHP 5.6/PHPUnit 5.x builds as well.

[jrfnl]

Let me do a test run with WP Core with the current dev version of Composer. I heard they were having some issues with PHP 5.6/PHPUnit 5.x builds as well.

Not seeing any failures on any of the PHP 5.6 with PHPUnit 5.7.27 jobs on WP Core at the moment, so at least, that's one less thing to worry about ;-)

[Seldaek]

Shall we say for next year: let's not release a significant minor just before the holidays ?

Yes and no ;) I am sure this would have been more stressful with everyone at work freaking out at the same time.

Please let me know if there's anything I can do to help with this. The CI script is actually pretty straight-forward, but the repo is combining a number of test utilities (Patchwork, Mockery and BrainMonkey native), so that may be making things more complex.

75e4d30 fixes the warnings coming from Patchwork.

I still cannot reproduce the hanged phpunit run locally with PHP 5.6. I have all the exact same dependencies as your 5.6 highest build installed, but somehow it runs fine.

$ php5.6 ./vendor/bin/phpunit
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

...............................................................  63 / 226 ( 27%)
............................................................... 126 / 226 ( 55%)
............................................................... 189 / 226 ( 83%)
..S..................................                           226 / 226 (100%)

Time: 5.59 seconds, Memory: 30.50MB

OK, but incomplete, skipped, or risky tests!
Tests: 226, Assertions: 422, Skipped: 1.

[Seldaek]

Oh I see now what is going on.. we disabled snapshot builds for PHP < 7.2.5 in preparation of Composer 2.3 snapshots requiring 7.2.5, so it keeps using 2.2.2 for you on those old-PHP builds.

So this should be resolved with 2.2.3 👍🏻

[Seldaek]

2.2.3 is out now - let's hope we can finally put this issue behind for good 😄

[jrfnl]

@Seldaek I can't say it enough: thanks for all your work on resolving this! ❤️ I've just rerun the BrainMonkey master build and it's passing again, so I can confirm that everything we saw there is fixed 🎉