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

Prerelease version is ignored when generating YAML content #140

Open
Jericho opened this issue Dec 1, 2020 · 12 comments
Open

Prerelease version is ignored when generating YAML content #140

Jericho opened this issue Dec 1, 2020 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@Jericho
Copy link
Member

Jericho commented Dec 1, 2020

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

@Jericho Jericho added the bug Something isn't working label Dec 1, 2020
@Jericho Jericho added this to the 3.56.1 milestone Dec 1, 2020
@Jericho Jericho self-assigned this Dec 1, 2020
@Jericho
Copy link
Member Author

Jericho commented Dec 2, 2020

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
Copy link
Member Author

Jericho commented Dec 2, 2020

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

@augustoproiete
Copy link
Member

@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
Copy link
Member Author

Jericho commented Dec 2, 2020

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
Copy link
Member

gep13 commented Dec 2, 2020

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

pascalberger commented Dec 2, 2020

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

@augustoproiete
Copy link
Member

augustoproiete commented Dec 2, 2020

@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
Copy link
Member Author

Jericho commented Dec 3, 2020

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.

@Jericho Jericho removed this from the 3.56.1 milestone Dec 3, 2020
@augustoproiete
Copy link
Member

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

  • Cake.Extensions.PoC (nuget) (github repo) is the meta-package that an add-in developer would reference in their add-in projects (instead of Cake.Core and Cake.Common). This meta-package brings Cake.Core and Cake.Common as a dependency, so it feels the same from the point of view of the add-in developer.

  • Cake.Addins.Sample (nuget) (github repo) is an example of an add-in that uses the Cake.Extensions.PoC package above - which can be used to prove that the AddinDiscoverer can retrieve the metadata.


The Cake.Extensions.PoC package automatically embeds an AssemblyMetadataAttribute during build called CakeTargetCakeVersion (we can improve on the naming later 😅 ) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants