Change the calculation of the TFM for Cake addins

[nils-a]

In versions up until Cake 1.0.0 we has only one rule for TargetFramework.

In current development, there is three of them:

• addins referencing Cake < 1.0.0: netstandard2.0 (required) and net461 (suggested)
• addins referencing Cake ≥ 1.0.0: netstandard2.0 (required) and net461 (suggested) and net5.0 (suggested)
• modules: netstandard2.0 (required)
• recipes: no required or suggested references

Currently for addins not referencing Cake (Core or Common) the same rule as for Cake < 1.0.0 is selected.

@Jericho asked in #107

Why are the targeting rules different depending on whether an addin targets Cake.Core or not? Shouldn't we be consistent and always make the same recommendation?

And my reply was

Currently the rule defaults to the "lowest" requirement - in this case that is the "Cake < 1.0.0"-rule. We could argue, that since Cake 1.0.0 is now the default, the default-rule should be the "Cake ≥ 1.0.0"-rule.

The question is: Does someone have other ideas, or opinions on this? I really don't have a strong opinion on this, the way it is currently implemented was only the first thing that came to my mind when implementing this.

[augustoproiete]

Do we have examples of Cake addins that don't reference Cake.Core nor Cake.Common but are still considered true Cake addins? 🤔

In fact, what is the definition of a true Cake addin?

To me, it's a class library that has tailor-made aliases specifically to be used in Cake builds, by adding extension methods to ICakeContext. So if this definition is correct, then it wouldn't be possible for a true Cake addin not to reference Cake.Core (directly or transitively).

Similarly, true Cake modules need to implement the ICakeModule interface and have the CakeModule attribute in the assembly... So like addins, it wouldn't be possible for a true Cake module not to reference Cake.Core (directly or transitively).

The fact Cake can leverage/load any assembly via the #addin directive doesn't make them true Cake addins, so I wouldn't expect, say, Polly to ever install the CakeContrib.Guidelines packages.

Finally my understanding is that the TFM rules exist to help ensure that an addin/module will be compatible with Cake runners. If a Cake-Contrib project is not "loaded" by the Cake runners (and therefore doesn't need Cake.Core nor Cake.Common), then I think CakeContrib.Guidelines shouldn't care about what TFMs are being targeted, as it doesn't affect Cake at all... It's at the discretion of the project maintainer.

[nils-a]

The point came up when adding CakeContrib.Guidelines to Cake.Email and Cake.Email.Common.

And while I do agree to the "not caring about not-addins", I think there is probably a reason to reference CakeContrib.Guidlines and that it is probably because a use in a Cake context is intended.

So, looking at it that way, for a "common" project that should be used in other cake related projects - shouldn't that "common" project target the same targetFramworks (or a superset) as the intended using packages?

[devlead]

If it has no direct reference Core, Common nor runners then it's more a case of Cake supporting it than the other way around. Plus what @augustoproiete said.

[Jericho]

We are trying to encourage addin authors to upgrade their addins to target Cake 1.0.0, therefore in my mind it makes sense that the "default" target rule would be "Cake > 1.0.0".

[nils-a]

I kind of agree to both "positions" currently.
I feel the way I implemented the default-rule currently is wrong. I propose the following change:

• projects named *.cake (addins): Cake based rules, as above with the default being Cake ≥ 1.0.0
• projects named *.module (modules): Rule as above.
• projects named *.recipe (recipes): Rule as above.
• projects not named *.cake, *.module, *.recipe : Do not require any specific TargetFramework

Additionally, to address what @devlead said I propose to extend #98 to raise a warning if the project is named either *.cake (addin) or *.module (module) but does not reference Cake.

[augustoproiete]

So, looking at it that way, for a "common" project that should be used in other cake related projects - shouldn't that "common" project target the same targetFramworks (or a superset) as the intended using packages?

A human looking at Cake.Email.Common would say "YES", but a machine would say "I DON'T KNOW" as it doesn't have enough information to decide which TFM(s) should be targeted because it's a dependency package... - The machine can't know where it's going to be used...

The entry-point is the Cake.Email addin, so it owns the responsibility to choose the right dependencies that it can use/take to satisfy its implementation.

Now if you wanted to - as part of analyzing a true Cake addin such as Cake.Email - you could check its referenced assemblies and detect if any of the referenced assemblies don't match the TFMs being used by the addin - That would be the right place to issue warnings... Not in the dependency package. Of course this would be complex as you'd have to walk the whole dependency tree.

[augustoproiete]

• projects named *.cake (addins): Cake based rules, as above with the default being Cake ≥ 1.0.0
• projects named *.module (modules): Rule as above.
• projects named *.recipe (recipes): Rule as above.
• projects not named *.cake, *.module, *.recipe : Do not require any specific TargetFramework

Hmm I don't think rules should be based on the project name. I think we have better ways to determine if the project is an addin/module/recipe, to then recommend the right TFM and Cake.Core/Cake.Common version to depend on if it's a Cake extension.

[nils-a]

I think we have better ways to determine if the project is an addin/module/recipe

Any ideas that work from msbuild are highly welcome 😄

[augustoproiete]

Any ideas that work from msbuild are highly welcome 😄

Would it work if it was an MSBuild task that runs after the build, and analyzes the generated assemblies via Mono.Cecil (or similar)?

[nils-a]

Would it work if it was an MSBuild task that runs after the build, and analyzes the generated assemblies via Mono.Cecil (or similar)?

Probably. I have created #120 to track that.

[augustoproiete]

Hmm I don't think rules should be based on the project name.

I thought more about this and realized that my concern about with using project names to detect what kind of Cake extension it is was mainly because it's hard to differentiate between Cake addins and other Cake supporting projects just based on project names - which is not the case for Modules and Recipes where using the project name would prob. be fine.

If we check for Cake.Core and/or Cake.Common first before applying any rules, I think using project names might be fine.

• Project references Cake.Core and/or Cake.Common

  • and project named Cake.**.Module (assume it's a module): Apply TFM rule for modules
  • and project is named Cake.**.Recipe (assume it's a recipe): Apply TFM rule for recipes
  • and project name starts with Cake. (assume it's an addin): Apply TFM rule for addins
  • none of the above: Do not apply TFM rule

• Project don't reference Cake.Core or Cake.Common

  • Do not apply TFM rule

Would that be easier to implement with the current codebase, without going the path I suggested with Mono.Cecil, etc?

[gep13]

This seems like a sensible approach to me! We can always enhance the detection rules later, but this would be a solid first pass.

[nils-a]

Would that be easier to implement with the current codebase,

Yes, that's only a real small change compared to the current codebase. I'll proceed with that change and leave #120 open to be revisited later.