Skip to content

Conversation

@weaverryan
Copy link
Contributor

Hi!

Flex recipes support a conflict key - e.g. https://github.com/symfony/recipes/blob/ee79aec15811a380b09c53d058972c74e77e25d9/doctrine/doctrine-bundle/2.4/manifest.json#L48-L50

In this case, we added that so that we could use when@dev in the recipe, but that requires symfony/framework-bundle 5.3 or greater.

Anyways, the current conflict behavior is overly simple: if a recipe conflicts with a package you have installed, it simply skips the recipe entirely. This, for example, makes the following fail right now:

symfony new 4.4_app --version=4.4
cd 4.4_app
composer require doctrine

If you try this, the doctrine/doctrine-bundle will be skipped entirely.

This PR makes that behavior smarter. For each recipe that conflicts with your app (e.g. doctrine/doctrine-bundle 2.4 recipe), it will try the next oldest recipe version (e.g. doctrine/doctrine-bundle 2.3) until it finds one that does not conflict. So, you will always get the latest recipe version without any conflicts. If no compatible recipes are found, none would be installed (but I don't believe we have this situation anywhere anyways).

Tested locally on an app also.

Cheers!

public function removeRecipeFromIndex(string $packageName, string $version)
{
unset($this->index[$packageName][$version]);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By far the cleanest and easiest way to implement this was to: (A) allow the Downloader::getRecipes() method to be called normally and then (in the case of a conflict) (B) remove that recipe version from Downloader (that's this method) and (C) call Downloader::getRecipes() again.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach has the drawback of making as many synchronous HTTP calls as there are conflicting recipes. For 4.4, this means maximum 6 extra HTTP calls right now. This number will grow as we add conflicts.

We could improve the code to make all those calls async. But a better approach if we care could be to add a recipe-conflicts entry to the index.json.

Yet, I think merging as is is fine. We can always improve later! PR welcome anyone :)

👍

@nicolas-grekas
Copy link
Member

Thank you @weaverryan.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants