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

It doesn't work with composer 2 #383

Closed
cataragak90 opened this issue Oct 27, 2020 · 59 comments
Closed

It doesn't work with composer 2 #383

cataragak90 opened this issue Oct 27, 2020 · 59 comments

Comments

@cataragak90
Copy link

Is it supposed to work with composer 2?

@Jean85 Jean85 added the support label Oct 27, 2020
@Jean85
Copy link
Collaborator

Jean85 commented Oct 27, 2020

It should, are you encountering any particular issue?

@Yozhef
Copy link
Contributor

Yozhef commented Oct 27, 2020

I am currently struggling with the problem as well. When run phpstan see error:
php -d memory_limit=-1 ./vendor/bin/phpstan --error-format=checkstyle analyse -l 5 -c phpstan.neon.dist --debug

Fatal error: Uncaught OutOfBoundsException: Package "project/project" is not installed in /app/payment/bin/.phpunit/phpunit-9-0/vendor/composer/InstalledVersions.php:451
Stack trace:
#0 /app/payment/vendor/composer/package-versions-deprecated/src/PackageVersions/Versions.php(254): Composer\InstalledVersions::getPrettyVersion('platform/paymen...')
#1 /app/payment/vendor/jean85/pretty-package-versions/src/PrettyVersions.php(13): PackageVersions\Versions::getVersion('platform/paymen...')
#2 /app/payment/vendor/jean85/pretty-package-versions/src/PrettyVersions.php(23): Jean85\PrettyVersions::getVersion('platform/paymen...')
#3 /app/payment/vendor/sentry/sentry-symfony/src/DependencyInjection/Configuration.php(117): Jean85\PrettyVersions::getRootPackageVersion()
#4 /app/payment/vendor/symfony/config/Definition/Processor.php(50): Sentry\SentryBundle\DependencyInjection\Configuration->getConfigTreeBuilder()
#5 /app/payment/vendor/symfony/dependency-injection/Extension/Extension.php(111): Symfony\Component\Config\Definition\Proc in /app/payment/bin/.phpunit/phpunit-9-0/vendor/composer/InstalledVersions.php on line 451

Sentry version:

"sentry/sentry-symfony": "3.5.*",
"jean85/pretty-package-versions": "^1.5 || ^2.0",

@Jean85
Copy link
Collaborator

Jean85 commented Oct 27, 2020

@Yozhef that seems unrelated; it also seems that you're doing something strange with composer install...

@cataragak90 the CI just run successfully with Composer 2.0.2: https://github.com/getsentry/sentry-symfony/runs/1315282756?check_suite_focus=true#step:4:11 so it doesn't seem an issue on my side.

@VincentLanglet
Copy link

@Yozhef that seems unrelated; it also seems that you're doing something strange with composer install...

It seems related to Composer because I had to add a conflict with Composer >= 2 to fix this error.

Why do you think there is something strange with composer install ? @Jean85
It's just because we're using symfony/phpunit-bridge and phpstan is looking the vendor inside the .phpunit folder too.

@Yozhef
Copy link
Contributor

Yozhef commented Oct 30, 2020

@VincentLanglet you correctly thought the problem in PHPStan when in config autoload_files: path to Symfony/Phpunit-bridge PHPUnit.
I fixed this problem by simply removing the configuration config autoload_files in PHPStan.
In fact, the problem was not in the sentry.

@Yozhef
Copy link
Contributor

Yozhef commented Oct 30, 2020

@Jean85 I think it is possible to close Sentry supports Composer 2 - we have already started testing our project.

@VincentLanglet
Copy link

@VincentLanglet you correctly thought the problem in PHPStan when in config autoload_files: path to Symfony/Phpunit-bridge PHPUnit.
I fixed this problem by simply removing the configuration config autoload_files in PHPStan.
In fact, the problem was not in the sentry.

Do you require phpunit in your project ?
Because I don't. So I have to keep

    bootstrapFiles:
        - symfony/bin/.phpunit/phpunit-9.3-0/vendor/autoload.php

In order to load the phpunit classes.

So I still believe there is something wrong between Composer 2 / Sentry/symfony and the usage of pretty version.

@Jean85
Copy link
Collaborator

Jean85 commented Oct 30, 2020

Which version of jean85/pretty-package-versions are you using? And which of the underlying ocramius package too?

@VincentLanglet
Copy link

If I remove the composer/composer >= 2 conflict and run composer upgrade I only have

  • Updating composer/semver (1.7.1 => 3.2.2): Loading from cache
  • Updating composer/composer (1.10.16 => 2.0.4): Loading from cache

I use jean85/pretty-package-versions 1.5.1 version.
I don't have ocramius/package-version ; I have composer/package-versions-deprecated

composer why ocramius/package-versions
Warning from https://repo.packagist.org: You are using an outdated version of Composer. Composer 2.0 is now available and you should upgrade. See https://getcomposer.org/2
composer/package-versions-deprecated  1.11.99  replaces  ocramius/package-versions (1.11.99)  
doctrine/migrations                   3.0.1    requires  ocramius/package-versions (^1.3)     
ocramius/proxy-manager                2.7.1    requires  ocramius/package-versions (^1.5.1) 

When phpunit-bridge is installing dependencies, a class InstalledVersions is in .phpunit/vendor/composer with only the dependencies needed by phpunit.
But there are none in my real vendor/composer

It seems like I'm using composer 1 for my project but phpunit is using composer 2. And then your library is believing I'm using composer 2.

@Jean85
Copy link
Collaborator

Jean85 commented Oct 31, 2020

It seems to me that you're having issues with the PHPUnit Bridge then...

Oh but you already opened symfony/symfony#38889 (and closed it too early IMHO).

@Kocal
Copy link

Kocal commented Nov 6, 2020

I'm having the same issue with the Sentry bundle, PHPStan and the PHPUnit bridge...

I think it could be solved by requiring "jean85/pretty-package-versions": "^1.5 || ^2.0" and removing explicit ocramius/package-versions dependency.

EDIT: Well, nope! I will wait for symfony/symfony#38889 and phpstan/phpstan#4022 to be fixed and configure the CI for using Composer 1 😅
Thanks you @ossinkine for the reproducer ❤️

@niklasnatter
Copy link

Hey,
just ran into this issue. Is there any workaround beside of using composer:1?

Thanks in advance!

@niklasnatter
Copy link

niklasnatter commented Jan 12, 2021

I investigated this a bit further - the error appears when adding the following configuration:

sentry:
    monolog:
        error_handler:
            enabled: true

monolog:
    handlers:
        sentry:
            type: service
            id: Sentry\Monolog\Handler

It is also somehow related to the the symfony/phpunit-bridge package; the error disappears if i remove the bin/.phpunit/phpunit-8.5-0/vendor/composer/InstalledVersions.php file in my project or remove the following code from my phpstan.neon:

parameters:
    bootstrapFiles:
        - bin/.phpunit/phpunit-8.5-0/vendor/autoload.php

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Jan 12, 2021

The problem here is that there is now a: vendor/composer/InstalledVersions.php and a bin/.phpunit/phpunit-8.5-0/vendor/vendor/composer/InstalledVersions.php loaded and the second does overwrite the first one.

There is a workaround by override the InstalledVersions again using the following in the phpstan.neon file:

    bootstrapFiles:
        - bin/.phpunit/phpunit-8.5-0/vendor/autoload.php
        - vendor/composer/InstalledVersions.php # workaround for: https://github.com/getsentry/sentry-symfony/issues/383

@VincentLanglet
Copy link

@alexander-schranz Who do you think this issue should be addressed to ?

Either their could be something to improve to phpunit-bridge, either the documentation of phpstan could help to correctly use the bridge.

@alexander-schranz
Copy link
Contributor

Not sure where this could be fixed its a combination of 3 different libraries so maybe we can get some feedback from one of there core contributers:

  • @composer (@Seldaek)
    • require 2 different composer autoloader loses the InstalledVersion so composer could workaround this problem pushing to a global variable instead of dump a class with it as static which will override it.
  • @symfony phpunit-bridge (@nicolas-grekas)
    • the phpunit bridge could remove the bin/.phpunit/phpunit-8.5-0/vendor/vendor/composer/InstalledVersions.php on installation not sure what composer would say about this but also did not get an error about it aslong I'm not use the composer api to get the phpunit/phpunit version.
  • @symfony phpstan-phpunit extension (@ondrejmirtes)
    • phpstan could maybe make sure the correct "vendor/composer/InstalledVersions.php" one is loaded or document it

@ondrejmirtes
Copy link

In symfony/proxy-manager-bridge this was fixed by stopping using Version class and doing feature detection instead: symfony/symfony#39017

I guess a similar thing could be done in phpunit-bridge as well.

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Jan 12, 2021

@ondrejmirtes thx for the fast response 🚀 . Sentry uses the version to get the version of itself (example). To tell there api which version of there library did the request the endpoint.

So the fix sentry could do here is adding a Version constant and don't use a composer plugin to get its installed version.

@ondrejmirtes
Copy link

@alexander-schranz Also what would probably help with the problem would be to get the version lazily once it's needed so that it gets out of this stack trace:

#0 /app/payment/vendor/composer/package-versions-deprecated/src/PackageVersions/Versions.php(254): Composer\InstalledVersions::getPrettyVersion('platform/paymen...')
#1 /app/payment/vendor/jean85/pretty-package-versions/src/PrettyVersions.php(13): PackageVersions\Versions::getVersion('platform/paymen...')
#2 /app/payment/vendor/jean85/pretty-package-versions/src/PrettyVersions.php(23): Jean85\PrettyVersions::getVersion('platform/paymen...')
#3 /app/payment/vendor/sentry/sentry-symfony/src/DependencyInjection/Configuration.php(117): Jean85\PrettyVersions::getRootPackageVersion()
#4 /app/payment/vendor/symfony/config/Definition/Processor.php(50): Sentry\SentryBundle\DependencyInjection\Configuration->getConfigTreeBuilder()
#5 /app/payment/vendor/symfony/dependency-injection/Extension/Extension.php(111): Symfony\Component\Config\Definition\Proc in /app/payment/bin/.phpunit/phpunit-9-0/vendor/composer/InstalledVersions.php on line 451

@alexander-schranz
Copy link
Contributor

@ondrejmirtes could not find a way for it, it seems always be triggered because used at constructing and configure container services.
@Seldaek thank you also for the fast feedback. @ondrejmirtes do you think this could be something phpstan could call automatically?

@ondrejmirtes
Copy link

PHPStan could merge arrays from all the used Composer autoloaders, but I'd need a failing integration test similar to this one phpstan/phpstan#4047 first from someone that's experiencing this error. Thanks.

@Jean85
Copy link
Collaborator

Jean85 commented Jan 27, 2021

@ondrejmirtes any reason for not doing it yet? Can we help somehow?

@Seldaek
Copy link

Seldaek commented Jan 27, 2021

@ondrejmirtes but it'll load an outdated ClassLoader which makes this whole fix in Composer not work if the ClassLoader does not have getRegisteredLoaders on it. So regardless it's not the best use case to test if this fix works.

@ondrejmirtes
Copy link

Until recently humbug/box or php-scoper combo weren't compatible with Composer v2. Also I thought that building PHAR with Composer v2 would actually make this problem worse. Because right now to bump into this problem (if I understand it correctly) you need to have two conflicting Composer v2 vendor directories. So when PHPStan is built with Composer v1, there need to be 2 additional Composer autoloaders present in order for this to be a problem, but with PHPStan built with Composer v2, it'd happen with only 1 additional autoloader.

If you need PHPStan built with Composer v2 in order to verify this fix, I can look into it again and produce the updated PHAR for phpstan/phpstan dev-master today.

@Seldaek
Copy link

Seldaek commented Jan 27, 2021

I guess the thing is @alexander-schranz has vendor/ and bin/.phpunit/phpunit-8.5-0/vendor/ so two vendor dirs, but also bootstrapFiles: bin/.phpunit/phpunit-8.5-0/vendor/autoload.php so when running phpstan I don't know if phpstan would automatically load vendor/autoload.php, or first load the autoload.php from phpunit? But I guess if it loads both, in the wrong order, and first loaded the ClassLoader from phpstan (composer v1), then I can see how that would go bad yes. If there's a way to make sure vendor/autoload.php gets loaded before bin/.phpunit/phpunit-8.5-0/vendor/autoload.php, that would be a workaround too, but that would not confirm the fix in Composer.

@ondrejmirtes
Copy link

