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

Composer updates packages on removing #7476

Closed
ossinkine opened this Issue Jul 18, 2018 · 50 comments

Comments

Projects
None yet
10 participants
@ossinkine
Copy link

ossinkine commented Jul 18, 2018

When I add package by call require command and then call remove I expect the lock file is not changed but this is not so.

The reproducer

Install packages

$ composer init
$ composer require symfony/flex symfony/maker-bundle

Downgrade package symfony/framework-bundle to 4.1.0 (imagine it has been installed when 4.1.0 was the latest version)

$ composer require symfony/framework-bundle:4.1.0
$ composer remove symfony/framework-bundle --no-update
$ composer update --lock

Install and remove api package

$ composer require api
$ composer remove api

symfony/framework-bundle package has been updated to 4.1.1

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 18, 2018

Your expectations are incorrect. The behavior is as it should be. If you have any further questions, feel free to add them, but this is not an issue.

@alcohol alcohol closed this Jul 18, 2018

@ossinkine

This comment has been minimized.

Copy link

ossinkine commented Jul 18, 2018

@alcohol Why is this behavior is correct? Why does the composer update packages on removing but not on installing? An implicit update can lead to unexpected behavior.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 18, 2018

I do not understand your questions or concerns. Maybe if you could explain what you actually see as a problem here, I could provide a more meaningful answer.

@ossinkine

This comment has been minimized.

Copy link

ossinkine commented Jul 18, 2018

Why does the remove command update packages? The update command should update packages and the remove command should remove packages.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 18, 2018

Because removing a requirement changes the dependency chain and thus the rules that the solver has to analyze. You are taking the verbs too literal. Dependency solving is not that black and white.

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Jul 18, 2018

removeis a sibling of require, which are both performing a partial update internally, to apply the new dependency rules. Comparing remove to install does not make sense, as they don't work the same at all. install does not make a full dependency resolution (it only allows using the locked versions)

@ossinkine

This comment has been minimized.

Copy link

ossinkine commented Jul 18, 2018

remove is a sibling of require, which are both performing a partial update internally

@stof Why require does not upgrade the version in my case but remove does?

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 18, 2018

Can you rephrase your question so it makes more sense?

does not upgrade the version

Does not upgrade the version of what?
And what version does remove upgrade according to you?

Both those statements do not make much sense to me.

@ossinkine

This comment has been minimized.

Copy link

ossinkine commented Jul 18, 2018

I have installed symfony/framework-bundle of 4.1.0 version.
Then I have installed api package, version of symfony/framework-bundle is still 4.1.0.
Then I have removed api package, version of symfony/framework-bundle is updated to 4.1.1.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 18, 2018

When you run require api, it sees there already is a version of symfony/framework-bundle installed (because you never cleaned it up due to passing --no-update to remove symfony/framework-bundle) which satisfies the constraint that api has for framework-bundle. It does not get updated at this point because that would require passing --update-with-dependencies to require api. When calling remove api, it does get updated because --update-with-dependencies is the default for remove.

Honestly, the workflow described here does not make sense and has no practical use. So while this discussion might be informative, it is somewhat pointless. But I do hope you understand now why it behaves this way.

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Jul 18, 2018

One thing we might want to change is allowing remove to remove unneeded dependencies by default, but not to update other dependency packages. But that would require much more work and so might not be worth it.

@ossinkine

This comment has been minimized.

Copy link

ossinkine commented Jul 18, 2018

@alcohol I've faced with this case on my work, so I don't agree that it does not make sense. Moreover this broke my code.

On my project I've installed api-platform/api-pack and all was ok. Then I decided that this package is redundant and I've removed it and installed api-platform/core.
Removing api-platform/api-pack package leads to symfony/dependency-injection has been updated to 4.1.1, but it is incompatible with symfony/http-kernel which still of 4.1.0 version (it has not been updated). Yes, this is Symfony component versioning issue but I lost a lot of time while figuring out the issue. I could not imagine that removing has updated my packages.

Well, I understood how the composer works, but I didn't understand why it works that way. Why remove is called with --update-with-dependencies by default? What command should I call to remove useless packages but don't update useful ones?

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jul 19, 2018

You are confusing multiple things.

What command should I call to remove useless packages but don't update useful ones?

There is nothing that objectively determines a package to be useful or useless. Composer only knows about versions and constraints.

The fact that you got stuck in a bad situation because a package did not have its constraints in order is not a good enough reason to revamp how Composer works.

I understand that it might be frustrating, but I think you can also understand that this does not happen often. And while it is okay to vent your frustrations, there comes a point where you should just accept this and move on.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Dec 17, 2018

There is nothing that objectively determines a package to be useful or useless. Composer only knows about versions and constraints.

In this case, there really is something that determines this. If you say 'composer remove foo/bar', then that is the developer explicitly telling Composer that foo/bar is useless, and everything else is useful.

The question still stands:

What command should I call to remove useless packages but don't update useful ones?

If I use the --no-update option, then Composer only edits my composer.json file, and doesn't change any package files. So that's not the desired effect.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

The bug seems to be caused by a change in packages that specify a version constraint.

For example, this is my composer why output for drupal/search_api, which was at 1.8.0:

$ composer why -t drupal/search_api
drupal/search_api 1.8.0 Provides a generic framework for modules offering search capabilities.
├──drupal/lightning_core 2.7.0 (requires drupal/search_api ^1.0)
│  └──skunkworks/endeavor-factory dev-c723a51a655052753ddcb729f70f14299792cf7a (requires drupal/lightning_core ^2.7)
└──drupal/search_api_sort_priority 1.8.0 (requires drupal/search_api *)
   └──skunkworks/endeavor-factory dev-c723a51a655052753ddcb729f70f14299792cf7a (requires drupal/search_api_sort_priority ^1.7)

When I removed drupal/lightning_core, drupal/search_api was updated to 1.11.0, and the composer why output is now:

drupal/search_api 1.11.0 Provides a generic framework for modules offering search capabilities.
└──drupal/search_api_sort_priority 1.8.0 (requires drupal/search_api *)
   └──skunkworks/endeavor-factory dev-sandbox-1452-removals (requires drupal/search_api_sort_priority ^1.7)

This is a bug, because it's going beyond what is requested in the remove command, and makes it harder to cleanly test the effects of removing packages because other things are changed as well.

Please could this be reopened.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

The question still stands:

What command should I call to remove useless packages but don't update useful ones?

If I use the --no-update option, then Composer only edits my composer.json file, and doesn't change any package files. So that's not the desired effect.

Assuming that foo/bar is the useless package:

composer remove foo/bar --no-update-with-dependencies
@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

composer remove foo/bar --no-update-with-dependencies only removes foo/bar, it doesn't remove any dependencies of foo/bar that are not required anywhere else.

So it's not actually utilising Composer's abilities as a package manager! It'd have to go through foo/bar's composer.json myself (which has just been deleted, so I'd first have to find a copy somewhere else!) and try to remove the packages listed there, one by one.

So that's not a fix to the problem here.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

Then run

composer remove foo/bar
@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

composer remove foo/bar will occasionally have the undesirable side-effect of updating unrelated packages, which is the subject of this issue!

See my previous comment #7476 (comment) for an example.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

Your specific use-case is not supported.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

Your specific use-case is not supported.

I don't understand what you mean by that.

Updating an unrelated package means that what should be a simple operation now requires testing before it can be deployed, to check the effects of the package update. It's simply not what Composer should be doing here.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

It means that what you want is not supported, I cannot phrase it more simpler than that.

Imagine you call composer remove foo/bar.

All remove does is modify your composer.json to no longer explicitly require the given argument. It then functions similar to update with the given argument and with --with-dependencies depending on whether or not you called remove with --no-update-with-dependencies or not. By default this allows transitive requirements to be updated. So after update has resolved the new dependency tree, it will look at all packages that foo/bar required and either remove them if they are no longer required or update them if they are still required by other packages.

It's simply not what Composer should be doing here.

That is your opinion, and you are entitled to your own opinion. But having an opinion does not change the way things work right now.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

All remove does is modify your composer.json to no longer explicitly require the given argument. It then functions similar to update

That is your opinion, and you are entitled to your own opinion. But having an opinion does not change the way things work right now.

I understand how this particular command has been written in Composer, and why it's doing what it's doing.

But could you explain how, supposing you're a developer managing a project's codebase, if you want to remove one package and its dependencies, it's not a problem for another package to get a version bump at the same time? Are you as a developer OK with just deploying that code immediately? Do you not need to check for any bugs, regressions, other side effects that any update brings? Should changes to your codebase not be as simple as possible?

You seem to be approaching this from the point of view of 'this is how Composer is coded, so this is how the command works', but that's not considering the real-world use cases.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

You are too fixated on the "remove" verb. It is more like you are telling Composer that you no longer require this dependency and to re-evaluate the dependency tree.

I personally have never had a real-world use case where this was an issue, nor have I ever worked at a client where this was an issue. Updates are almost always applied on a daily or weekly schedule. Removals happen incredibly infrequently. So this particular scenario you describe either never happened or it was simply not considered an issue.

But my personal experience and use-cases are irrelevant really. We simply cannot cater to everyone's individual needs. It would make Composer completely unmaintainable.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

We simply cannot cater to everyone's individual needs. It would make Composer completely unmaintainable.

When I removed drupal/lightning_core, drupal/search_api was updated to 1.11.0

That's a use case coming from the Drupal ecosystem, and I don't think any sane and competent Drupal developer would want to update one Drupal module at the same time as removing another. So I don't think it's an individual need.

In this particular example, a module such as search_api is large and complex and affects critical parts of the site. Updating even a patch version is not a task to be taken lightly -- as a Drupal developer, I would expect a process involving developers, managers, testers, and clients.

You are too fixated on the "remove" verb. It is more like you are telling Composer that you no longer require this dependency and to re-evaluate the dependency tree.

Maybe the command has the wrong name then?

That doesn't change the point that re-evaluating the dependency tree is harmful when all that is required is the removal of one package.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

But you can remove only this one package if that is what you want by telling Composer to not update anything else, not even the transitive requirements. Yet then you complain that other "useless" packages are left installed. So either way, we cannot please you. I have nothing further to contribute or add to this issue at this point. I'm sorry that you are unhappy with the way things work, but unless you take the time to come up with an elegant solution and submit a PR, nothing will change regardless of how much you complain.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

So either way, we cannot please you

Yes, you really can!

  • remove dependencies
  • don't UPDATE anything!

nothing will change regardless of how much you complain.

The least you could do is to reopen this and class it as a bug.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 9, 2019

BTW, for anyone else who has this problem, the workaround is:

  1. do 'composer remove foo/bar'
  2. commit the result
  3. check out the previous version of composer.lock
  4. use your preferred git GUI to commit just the changes to composer.lock that updated packages, so the diff of composer.lock from before you started is JUST the package removals
  5. do 'composer update --lock', which will downgrade the updated packages.
@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 9, 2019

It's a bug in your opinion. What you want is a new feature in my opinion. And I would prefer if you opened a new PR that clearly outlines the desired functionality.

@damienmckenna

This comment has been minimized.

Copy link

damienmckenna commented Jan 9, 2019

+1 for removing something not forcing everything to be updated.

There is zero reason to re-download and re-process upstream files when removing a local file.

The current behavior is absurd and illogical.

@geerlingguy

This comment has been minimized.

Copy link

geerlingguy commented Jan 10, 2019

Just to add a little color to the Drupal-related aspect of this discussion: this is one highly valid reason to define very rigid version constraints on the Drupal modules themselves in your composer.json file. For sites where it matters (most sites), I set the specific x.y.z version in the requirement, and don’t allow Composer to automatically update any modules.

Outside the Drupal sphere, I have to agree with @alcohol that I’ve never seen this be an issue, since most other parts of the ecosystem adhere to semantic versioning in a meaningful way. Many Drupal contrib module authors do not (especially since right now the Drupal.org Packagist integration for Drupal modules is somewhat of a hack and contrib tooling on Drupal.org does not have any kind of semantic meaning), and the really rears its ugly head when you do upgrade a module from something like 1.5.0 to 1.8.0, that sort of thing.

It would be nice, however, to be able to remove a dependency and not have other deps be touched at all... but it sounds like that feature is not going to be supported, according to the comments from @alcohol :(

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 10, 2019

but it sounds like that feature is not going to be supported, according to the comments from @alcohol :(

I think I made it clear that if someone is willing to invest the time to implement this in an elegant fashion, we will happily accept pull requests. This is an open source project after all. But as it stands right now, I do not believe any of the core maintainers have the spare time available to spend on this, considering the use-case is quite specific and rarely necessary. The investment required simply does not justify the gain, when there are other things that have a much higher priority for us.

@hansfn

This comment has been minimized.

Copy link

hansfn commented Jan 10, 2019

Reading the whole thread, I really feel like going meta and discussing how we handle real needs from users, but I'll leave that for another time.

<rant>
I look upon Composer as a package manager even though the first line of the documentation says

Composer is a tool for dependency management in PHP.

In my experience (Linux distro) package manager, like apt-get, emerge, remove a package if requested. Afterwards (or at the same time), they run a package dependency check and remove packages that are no longer needed - explicitly required or needed by explicitly required packages. (Check out apt-get autoremove.) No updating going on.

Why isn't this considered useful in the Composer world? And yes, I think the verb "remove" is wrongly used here.
</rant>

@weseze

This comment has been minimized.

Copy link

weseze commented Jan 10, 2019

I never noticed this issue (it really is an edge cased), but it really does feel like a bug if composer updates packages on a "remove" command.

I fail to see ANY point in that other then making our lives miserable :)

@alcohol: I don't understand why you would close an issue because you do not think any of the core maintainers have time to fix this? It is still a valid issue. Someone might have stepped up and fixed it here, but now it is closed...

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 10, 2019

I never noticed this issue (it really is an edge cased)

In the last few weeks, I've removed about 5-6 packages from my codebase, which is for a set of 8 Drupal websites. This issue has come up every time. One package that kept getting updated was composer/installers, because lots of Drupal modules apparently request it, but there were others too.

From what I can tell, it happens if you have these conditions:

  1. You have this sort of dependency tree before removal:
my/root
- to/remove
-- common/dep
- not/removed
-- common/dep
  1. a newer version of common/dep exists. That is:
  • if you did 'composer update common/dep' before the removal, it would update
  • if you have a separate codebase with to/remove not present, it would update

In other words, the presence of to/remove does not need to be impeding the potential update of common/dep. It's just that the tree of dependencies for common/dep has changed, so it's recalculated, and as a newer version happens to be available, it's installed.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 10, 2019

Someone might have stepped up and fixed it here, but now it is closed...

This being closed doesn't block anyone from doing that. I still think it would better to create a feature request that clearly outlines the use-case and expectations. This thread is way too opinionated/heated and distracting to keep open.

@hansfn

This comment has been minimized.

Copy link

hansfn commented Jan 10, 2019

Sure, I think both the OP and Joachim are willing to write a clear feature request, but that is wasted if the maintainers just close it as out of scope and irrelevant ;-)

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 10, 2019

Why would we close it when I just said a feature request is fine to submit and that we will happily accept pull requests?

@JorisVanEijden

This comment has been minimized.

Copy link

JorisVanEijden commented Jan 11, 2019

I understand people being uncomfortable with the seeming inconsistency as I understand it:

  • composer require foo/bar does not install available updates for existing packages required by foo/bar
  • composer update foo/bar does not install available updates for existing packages required by foo/bar
  • composer remove foo/bar does install available updates for existing packages required by foo/bar

Is this correct?

And I have a question for those who want this feature:

  • I have package A version 1.0 installed
  • I add package B which requires A 1.1, composer updates A
  • I remove package B
    Should composer now downgrade A to 1.0 again? How?
@hansfn

This comment has been minimized.

Copy link

hansfn commented Jan 11, 2019

Should composer now downgrade A to 1.0 again? How?

No, definitely not. The whole point of this issue is that a removal shouldn't change anything except remove the requested package and packages no longer needed. (We know that currently our software works with version 1.1 of A. We might have changed things so it no longer works with 1.0 of A.)

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 11, 2019

Semantic versioning solves almost all of these issues. If developers/maintainers of packages opt to not use semantic versioning, then I would recommend you add such transitive dependencies as root requirements to your composer.json to avoid these issues (and submit an issue to their issue tracker to encourage them to adopt best practices).

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 11, 2019

Semantic versioning solves almost all of these issues.

Only in the ideal world. Any new version can introduce bugs and regressions.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 11, 2019

I'm an optimist, not a pessimist :-)

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 11, 2019

That's nice, but when you're managing a codebase that runs one (or several) high profile sites, you can't be. You have to think worst case scenario. Your changes and deployments should be as small as possible.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 11, 2019

then I would recommend you add such transitive dependencies as root requirements to your composer.json

@JorisVanEijden

This comment has been minimized.

Copy link

JorisVanEijden commented Jan 11, 2019

No, definitely not. The whole point of this issue is that a removal shouldn't change anything except remove the requested package and packages no longer needed.

I was asking because some people apparently expect composer.lock to remain unchanged after adding a package and then immediately removing it again.

@joachim-n

This comment has been minimized.

Copy link

joachim-n commented Jan 11, 2019

then I would recommend you add such transitive dependencies as root requirements to your composer.json

If you are recommending this as a workaround for this bug, then I'd have to require all dependencies in the root, AND pin their versions.

At that point, Composer just becomes a glorified unzipper & 'rm -rf' script. What's the point of it?

@JorisVanEijden

This comment has been minimized.

Copy link

JorisVanEijden commented Jan 11, 2019

I'd have to require all dependencies in the root, AND pin their versions.

In the case of Drupal modules, yes you do. Just like you did with drush make files.

@alcohol

This comment has been minimized.

Copy link
Member

alcohol commented Jan 11, 2019

I'm just giving you options you can use RIGHT NOW rather than waiting around idly hoping for some saviour to implement this feature for you.

@alexdoronin

This comment has been minimized.

Copy link

alexdoronin commented Jan 17, 2019

I agree with @joachim-n , removing modules and auto updating dependencies at the same time is a constant headache while working with Drupal.

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