Fixes Issue #4705 - Wrong version numbers in dlls #4706
Conversation
…ed csprojs and assemblyinfo.cs files accordingly.
…o fileversionissue
There was a problem hiding this comment.
@OsirisTerje Sorry for the delay, I was off sick and couldn't concentrate enough to do reviews.
I also like code generation, but I do think your second option is nicer.
I think we can remove a lot of conditionals in the other projects by utilizing the AssemblyConfiguration set in the Directroy.Build.props
src/NUnitFramework/nunit.framework.legacy/nunit.framework.legacy.csproj
Outdated
Show resolved
Hide resolved
OsirisTerje
changed the title
Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
…cy.csproj Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
…o fileversionissue
…iguration .NET 7.0 is out of support as of May 14th 2024.
| [TestCase("1.2.3-beta.1", "1.2.3.1")] | ||
| [TestCase("1.2.3", "1.2.3.0")] | ||
| public void ThatWeCanParseFileVersionStrings(string versionString, string expected) | ||
| { | ||
|
|
||
| var parsedVersion = VersionParsers.ParseFileVersion(versionString); | ||
| Assert.That(parsedVersion, Is.EqualTo(expected)); | ||
| } | ||
|
|
||
| [TestCase("1.2.3-beta.1", "1.2.3.0")] | ||
| [TestCase("1.2.3", "1.2.3.0")] | ||
| public void ThatWeCanParseAssemblyVersionStrings(string versionString, string expected) | ||
| { | ||
| var parsedVersion = VersionParsers.ParseAssemblyVersion(versionString); | ||
| Assert.That(parsedVersion, Is.EqualTo(expected)); | ||
| } |
There was a problem hiding this comment.
I think that you have Assembly and File version the wrong way around.
As per documentation the AssemblyVersion must be unique
There was a problem hiding this comment.
@manfred-brands The AssemblyVersion should indicate the Major.Minor version (above also patch in included, but we're not quite semver yet), and not change for each build or revision. The FileVersion (aka AssemblyFileVersion) should change for all patches and revisons.
To me it looks like that is what the code above do.
Or do you see something I don't (not surprised though).
There was a problem hiding this comment.
The code does what you say it does. But my interpretation of the linked documentation is that the AssembyVersion should be unique so the .NET Resolver find the correct instance.
If one builds against 1.2.3-beta.1 then later against 1.2.3-beta.2 which has a bugfix, how would the compiler (and developer) know that we have the correct version if both are versioned as 1.2.3.0.
Maybe one of our native English (@stevenaw ?) speakers can read the linked documentation.
There was a problem hiding this comment.
Good idea! (I am not at all certain about how this should be) Perhaps also @CharliePoole who raised the issue can chime in here.
There was a problem hiding this comment.
The key idea in the documentation seems to be expressed here:
"Suppose you're building a framework assembly for your project that is used by lot of developers while building the application assemblies. If you release new version of assembly frequently, say once every day, and if assemblies are strong named, Developers will have to change the reference every time you release new assembly. It can be cumbersome and may lead to wrong references also. A better option in such closed group and volatile scenarios would be to fix [i.e. keep the same] the AssemblyVersion and change only the AssemblyFileVersion. Use the assembly file version number to communicate the latest release of assembly. In this case, developers won't have to change the references and they can overwrite the assembly in reference path. In central or final release builds it makes more sense to change the AssemblyVersion and most keep the AssemblyFileVersion same as assembly version."
So Microsoft is suggesting to do one thing for internal development changes and the opposite for releases. In fact, that makes perfect sense to me. Normally, it should be easy for those inside the project to keep using a reference but outside users should have to make a decision to move ahead.
However, I don't think any of this addresses the issue I raised by email with @OsirisTerje. I was concerned that the three component Package Version did not match the either the AssemblyVersion or the AssemblyFileVersion of the primary assembly included in the package. While NuGet allows package versions to be completely independent from the versions of included assemblies, maintaining consistency between the two is helpful to users and I think it would be a good practice for NUnit to ensure that all three use the same major, minor and build components.
There was a problem hiding this comment.
In central or final release builds it makes more sense to change the AssemblyVersion and most keep the AssemblyFileVersion same as assembly version.
Thanks @CharliePoole ! I then think this is the appropriate solution for us.
And Charlie - this does fix the issue. The issue arised because we had to manually set these numbers, and that was forgotten. Now they are automatically set together with the package version, and will follow the package version, given we now have a resolution for how these numbers should be :-)
There was a problem hiding this comment.
This is actually not that easy. We want the AssemblyVersion to be (equal to or) higher than the beta releases. If we add the beta number as the 4th digit, that will be 0 for release, which means lower than the beta ones.
Earlier when we just did this manually, we kept the File and Assembly version numbers equal and fixed through the beta period.
and
So if we continue to follow this practice we would not create anything more than we potentially have had for years.
Fixes #4705