The order of loading is:

  1. require_once vendor/autoload.php from inside PHPStan's PHAR
  2. Preload some classes eagerly from PHPStan's PHAR
  3. ->unregister() on the autoloader from 1)
  4. require_once autoloader in working directory - $CWD/vendor/autoload.php
  5. require_once project's autoloader - it looks where the phpstan.phar that's executed is and tries to looks for '/../../autoload.php' from there.
  6. ->register(true) (prepend) the autoloader from 1) again.
  7. Execute registered bootstrapFiles.

So I think this is true in the current version of PHPStan:

If there's a way to make sure vendor/autoload.php gets loaded before bin/.phpunit/phpunit-8.5-0/vendor/autoload.php, that would be a workaround too

@Seldaek
Copy link

Seldaek commented Jan 27, 2021

Ok then I don't understand what is going on, but I also don't have all the information..

@ondrejmirtes
Copy link

I'll just try to build the PHAR with Composer v2 and see what happens.

@ondrejmirtes
Copy link

I tried building PHPStan PHAR with composer:snapshot but looks like setup-php doesn't have the latest version yet, it's on composer/composer@125f8a3. I guess we'll have to wait.

@Seldaek
Copy link

Seldaek commented Jan 27, 2021

You can add a composer self-update --snapshot to run after setup-php is done I believe.

@ondrejmirtes
Copy link

@Jean85 I bumped into this problem (box-project/box#528). I guess the PHAR is now completely broken for this purpose and won't be able to find any package version.

@ondrejmirtes
Copy link

Alright, so I was able to work around the problem. @alexander-schranz Can you please try the latest phpstan/phpstan dev-master to see if it works? Thank you!

@VincentLanglet
Copy link

I don't know if it can help you but I tried on my CI

  • composer self-update --snapshot
  • Adding "composer/composer": "2.0.x-dev" in my composer.json

But I still get

PHP Fatal error:  Uncaught OutOfBoundsException: Package "jdhm/asv" is not installed in /home/runner/work/asv_sf_modx/asv_sf_modx/symfony/bin/.phpunit/phpunit-9.5-0/vendor/composer/InstalledVersions.php:444

Tried to only change phpstan/phpstan to dev-master, but it doesn't work too.

But if I do all of this:

  • composer self-update --snapshot
  • Adding "composer/composer": "2.0.x-dev" in my composer.json
  • Update phpstan/phpstan to dev-master

It works !!!
So I think a release for composer/composer and phpstan/phstan should solve everything

@ondrejmirtes
Copy link

ondrejmirtes commented Jan 27, 2021

I think these steps should work:

  • composer self-update --snapshot
  • composer dump-autoload
  • Update phpstan/phpstan to dev-master

This shouldn't be necessary but it probably just triggered exporting the new autoloader:

Adding "composer/composer": "2.0.x-dev" in my composer.json

@alexander-schranz
Copy link
Contributor

@ondrejmirtes @Seldaek the issue is gone with the dev-master phpstan release 🎉

@ondrejmirtes
Copy link

Awesome, I'm happy to release PHPStan once Composer is released and setup-php GitHub action picks it up when using composer:v2 :)

@Seldaek
Copy link

Seldaek commented Jan 27, 2021

2.0.9 is out

@Jean85
Copy link
Collaborator

Jean85 commented Jan 27, 2021

Great job everyone!

@ondrejmirtes
Copy link

I just released PHPStan 0.12.70 that contains the fix :) https://github.com/phpstan/phpstan/releases/tag/0.12.70

@VincentLanglet
Copy link

@ondrejmirtes it does not work anymore for me.

e238668648be5ff3225875c39346bd9a16f5438b was working
but 0.12.70 does not.

@alexander-schranz
Copy link
Contributor

alexander-schranz commented Jan 27, 2021

For me it still works.

I did:

composer self-update --2 # did get the new 2.0.9
# removed the old autoloader files
rm -rf vendor
rm -rf bin/.phpunit

composer update phpstan/phpstan # to get phpstan 0.12.70 and in this case also the new autoloader

bin/phpunit install # generate the phpunit autoloader
vendor/bin/phpstan analyse # no errors did appear

@VincentLanglet did you in your case reinstall the dependencies so both autoloaders are also reinstalled?

@VincentLanglet
Copy link

I think I had some weird issue with my CI, it's fixed now :)

@ondrejmirtes
Copy link

And now the fun starts... phpstan/phpstan#4453 I hope it can be solved by the steps I've written there...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests