cake-contrib / Cake.AddinDiscoverer

Tool to aid with discovering information about Cake Addins
MIT License
5 stars 6 forks source link

Prerelease version is ignored when generating YAML content #140

Open Jericho opened 3 years ago

Jericho commented 3 years ago

As @pascalberger mentioned here, prerelease is ignored when generating YAML content.

Jericho commented 3 years ago

The plot thickens: it seems like the prerelease part of the version is LOST when loading an assembly and its dependencies from a NuGet package as you can see here:

image

I'm loading a prerelease version of the Cake.MinVer assembly which references a prerelease version of Cake.Core but in both cases it shows "1.0.0". This explains why AddinDiscoverer emits "1.0.0" in the YAML instead of "1.0.0.xxxxx" as expected.

Jericho commented 3 years ago

Seems like System.Version does not support "prerelease"?!?!?!? Only Major, Minor and Revision???

augustoproiete commented 3 years ago

@Jericho Correct. Unfortunately System.Version doesn't understand semantic versioning. It's a very old type from the beginnings of the .NET Framework that never evolved.

Most projects store the SemVer version in the AssemblyInformationalVersion attribute which is text based.

In the case of Cake, it's being stored in the AssemblyDescription attribute, so you'll have to read that instead:

image

Jericho commented 3 years ago

I came to the same conclusion: I am able to get an assembly's version by looking at the AssemblyInformationalVersion attribute. Unfortunately, I don't think it will help me get the version for references because the returned type is AssemblyName as opposed to assembly:

var references = assemblyInfoToAnalyze.Assembly.GetReferencedAssemblies();
gep13 commented 3 years ago

There is a SemanticVersion class in the NuGet code base that may be of help:

https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Versioning/SemanticVersion.cs

pascalberger commented 3 years ago

Unfortunately not. We're need to know the reference of Cake.MinVer assembly to the Cake assembly. This is something which is (at least in most cases and by our best practices) not a NuGet package dependency. As it seems there's no way to find out the exact version through the assembly references (AssemblyName class).

augustoproiete commented 3 years ago

NB: I'm happy to move the comments below to its own issue if it makes sense, but for now just discussing

This is something which is (at least in most cases and by our best practices) not a NuGet package dependency

@pascalberger This is interesting and if this is the case, then Cake.Core and Cake.Common should be marked as developmentDependency packages to prevent the Cake Add-in nuget package from having a reference to Cake.Core and/or Cake.Common nuget packages.

Any recent add-in that uses the SDK-style project & dotnet pack will have these package references automatically added to the Cake add-in package, which is what happened to Cake.MinVer.

image

If marking Cake.Core and Cake.Common as developmentDependency is not a possibility (e.g. breaking other use-cases), then I'd suggest creating a meta-package at Cake extension developers, mark that as a developmentDependency, and have the meta-package reference Cake.Core and Cake.Common.

image

augustoproiete commented 3 years ago

ps: The good thing about creating a meta-package for add-ins, is that it opens the door for participating in the build process (*).

For example, we could add a .targets to the "Cake.Extensions" meta-package that automatically include some Cake attributes to the add-in assembly, which could be leveraged by AddinDiscoverer (and even Cake Host for that matter).

e.g. If add-ins automatically had the attribute [CakeTargetVersion("1.0.0-rc0001")] added during build, it would have been handy for this particular issue.

* Which you prob. wouldn't want to do with Cake.Core and Cake.Common directly.

Of course, it would take time for add-ins to move to using the meta-package, etc. it's a process... (perhaps another step for AddinDiscoverer to automate, create issue and/or submit PR to add-in repo as it does for other things today).

pascalberger commented 3 years ago

@augustoproiete Cake-Contrib org has some guidelines regarding this. This is something which we maybe should make a first class citizen on cakebuild.net (https://github.com/cake-build/website/issues/1049). There's also a CakeContrib.Guidelines project which provides analyzers for this.

augustoproiete commented 3 years ago

@pascalberger The documentation in the guidelines is great, and I remember reading it and doing it correctly for the initial version of the add-in targeting 0.33.0, but when I upgraded the add-in to target 1.0.0-rc0001 I copied the PackageReference from nuget.org (to make sure I had the right version number), and that's when I lost the PrivateAssets="All".

It would be a nice if I didn't have to worry about it every time I update an add-in to target a higher Cake version, and the developmentDependency would provide exactly that.

Anyway, I'll check out the analyzers you mentioned as that seems useful.

Jericho commented 3 years ago

I really like the idea of meta package that can automatically set the references to be private and can also add a custom attribute to make it easy to detect the version of the reference Cake package.

augustoproiete commented 3 years ago

I really like the idea of meta package that can automatically set the references to be private and can also add a custom attribute to make it easy to detect the version of the reference Cake package.

I put together a quick proof-of-concept to test embedding metadata in add-ins automatically.

I created two NuGet packages: Cake.Extensions.PoC and Cake.Addins.Sample.

![image](https://user-images.githubusercontent.com/177608/101125365-74434480-35cf-11eb-955d-4d7161743754.png)

The Cake.Extensions.PoC package automatically embeds an AssemblyMetadataAttribute during build called CakeTargetCakeVersion (we can improve on the naming later :sweat_smile: ) with the Cake version that the add-in targets.

It's the equivalent of adding [assembly: System.Reflection.AssemblyMetadata("CakeTargetCakeVersion", "{version}")] to a project.

If you read the add-in assembly inside the Cake.Addins.Sample package, you should be able to retrieve the attribute and read its value:

image

The Cake.Addins.Sample package multi-targets to net5.0, netstandard2.0, and net461 so you should be able to read the attribute on all three assemblies.

NB: I'm adding only one attribute per assembly for testing purposes, but I could add as many needed, just with different names.


Let me know what you guys think (and if I missed anything).

This is just a test, of course. If we were to move forward with it there's more work to do in terms of hardening and testing to ensure it works on VB .NET and FSharp projects, as well as the old csproj/packages.config kind of project.