composer / satis

Simple static Composer repository generator - For a full private Composer repo use Private Packagist
MIT License
3.15k stars 519 forks source link

issues with `only-best-candidates` and `blacklist` options #587

Open tobybellwood opened 4 years ago

tobybellwood commented 4 years ago

only-best-candidates

When using only-best-candidates to reduce the number packages provided by the Satis, the resolver incorrectly resolves multiple versions of a package. My research indicates that this occurs when there are multiple packages that all require a common package, but with different version constraints, and often in contravention of a direct explicit requirement.

For example

$ composer why drupal/consumers
drupal/simple_oauth  3.14.0  requires  drupal/consumers (^1.2)  
govcms/govcms        1.1.0   requires  drupal/consumers (1.9) 

Even with only-best-candidates set, Satis will include both 1.9.0 and 1.10.0 as valid options, as it resolves ^1.2 separately to the 1.9 requirement. Ideally, if the explicit requirement 1.9 also satisfies the ^1.2 (which it does), no additional version should be downloaded.

Whilst we can then use the blacklist option to remove the version not required - we can run into issues if the package is a non-specified dependency.

I think #470 may go some way towards fixing this, but i could not get it to work, as there seems to be some issues with the code.

blacklist

In another of our cases the Satis identified the 2.0.0 release of consolidation/log as being suitable (and using only-best-candidates, excluding the other releases) - however, this version has a dependency on symfony/console: ^4|^5, which are not compatible with Drupal 8, so we should be using 1.1.1 instead.

$ composer why consolidation/log 
consolidation/robo  1.4.12  requires  consolidation/log (^1.1.1|^2)

Because only-best-candidates was only identifying a single suitable target for consolidation/log, once we blacklisted the v2.0.0 version, as it runs after the resolution process, Satis is left with no valid versions, and cannot resolve composer requests that rely on consolidation/log.

In order to resolve these issues, the way that only-best-candidates is resolving will need looking at, and potentially rewriting to better evaluate any existent hard constraints for package versions against any loose ones - where the hardest constraint satisfies the looser constraints, there should be no additional packages included. With drupal/core-recommended in play now, this is going to become more of an issue. There are multiple ways that people can write their composer constraints, but they should all be testable.

In addition, the presence of the blacklist should also be evaluated during the resolution as an additional constraint to deal with, rather than excluding versions after resolution.

Happy to explain further, or provide more examples if needed.

I've resolved both these issues temporarily by including additional pinned packages in require, and creating a more comprehensive blacklist, but it'd be good to have Satis do the heavy lifting.

Quix0r commented 4 years ago

Some versions of packages are broken. There should be an easy way to blacklist those versions from causing interference.

tobybellwood commented 4 years ago

yup - that's where we got to - as blacklisting after resolution removes the entire package - the blacklist should happen during resolution for the safe version to be included!

Quix0r commented 4 years ago

Idea: Instead of "level-2/dice": "^4" use this for blacklisting 4.0.2 and x.y.z:

"level-2/dice": {
    "matches": "^4",
    "blacklist": "4.0.2,x.y.z"
}
tobybellwood commented 4 years ago

the issue there is that it becomes a composer anti-pattern. The blacklist issue raises it's head worst with child dependencies (which aren't defined in the root composer.json), or cross-dependency mismatches (as outlined above)