Failing test to outline problem in #9393

[Toflar]

Okay, managed to debug exactly on what's happening after way too many hours 😅 Here's the exact reason on what's happening in the case described by @Seldaek in #9393 (comment):

• drupal/webform is locked at 6.1.8.0 but allowed to be updated/removed because of -W
• PoolOptimizer::optimizeImpossiblePackagesAway() removes drupal/core 10.2.0.0 because drupal/webform is locked at 6.1.8.0 and requires drupal/core in ^9.3.

The problem is that drupal/webform itself is going to be removed during the update process so actually, its requirements should not be considered and thus the drupal/core 10.2.0.0 should not get removed.

This exact case was actually sort of considered in the $isUnusedPackage here:

composer/src/Composer/DependencyResolver/PoolOptimizer.php

Line 404 in 8e62977

if (isset($this->requireConstraintsPerPackage[$packageName])) {

However, this doesn't work properly because $this->requireConstraintsPerPackage[$packageName]) actually does contain drupal/webform because this array is built before any optimizations are even applied. It is required by the the locked package drupal/spamaway 2.0.0.0 which would also get removed.

So the bug happens because the $this->requireConstraintsPerPackage (and probably other things) are used as if this array were to contain a definitive list of what is going to be required. Something that we never know at the stage of the PoolOptimizer as this is what the Solver will do - it's a typical problem of recursive optimizations.

So long story short: The current optimization step for locked packages is flawed because it is built on top of false assumptions.

[Toflar]

This line here is the difference. We must expect drupal/core-10.2.0.0 to be present but it's removed.

[naderman]

Wonder if instead of changing much here, we can just trim anything from the lockfile - before starting optimizing - that isn't required by anything still required on a root level in composer.json? Would that just make the current solution work?

[Toflar]

I just think the general idea of the PoolOptimizer::optimizeImpossiblePackagesAway() step cannot work reliably, can it?

The PoolBuilder is designed to load all packages into the Pool that are referenced anywhere in the dependency tree. Meaning, all of those packages could potentially end up in the final solution. Finding a/the best solution is the task of the Solver.

The first step of the PoolOptimizer (optimizeByIdenticalDependencies()) optimizes the Pool by removing packages that share the same dependency definition and are never referenced anywhere in the Pool. However - and this is key - it does not make assumptions on which of the packages in the Pool could or could not be picked. It leaves this task to the Solver.

The PoolOptimizer::optimizeImpossiblePackagesAway() step, however, does make assumptions. It checks if a certain package is even by any package in

composer/src/Composer/DependencyResolver/PoolOptimizer.php

Line 404 in 8e62977

if (isset($this->requireConstraintsPerPackage[$packageName])) {

. This should always be true, right? Why would it even land in the Pool before optimizing if it's not required anywhere? But maybe that's exactly what you mean by trim anything from the lockfile - before starting optimizing?

[naderman]

So in general the optimizer does have access to the request so that it may use it as input to draw conclusions, if it can be sure something won't possibly get picked by the solver later, it can already remove it. In a sense that's what the part you described does, too. It's not that it leaves all packages in the pool, it draws conclusions on what won't get picked anyway and removes it.

Apart from that, yes, I think due to the lock file in a partial update case we get packages in the pool that potentially have no chance of getting installed anyway and could already get removed.

[Toflar]

Yeah what I was saying is that should they even end up being part of the pool in the first place. Aka should they not even get added in the PoolBuilder?

[naderman]

Yeah can maybe already try to consider that there instead of in the optimizer. In the end the optimizer is really just a step of pool building too, so it doesn't really matter, just depends what's easier/more understandable.