search

silverstripe / cow

Release tool for Silverstripe CMS
BSD 3-Clause "New" or "Revised" License
6 stars 8 forks source link

Plans should mandate that recipes have version bumps OR allow recipes to have looser constraints #132

Open robbieaverill opened 5 years ago

robbieaverill commented 5 years ago

When you generate a plan then tag a release, strict constraints e.g. 4.3.0@stable are baked into each recipe in the recipe (e.g. cwp-recipe-cms, recipe-blog, etc). This means that we must always bump a version for recipe dependencies, regardless of whether they have any changes in them.

Cow should mandate that recipe versions be bumped in this case so we don't forget and publish an un-installable release.

A better option might be that we allow recipes to have configuration saying it should have a loose constraint, so we can keep the strict constraints for silverstripe/recipe-blog etc out and in CWP and core recipes where they belong. (Needs discussion further if we take this approach)

ScopeyNZ commented 5 years ago

A better option might be that we allow recipes to have configuration saying it should have a loose constraint, so we can keep the strict constraints for silverstripe/recipe-blog etc out and in CWP and core recipes where they belong. (Needs discussion further if we take this approach)

I think this would really benefit us. I'm going to break it down:

Recipes are just a list of composer constraints. Composer constraints serve two purposes. One, list dependencies of your project. Two, indicate compatibility with those dependencies.

In our recipes we list all the components of the recipes but we also usually have a compatibility constraint. Take blog for example:

        "silverstripe/recipe-cms": "^4.2",
        "silverstripe/blog": "3.2.x-dev",
        "silverstripe/widgets": "2.0.x-dev",
        "silverstripe/content-widget": "2.0.x-dev",
        "silverstripe/spamprotection": "3.0.x-dev",

In this case "silverstripe/recipe-cms": "^4.2" is a compatibility constraint and the rest of them are components of the recipe.

When we release tags of our recipes we pin our recipes to individual tags. This means that:

The proposal is that we create a way to indicate to cow what constraints are compatibility constraints and let it assume the rest are components. My initial idea would be to add to the .cow.json schema something like:

"compatibility-constraints": [
  "silverstripe/recipe-cms"
]

The advantage of this would be:

In the future we might be able to inspect the maximum dependency from components of the recipe to auto-complete compatibility constraints. So cow could look at the silverstripe/recipe-cms constraint for each of the components and choose the highest of them to set as the compatibility constraint on the recipe 🤔 .

chillu commented 5 years ago

We have two cases in recipes here:

Would your suggestion change anything for core recipes? Would that make releasing CWP and core easier or harder? How much change would be required in cow to achieve that? How many manual decision points would be added for a release manager?

ScopeyNZ commented 5 years ago

Would your suggestion change anything for core recipes?

The new configuration would be opt-in. Nothing would be different without it.

Would that make releasing CWP and core easier or harder?

I personally think it would make it easier. Certainly would result in less tags for every release.

How much change would be required in cow to achieve that?

Ball parking - probably not much? We run through the dependencies to "stabilise" and ignore some already depending on some condition:

https://github.com/silverstripe/cow/blob/b5b11311a2ab20c3cbee032c836385ef5b5440f8/src/Steps/Release/PublishRelease.php#L129

We can just add to that condition? The .cow.json configuration should be available already at that point.

How many manual decision points would be added for a release manager?

Considering we already go through and decide whether we release the components of the recipe we can just say "okay - this recipe has no updates to any of its components, we'll skip releasing the recipe too then". We can't do that right now.

[...] constrains silverstripe/graphql:3.0.1@stable. You get whatever the latest tag is in the current minor release line.

I assume you mean "when you release". 3.0.1@stable only refers to the 3.0.1 tag and nothing else. Using recipe-cms:4.3.1 and trying to install GraphQL 3.0.2 (if it exists) will give you a "Your requirements could not be resolved to an installable set of packages".

ScopeyNZ commented 5 years ago

@sminnee makes a good point that makes this potentially irrelevant. Why bother even having a dependency like recipe-cms in a recipe like recipe-blog. It's only there to indicate support, but that's already indicated by the components of the recipe (eg. silverstripe-blog requires silverstripe-cms).

robbieaverill commented 5 years ago

It makes it infinitely easier to see what versions of recipe-blog will work with core versions when you can glance at it and see the dependency at the top of composer requirements.

I think the reason that these recipes have strict constraints is because we allow people to create projects from them in the same way that they would with installer, recipe-core, recipe-cms, etc. I don't know whether this happens in practice or whether we just make our lives more difficult.

Cheddam commented 5 years ago

I think the reason that these recipes have strict constraints is because we allow people to create projects from them in the same way that they would with installer, recipe-core, recipe-cms, etc.

I don't think the changes being proposed here would have any impact on that use-case, unless someone is trying to install a very specific version of the core components via a non-core recipe - in which case they can instead use a core recipe for the project creation, and then add the non-core recipe as a dependency. I'd wager the number of developers this will impact is close to, if not, 0.

robbieaverill commented 5 years ago

@sminnee makes a good point that makes this potentially irrelevant. Why bother even having a dependency like recipe-cms in a recipe like recipe-blog. It's only there to indicate support, but that's already indicated by the components of the recipe (eg. silverstripe-blog requires silverstripe-cms).

We just encountered this with recipe-content-blocks requiring versioned-admin to indicate compatibility with it, when it's a dependency of dnadesign/silverstripe-elemental. Because it was in the recipe, it received a specific constraint (which was incorrect, because we missed it assuming it was a core module handled in silverstripe/recipe-cms).

We fixed this by removing versioned-admin from recipe-content-blocks explicitly. It would then be installed with the constraint that elemental has for it, which is ^1.1 or something similar.

We rely on the core silverstripe/recipe-cms etc recipes to define strict constraints on core modules.

Cheddam commented 3 years ago

The debate on whether recipes should pin their dependencies at the patch level continues in 2020. Key points from my experience:

Neither approach makes the multi-package approach easier to comprehend - we'd still need to jump through the same hoops to get accurate information about what version a bug is present in. At least right now a developer can point to a recipe release, and we know it coincides with a specific set of module releases.

Anyway - the original suggestion in this issue (adding logic to Cow to make sure we release recipes correctly) is still worthwhile pursuing.