Replaced package is not installed if its replacer is updated to a version that no longer replaces

[driskell]

During discussion in #9619 @naderman discovered a failing test. This PR adds the test as it was proposed along with a possible fix for it. In summary, if a package (A) that replaces another (B) is updated to a new version that no longer replaces package B, then package B does not get unlocked and added to the pool, therefore the update would presumedly fail unless the package B was also added to the update allow list.

I believe from the discussion in #9619 the replaced package should get loaded and added automatically.

[Toflar]

@Seldaek I am late to the party - just stumbled across this PR and wanted to see if this is still relevant against the current main.
What I did is I took @driskell's test and ran it against the current main branch. It fails so it made me think we might still need this PR.
The test is expecting this:

--EXPECT--
[
    "root/no-update-1.0.0.0 (locked)",
    "root/dep-1.2.0.0",
    "replaced/pkg-1.0.0.0",
    "replacer/pkg-1.1.0.0"
]

--EXPECT-OPTIMIZED--
[
    "root/no-update-1.0.0.0 (locked)",
    "root/dep-1.2.0.0",
    "replaced/pkg-1.0.0.0",
    "replacer/pkg-1.1.0.0"
]

While the current main is expecting this:

--EXPECT--
[
    "root/no-update-1.0.0.0 (locked)",
    "root/dep-1.2.0.0",
    "replacer/pkg-1.1.0.0"
]

--EXPECT-OPTIMIZED--
[
    "root/no-update-1.0.0.0 (locked)",
    "root/dep-1.2.0.0",
    "replacer/pkg-1.1.0.0"
]

As you can see, the difference is replaced/pkg-1.0.0.0.
And I do think the changes in this PR are correct. I ran the current $pool against the Solver. If I do this, I get:

  Problem 1
    - root/no-update is locked to version 1.0.0 and an update of this package was not requested.
    - root/no-update 1.0.0 requires replaced/pkg 1.0.0 -> found replaced/pkg[1.0.0] but these were not loaded, likely because it conflicts with another require.

Which seems wrong and the changes in this PR do fix this issue and make the Solver find a valid solution. The resulting LockTransaction then is

Upgrading <info>replacer/pkg</info> (<comment>1.0.0</comment> => <comment>1.1.0</comment>)
Upgrading <info>root/dep</info> (<comment>1.1.0</comment> => <comment>1.2.0</comment>)
Installing <info>replaced/pkg</info> (<comment>1.0.0</comment>)

This seems to be the desired solution. Not sure about the Installing <info>replaced/pkg</info> (<comment>1.0.0</comment>) part as having this in the LockTransaction seems unnessecary but this might be the case in other replaced packages too and probably not related to this PR.

[driskell]

@Toflar

The reason replaced/pkg-1.0.0.0 should appear as something to install (which doesn't on main) is because although replacer/pkg-1.0.0.0 replaces it initially, the upgrade to replacer/pkg-1.1.0.0 no longer replaces it (so as part of the upgrade to replacer/pkg-1.1.0.0 the originally replaced package should now be installed).

That's why you get the Installing <info>replaced/pkg</info> - because as part of upgrading replacer/pkg it is now a needed package (replacer isn't replacing it anymore).

Apologies if the package names in the test are a little unclear!

[driskell]

To clarify - the replacer/pkg package has two version - 1.0.0.0 is a replacer of replaced - 1.1.0.0 is in fact not a replacer at all and doesn't do any replacing of anything, so replaced needs to be installed. And if you use main to run a fresh install of this package it would indeed resolve to install replaced. It's just through upgrade it doesn't.

[naderman]

Thanks @Toflar for checking this - I think to merge this we should have an additional test on the installer fixture level to highlight the actual bug being fixed by this change?

[driskell]

Thanks @Toflar for checking this - I think to merge this we should have an additional test on the installer fixture level to highlight the actual bug being fixed by this change?

Happy to help - not sure what test is required though - if you could elaborate I can take a look. I had thought as this was an issue with the package list passed to the solver the current test was enough.

[Toflar]

Basically you can promote your test to a level even higher (aka full e2e test) by adding another test to the functional tests of the Installer (https://github.com/composer/composer/tree/main/tests/Composer/Test/Fixtures/installer). Without your changes it should end up displaying the problem I mentioned in #10410 (review) and with your fix, this will change 😊
It basically would not only test the PoolBuilder and PoolOptimizer but also integrate with the Solver.

[Toflar]

I've added the integration test in #11449.