search

getsentry / sentry-symfony

The official Symfony SDK for Sentry (sentry.io)
https://sentry.io
MIT License
693 stars 169 forks source link

It doesn't work with composer 2 #383

Closed cataragak90 closed 3 years ago

cataragak90 commented 3 years ago

Is it supposed to work with composer 2?

Jean85 commented 3 years ago

It should, are you encountering any particular issue?

Yozhef commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@VincentLanglet you correctly thought the problem in PHPStan when in config autoload_files: pathto 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 commented 3 years ago

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

VincentLanglet commented 3 years ago

@VincentLanglet you correctly thought the problem in PHPStan when in config autoload_files: pathto 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 commented 3 years ago

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

VincentLanglet commented 3 years ago

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

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 commented 3 years ago

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

Oh but you already opened https://github.com/symfony/symfony/issues/38889 (and closed it too early IMHO).

Kocal commented 3 years ago

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 https://github.com/symfony/symfony/issues/38889 and https://github.com/phpstan/phpstan/issues/4022 to be fixed and configure the CI for using Composer 1 :sweat_smile: Thanks you @ossinkine for the reproducer :heart:

niklasnatter commented 3 years ago

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

Thanks in advance!

niklasnatter commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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:

ondrejmirtes commented 3 years ago

In symfony/proxy-manager-bridge this was fixed by stopping using Version class and doing feature detection instead: https://github.com/symfony/symfony/pull/39017

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

alexander-schranz commented 3 years ago

@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 commented 3 years ago

@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
Seldaek commented 3 years ago

See https://github.com/composer/composer/blob/94076c0bb980af15e29a72328f7bc1a41e46776f/src/Composer/InstalledVersions.php#L174-L195

alexander-schranz commented 3 years ago

@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 commented 3 years ago

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

alexander-schranz commented 3 years ago

@ondrejmirtes hope this helps you phpstan/phpstan#4366

Jean85 commented 3 years ago

FTR, I'm planning to work on https://github.com/Jean85/pretty-package-versions/issues/14 (as soon as I find the time), which would completely avoid the issue.

Seldaek commented 3 years ago

@Jean85 I doubt this would avoid the issue completely. If there are two vendor dirs at play with two different InstalledVersions classes etc, the only way to avoid the problem is making sure the right dataset is loaded at the right time. This is an ugly solution to an ugly problem, but if people don't install their dev deps as dev deps that's what we end up with :)

VincentLanglet commented 3 years ago

@Jean85 I doubt this would avoid the issue completely. If there are two vendor dirs at play with two different InstalledVersions classes etc, the only way to avoid the problem is making sure the right dataset is loaded at the right time. This is an ugly solution to an ugly problem, but if people don't install their dev deps as dev deps that's what we end up with :)

So https://github.com/symfony/phpunit-bridge is a bad practice ? @Seldaek

Jean85 commented 3 years ago

@Seldaek you're right, I was conflating an other issue with that, which was https://github.com/Ocramius/PackageVersions/issues/165.

At the very least, with Jean85/pretty-package-versions#14 I'll be referencing the vendor, cutting out any middleman, and from there on out I can't do much more.

Jean85 commented 3 years ago

I released https://github.com/Jean85/pretty-package-versions/issues/14 with the 2.0, but I just moved the issue, as evident in the CI of https://github.com/getsentry/sentry-php/pull/1170

Having two autoloaders is a total mess :cry:

niklasnatter commented 3 years ago

I released Jean85/pretty-package-versions#14 with the 2.0, but I just moved the issue, as evident in the CI of getsentry/sentry-php#1170

Having two autoloaders is a total mess 😢

I cannot contribute anything valuable, but I really want to thank you for investing your time into this! 🙂

Jean85 commented 3 years ago

Thank you @nnatter.

Closing this as it is evident that the issue stems from having dev tools that load their own autoloader, messing with the main one. Can't fix it from this side.

nicolas-grekas commented 3 years ago

See https://github.com/composer/composer/pull/9635 for a possible fix in Composer.

Seldaek commented 3 years ago

https://github.com/composer/composer/pull/9635 is merged now, if someone can try after a composer self-update --snapshot if things are looking brighter :) It probably needs a composer install after to be run in the various places you have to make sure all vendor dirs are up to date with the latest InstalledVersions class.

alexander-schranz commented 3 years ago

@Seldaek I sadly still get the error:

I did run:

composer self-update --snapshot # 51371944e1c95b0fa3bc4ed921a366f553938ef3

# removed the both autoloader files and dependencies
rm -rf bin/.phpunit
rm -rf vendor/

# reinstall them
composer install
bin/phpunit install # install phpunit bridge 

# run phpstan
vendor/bin/phpstan analyse

And did get again:

 ------ --------------------------------------------------------------
  Line   src/Security/LoginFormAuthenticator.php
 ------ --------------------------------------------------------------
         Internal error: Package "sentry/sentry" is not installed
         Run PHPStan with --debug option and post the stack trace to:
         https://github.com/phpstan/phpstan/issues/new
 ------ --------------------------------------------------------------

The generated InstalledVersion.php

vendor/composer/InstalledVersions.php bin/.phpunit/phpunit-8.5-0/vendor/composer/InstalledVersions.php

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.0-dev+51371944e1c95b0fa3bc4ed921a366f553938ef3
PHP version: 7.4.9 - Package overridden via config.platform, actual: 7.4.14
PHP binary path: /usr/local/Cellar/php@7.4/7.4.14_1/bin/php
OpenSSL version: OpenSSL 1.1.1i  8 Dec 2020
cURL version: 7.74.0 libz 1.2.11 ssl (SecureTransport) OpenSSL/1.1.1i
zip: extension present, unzip present

My phpstan.neon file:

includes:
    - vendor/jangregor/phpstan-prophecy/extension.neon
    - vendor/phpstan/phpstan-doctrine/extension.neon
    - vendor/phpstan/phpstan-doctrine/rules.neon
    - vendor/phpstan/phpstan-symfony/extension.neon
    - vendor/phpstan/phpstan-phpunit/extension.neon
    - vendor/phpstan/phpstan-phpunit/rules.neon
    - vendor/phpstan/phpstan-webmozart-assert/extension.neon
    - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon

parameters:
    level: max
    paths:
        - src
        - tests
        - config
    excludes_analyse:
        - src/Migration/*
        - config/*
    inferPrivatePropertyTypeFromConstructor: true
    bootstrapFiles:
        - bin/.phpunit/phpunit-8.5-0/vendor/autoload.php
    symfony:
        container_xml_path: %rootDir%/../../../var/cache/website/dev/App_KernelDevDebugContainer.xml
        console_application_loader: tests/phpstan/console-application.php
    doctrine:
        objectManagerLoader: tests/phpstan/object-manager.php
VincentLanglet commented 3 years ago

I'm not sure the self-update should fix the issue since the issue is in vendor/composer.

Isn't this related to the fact that one of the dependencies are requiring composer instead ? I fixed the issue with a conflict in my composer.json composer/composer: <2. So I assume that a new composer release is the only way to test (with a composer update).

Seldaek commented 3 years ago

If something requires composer, you can require it with 2.0.x-dev to get the latest.

Seldaek commented 3 years ago

@alexander-schranz testing with phpstan is tricky because it may be including its own internal autoloader in the phar file as I understand it.. That won't be up to date yet for sure.

ondrejmirtes commented 3 years ago

PHPStan's PHAR is not yet built with Composer v2 so there's no InstalledVersions.php inside.

Jean85 commented 3 years ago

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

Seldaek commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

ondrejmirtes commented 3 years ago

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

ondrejmirtes commented 3 years ago

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

Seldaek commented 3 years ago

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

ondrejmirtes commented 3 years ago

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

ondrejmirtes commented 3 years ago

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 commented 3 years ago

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

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:

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

ondrejmirtes commented 3 years ago

I think these steps should work:

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