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

Jump to bottom

[Composer2] Downgrade transitive dependency to satisfy deps on "require" #8910

Closed
weaverryan opened this issue May 15, 2020 · 7 comments
Closed

[Composer2] Downgrade transitive dependency to satisfy deps on "require" #8910

weaverryan opened this issue May 15, 2020 · 7 comments
Milestone
2.0-core

Comments

@weaverryan
Copy link
Contributor

Hi!

Apologies if an issue exists or if this has been discussed - I couldn't find an issue immediately.

Suppose there are 2 packages:

  • A) acme/foo, which requires transitive/dependency: "^1 | ^2
  • B) acme/bar, which requires transitive/dependency: "^1

The problem:

# this WORKS - transitive/dependency 1 is downloaded
composer require acme/bar
composer require acme/foo

# this BREAKS. The first command installs and locks transitive/dependency at v2
# then, the second command fails because transitive/dependency v2 is locked and
# acme/bar only works with v1
composer require acme/foo
composer require acme/bar

In theory (because transitive/dependency is not a root requirement), Composer could notice that, while acme/bar doesn't work with v2, that dependency could be downgraded to v1 to satisfy all packages. That "should" (with huge air-quotes) be safe, as it's just a transitive dependency. However, if we're worried that users might be inadvertently using the dependency directly, and this could break things, we could (instead) issue a clear warning on how to fix this:

acme/bar requires transitive/dependency ^1, but version 2.0.0 is locked. However,
your project is compatible with transitive/dependency ^1 and it could be downgraded.
Re-run the command with --allow-major-downgrades to allow this dependency to
be downgraded.

(This invents a new --allow-major-downgrades flag).

I'm sure this shouldn't be done in v1, but is it possible for v2?
weaverryan added a commit to weaverryan/maker-bundle that referenced this issue May 15, 2020
@weaverryan
Dropping doctrine/inflector v2 support temporarily
4885da6 
This was causting doctrine/inflector v2 to be installed. Then,
if a user required another package that only worked with v1 - like
doctrine/orm, that package would fail to install because the user's
app is locked on v2.

Dropping support for v2 isn't really a fix, but will make it more
compatible with the ecosystem at the current time. Later, especially
when doctrine/orm allows 2, we can again.

See also composer/composer#8910
@weaverryan weaverryan mentioned this issue May 15, 2020
Merged
weaverryan added a commit to symfony/maker-bundle that referenced this issue May 15, 2020
@weaverryan
feature #611 Dropping doctrine/inflector v2 support temporarily (weav…
b38c75b 
…erryan)

This PR was squashed before being merged into the 1.0-dev branch.

Discussion
----------

Dropping doctrine/inflector v2 support temporarily

This was causting doctrine/inflector v2 to be installed. Then,
if a user required another package that only worked with v1 - like
doctrine/orm, that package would fail to install because the user's
app is locked on v2.

Dropping support for v2 isn't really a fix, but will make it more
compatible with the ecosystem at the current time. Later, especially
when doctrine/orm allows 2, we can again.

See also composer/composer#8910

Note: I kept the v2 compatible code in the bundle for now.

Fixes #609

Commits
-------

e75a956 Dropping doctrine/inflector v2 support temporarily
@Seldaek
Copy link
Member

Seldaek commented May 19, 2020

The error reporting in v2 handles this better to suggest you a way out. I tweaked things so it covers more cases. So now you'd see this in your case:

$ composer require acme/bar
Using version ^1.0 for acme/bar
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires acme/bar ^1.0 -> satisfiable by acme/bar[1.0.0].
    - acme/bar 1.0.0 requires transitive/dependency ^1.0 -> found transitive/dependency[1.0.0]
but the package is fixed to 2.0.0 (lock file version) by a partial update and that version does
not match. Make sure you list it as an argument for the update command.

Use the option --with-all-dependencies to allow upgrades, downgrades and removals for
packages currently locked to specific versions.

I think/hope that's clear enough :)

@Seldaek Seldaek added this to the 2.0-core milestone May 19, 2020
@naderman
Copy link
Member

@Seldaek In that error message I find it confusing that it says "is fixed to 2.0.0 (lock file version) by a partial update", what partial update? Should we change the message if it's the require command?

We could generally improve this a little bit by changing the require output to:

$ composer require acme/bar
Adding acme/bar with constraint ^1.0 to composer.json
./composer.json has been updated [can this be removed too?]

Running composer update acme/bar:

Loading composer repositories ....

@Seldaek
Copy link
Member

Seldaek commented May 19, 2020

I like the last suggestion, because special casing the error message based on it being a require/remove command sounds horrible :) I'll try and implement that.

Seldaek added a commit that referenced this issue May 19, 2020
@Seldaek
Add output to remove/require command to highlight the fact it is runn…
1d2df5e 
…ing an update command, refs #8910
@weaverryan
Copy link
Contributor Author

This looks much better - thank you!

@stof
Copy link
Contributor

stof commented May 19, 2020

@Seldaek should composer install without a lock file also include some output saying it will actually run an update ?

@naderman
Copy link
Member

@stof Doesn't it already in v2?

@Seldaek
Copy link
Member

Seldaek commented May 19, 2020

image

yes it does

@R-cloud-pixel R-cloud-pixel mentioned this issue May 19, 2020
Closed
@alcaeus alcaeus mentioned this issue May 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.0-core
Development

No branches or pull requests
4 participants
@weaverryan @naderman @Seldaek @stof