Skip to content
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

Allow to prioritize some recipes #957

Open
jakubtobiasz opened this issue Nov 7, 2022 · 4 comments
Open

Allow to prioritize some recipes #957

jakubtobiasz opened this issue Nov 7, 2022 · 4 comments

Comments

@jakubtobiasz
Copy link

Hi 👋🏼!

I am coming here to gather some feedback on my idea before starting working on it.

Background

I wanted to create a POC of https://github.com/symfony/skeleton made for Sylius. I created a simple recipe for sylius/core-bundle, then an example skeleton repo and I have found out my recipes does not work as another recipe already write files with the same name. Then, I noticed symfony/framework-bundle is always put as the first recipe to be executed, and this is a thing I wish to be able to configure.

Goal

Somehow allow myself to make (in this POC case) sylius/core-bundle as a first recipe to be executed. Of course, I can fork symfony/flex, but it would be perfect to avoid this way.

Idea

The idea is simple, we allow configuring such list for example in this way:

{
    ...
    "extra": {
        "flex": {
            "prioritized-recipes": [
                "sylius/core-bundle",
                "another/sylius-package",
                ...
            ]
        }
    }
    ...
}

In Flex we could implement this +/- this way:

        // symfony/framework-bundle recipe should always be applied first after the metapackages
        // however, we allow to override it with a list of prioritized recipes
        $recipes = $this->getPrioritizedRecipes();
        $recipes = array_merge($recipes, [
            'symfony/framework-bundle' => null,
        ]);
        $packRecipes = [];
        $metaRecipes = [];

instead current

        // symfony/framework-bundle recipe should always be applied first after the metapackages
        $recipes = [
            'symfony/framework-bundle' => null,
        ];
        $packRecipes = [];
        $metaRecipes = [];

Why?

  • In some projects, we may want to load our recipes before the Symfony's ones
  • In frameworks based on Symfony (like Sylius) we need to set up the whole project in our own way, so framework-bundle as a first recipe to be executed makes it unable for us

Other options

I have not checked it yet, but I believe we can achieve the similar feature using Composer's Event Dispatcher. But first, I would like to hear if such a feature is welcomed. Or maybe you have a better idea how to solve this. I am open to provide such feature right after we agree on some solution.

@nicolas-grekas
Copy link
Member

Did you consider running your own recipe-repo? Because this should be a better way to handle priorities. Would that work for you?

@jakubtobiasz
Copy link
Author

@nicolas-grekas, thanks for your reply 🙏🏻. We have our own recipe-repo, but we have also added flex://defaults to provide all existing recipes. Own recipe-repo also doesn't solve priorities problem as some collisions might occur, and we can't force core-bundle's recipes to be executed as first.

@jakubtobiasz
Copy link
Author

@nicolas-grekas I spend some time thinking about this problem and maybe adding a configuration might not be the best from DX perspective, but what about Composer Events 🤔.

I have created a PoC looking +/- like that:

        $event = new PreRecipesSet($flexRecipe); // $flexRecipe might not be required here as we set $recipes below, but I'm lefting it as is for PoC needs
        $eventDispatcher->dispatch(PreRecipesSet::class, $event);
        
        $recipes = [
            ...$event->recipes(), // we set all recipes set in the PreRecipesEvent event
            'symfony/framework-bundle' => null,
        ];

I know from Symfony perspective it isn't a crucial feature. It might have a low priority, but for frameworks like Sylius, Sulu, and other it might be great to have a better control over flex, without dropping support for already existing recipes.

Of course, this needs a lot of lifting, but I need your “approve” that you want such feature in flex 🙏🏻.

@alexander-schranz
Copy link
Contributor

We at @sulu are running into the same problems as Sylius. That other recipes already creating files which are missing some required changes or even incompatible with Sulu itself.

The incompatible part could be fixed via blacklist of some ignore or conflict definitions, from my point of recipes of dependencies of dependencies should not be installed aslong as there is no definition require other recipe. I even raisened another issue about that here: #940. In our case this is as example the doctrine_phpcr package: https://github.com/sulu/skeleton/blob/2.5/config/packages/doctrine_phpcr.yaml. We are writing some empty files with comments in it to force symfony/flex not writing that files which is really odd as that files should just not exist.

The other thing which I think @jakubtobiasz is trying to solve here is some part are missing things. This are 2 files which effects Sylius and Sulu the first one is the Kernel.php and the second one the security.yml:

We both has our own Kernel class and we both I think have our own security configuration.

If I understand @nicolas-grekas suggestion it would be possible that we provide symfony/framework-bundle and symfony/security-bundle in our own recipes. But I think that would not be good for the ecosystem as somebody would then need to keep them up2date with the original recipes changes. And we just want to override a single file not the whole recipe.

I'm not really sure what the best solution would be here, maybe recipes can provide .patch files for other executed recipes and flex will first load the Original Kernel.php, apply the patch so we would need a require part which recipes require another recipe to be executed first. How to handle the recipes:update here would be that flex first need to create the diff between previous Kernel.php + patch and then the new Kernel.php + patch and then can apply that new patch. But think that would be very hard to maintain as the patch would always be need to be compatible and it would add complexity to the flex codebase.

Another possible solution if we say we don't want to include patches is that every recipe can define a priority. So Sulu and Sylius recipe which writing the Kernel.php or security.yaml can set an higher priority then the original framework-bundle which can already have a high priority to be executed first. This way there is no hard coded what is coming first instead an priority in the recipes json response defines which recipes should be executed first. Sure we would need to load first both recipes endpoints json, not sure how the logic there is currently but think it should be possible.

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

No branches or pull requests

3 participants