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

deprecated PackageVersions\Versions class contains dev packages when using composer install --no-dev #17

Closed
pilif opened this issue Nov 6, 2020 · 5 comments

Comments

@pilif
Copy link

pilif commented Nov 6, 2020

When running

composer install --no-dev (composer v2.0.4)

the generated PackageVersions\Versions class will still contain information about dev-dependencies.

In our case, this badly breaks sentry/sentry which then proceeds to query for additional information for such packages and then ends up with an OutOfBounds exception being thrown by composer's own composer/InstalledVersions.php

I'll do some investigations as to the cause

@Seldaek
Copy link
Member

Seldaek commented Nov 11, 2020

Can you check if it works with the latest snapshot (composer self-update --snapshot)? I am wondering if perhaps this is the same issue as composer/composer#9457 - at least I can't reproduce what you describe.

@Seldaek
Copy link
Member

Seldaek commented Nov 11, 2020

Ah sorry I misread here.. It does seem indeed to contain all packages including dev deps..

But it seems like this was always the case with ocramius/package-versions, i.e. https://github.com/Ocramius/PackageVersions/blob/1.11.x/src/PackageVersions/Installer.php#L217

I guess the issue is now it reads from the Composer's native implementation if it's available and that one correctly only has the installed stuff, so it should filter which versions are listed accordingly.

In any case.. people should really migrate to Composer\InstalledVersions. If you can communicate that to sentry/sentry that'd be good, if the API is available they should use that, if not they can use this plugin still for Composer 1 users.

Seldaek added a commit that referenced this issue Nov 11, 2020
@Seldaek
Copy link
Member

Seldaek commented Nov 11, 2020

@pilif
Copy link
Author

pilif commented Nov 11, 2020

In any case.. people should really migrate to Composer\InstalledVersions. If you can communicate that to sentry/sentry that'd be good, if the API is available they should use that, if not they can use this plugin still for Composer 1 users.

that's precisely what I've done. I've replaced the sentry ModuleIntegration with a better one that makes use of composer 2.

The sentry implementation is shitty anyways, because it relies on that VERSIONS static array that's got two warnings next to it to not make use of it (through it being marked @internal and through a comment saying so).

@Seldaek
Copy link
Member

Seldaek commented Nov 11, 2020

🤦‍♂️ OK, still curious if you can confirm the fix works for you though.

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

No branches or pull requests

2 participants