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

Allow update --with for non-root packages #10436

Closed
GPHemsley-RELX opened this issue Jan 5, 2022 · 8 comments · Fixed by #10773
Closed

Allow update --with for non-root packages #10436

GPHemsley-RELX opened this issue Jan 5, 2022 · 8 comments · Fixed by #10773
Labels
Milestone

Comments

@GPHemsley-RELX
Copy link

#8860 added the --with parameter to update to run an update with a specific version constraint on a package.

In #8756, @Seldaek wrote without elaborating:

This should be only allowed for packages required by the root package for now, we can re-evaluate later if needed.

Following the resolution of the issue, @froemken (not OP) commented that the change did not satisfy his needs because of its restricted application:
#8756 (comment)

It seems that @louisitvn was also looking for this functionality for non-root packages:
#8756 (comment)

I, too, am looking for this functionality.

What was the motivation for restricting update --with to root packages? Is there a technical reason why it can't apply to any package? If not, can the restriction be removed?

@Seldaek
Copy link
Member

Seldaek commented Jan 6, 2022

The rationale is two-fold for disallowing it, one is that it currently modifies existing links on the root package, it doesn't add anything, but that's a weak reason I think.

The other more worrying reason is it lets you do really strange things if we allow adding requires, as you could suddenly end up with a lock file containing packages that were not required by anything, they were just added by some --with rule on update, and the next update will remove those dangling packages. I don't know how bad this is, but I think it's the main reason I figured we'd rather not do this initially. One could say it's just a foot-gun, which can be used properly as well..

@Seldaek Seldaek added this to the Nice To Have milestone Jan 6, 2022
@stof
Copy link
Contributor

stof commented Jan 6, 2022

@Seldaek maybe those temporary restrictions should be implemented by filtering the pool (before optimizing it) rather than by adding links. This way, those restrictions cannot cause extra packages to be installed if they are not required somewhere.

@Seldaek
Copy link
Member

Seldaek commented Jan 6, 2022

Yeah that's worth investigating, good call

@GPHemsley-RELX
Copy link
Author

@Seldaek Yeah, I don't think the request is to allow arbitrary packages to be introduced with --with, just that it be allowed to affect indirect dependencies. The restriction that the specifications be a subset of existing dependencies would remain in place.

@Seldaek
Copy link
Member

Seldaek commented Jan 21, 2022

The other reason why we restricted came back to me as it is in the docs:

The custom constraint has to be a subset of the existing constraint you have, and this feature is only available for your root package dependencies.

Ensuring it's a subset of all the requirements isn't so easy as it is with the root, but doing so at the pool level might still be possible..

@stof
Copy link
Contributor

stof commented Jan 21, 2022

Well, once we apply --with as a pool filter rather than a constraint replacement, this subset rule becomes void, as the actual constraint will still apply (among the packages matching the --with)

@OrkhanAlikhanov
Copy link

I wanted to downgrade psr/log for a laravel project but couldn't not via a simple command. Ended up updating composer.lock manually.

I needed to downgrade because running composer require rollbar/rollbar-laravel complained about psr/log version being high.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - rollbar/rollbar-laravel[v7.1.0-RC1, ..., v7.1.0] require rollbar/rollbar ^2 -> satisfiable by rollbar/rollbar[v2.0.0, v2.1.0].
    - rollbar/rollbar[v2.0.0, ..., v2.1.0] require psr/log ^1 -> found psr/log[1.0.0, ..., 1.1.4] 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.
    - Root composer.json requires rollbar/rollbar-laravel ^7.1 -> satisfiable by rollbar/rollbar-laravel[v7.1.0-RC1, v7.1.0].

Then I run composer why psr/log and saw that every installed package that requires psr/log supports v2 except the new rollbar/rollbar package that I was going to install.

composer/composer        2.2.6    requires  psr/log (^1.0 || ^2.0)            
composer/xdebug-handler  2.0.4    requires  psr/log (^1 || ^2 || ^3)          
doctrine/dbal            3.3.2    requires  psr/log (^1|^2|^3)                
filp/whoops              2.14.5   requires  psr/log (^1.0.1 || ^2.0 || ^3.0)  
laravel/framework        v8.82.0  requires  psr/log (^1.0|^2.0)               
monolog/monolog          2.3.5    requires  psr/log (^1.0.1 || ^2.0 || ^3.0)  
spatie/image-optimizer   1.6.2    requires  psr/log (^1.0 | ^2.0 | ^3.0)      
symfony/error-handler    v5.4.3   requires  psr/log (^1|^2|^3)                
symfony/http-kernel      v5.4.4   requires  psr/log (^1|^2)                   

So it makes sense to allow updating non-root packages in this case. Or maybe it should've asked me to downgrade psr/log to v1?

@Seldaek
Copy link
Member

Seldaek commented May 12, 2022

Ok I ran into this myself one too many time and decided to fix it :)

See #10773

Seldaek added a commit to Seldaek/composer that referenced this issue May 12, 2022
Seldaek added a commit to Seldaek/composer that referenced this issue May 12, 2022
Seldaek added a commit to Seldaek/composer that referenced this issue May 13, 2022
emahorvat52 pushed a commit to emahorvat52/composer that referenced this issue Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants