Invalid order with NuGet pack leading to invalid outputs

[xoofx]

Hey,
Thank you for MinVer. I'm starting to use it in all my projects and it simplifies a lot what I was doing manually! 👍

Version(s)

2.5.0

Problem

I'm developing a tooling that simplifies the whole build/publish process for .NET libraries/apps on GitHub (dotnet-releaser) and I discovered a weird behavior while using MinVer on a multi-targeting version (It seems that it doesn't affect packages with only one target framework).

I'm calling msbuild directly with a special logger from which I can extract results from msbuild and I discovered that the version used to name output files in msbuild with MinVer when packing is wrong.

For example, below, you have a screenshot of msbuild log:

It is supposed to create the package Tomlyn.0.11.0.nupkg and the pack command does it, but actually, when you look in the details, msbuild will think that it needs to generate Tomlyn.1.0.0.nupkg and will always re-run even if you build it just before.

Reason is that the file output is calculated in the target _GetOutputItemsFromPack which called by _CalculateInputsOutputsForPack (code here) but MinVer runs only just before GenerateNuspec (as you can see in the screenshot above), but not before _GetOutputItemsFromPack.

Expected behaviour

NuGet msbuild tasks should get the version calculated from MinVer in order to correctly compute output packages.

Actual behaviour

NuGet is originally thinking that the version is 1.0.0 instead the version calculated by MinVer. If you run multiple times NuGet pack with MinVer, it will always have to repack the package thinking that it doesn't exist (because 1.0.0 is not created in the end)

Workarounds

No great workaround as I would have to make my tooling MinVer aware to force MinVer to insert before _GetOutputItemsFromPack.

Fix Suggestion

Add a msbuild dependency to _GetOutputItemsFromPack instead of only GenerateNuSpec.
Hacking MinVer locally and it is now correctly generating the correct versions:

I will make a PR with this suggestion fix.

[xoofx]

Created also an issue on NuGet NuGet/Home#11617 to request for a better way to integrate auto-versioning helpers like MinVer into NuGet.

[adamralph]

@xoofx thanks for the detailed write up!

I'm having difficulty understanding the actual behavioural problem though. Are you saying that any project which multi-targets ends up with the wrong version?

If I build this test solution, it works fine:

[xoofx]

I'm having difficulty understanding the actual behavioural problem though. Are you saying that any project which multi-targets ends up with the wrong version?

So, the problem is that the packaging is working fine only because the final NuGet task is still going to use the version that is computed just before GenerateNuSpec, but the problem is that the pre-calculated outputs "@(NuGetPackOutput)" are wrong:

https://github.com/NuGet/NuGet.Client/blob/352ba94f120f1a3481a2cae6190a1947a85a160f/src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets#L211-L214

if you do a verbose log of dotnet pack --verbosity diag, you should see some test.1.0.0.nupkg in your logs (generated for NuGetPackOutput, assuming that you are using MinVer). You can see that in the screenshot above:

That means that the output recorded in @(NuGetPackOutput) are invalid. That means that when using MinVer, and you try to do another pack after a pack you juts did, you will see that it will overwrite the generated package, because it will look for test.1.0.0.nupkg to detect if it needs to rebuild while test.2.3.4.nupkg is only generated in the end.

In the details, this is the order of the target executed by msbuild:

_GetOutputItemsFromPack
_CalculateInputsOutputsForPack
MinVer
GenerateNuspec

MinVer is executed after _GetOutputItemsFromPack, but _GetOutputItemsFromPack is actually already accessing PackageId and PackageVersion here

  <Target Name="_GetOutputItemsFromPack"
          DependsOnTargets="_GetAbsoluteOutputPathsForPack"
          Returns="@(_OutputPackItems)">

    <!-- 'PackageOutputAbsolutePath' and 'NuspecOutputAbsolutePath' will be provided by '_GetAbsoluteOutputPathsForPack' target -->

    <GetPackOutputItemsTask
        PackageOutputPath="$(PackageOutputAbsolutePath)"
        NuspecOutputPath="$(NuspecOutputAbsolutePath)"
        PackageId="$(PackageId)"
        PackageVersion="$(PackageVersion)"
        IncludeSymbols="$(IncludeSymbols)"
        IncludeSource="$(IncludeSource)"
        SymbolPackageFormat="$(SymbolPackageFormat)">

      <Output
        TaskParameter="OutputPackItems"
        ItemName="_OutputPackItems" />
    </GetPackOutputItemsTask>
  </Target>

[xoofx]

To show the problem with a HelloWorld. If you run twice pack on an HelloWorld library:

> dotnet pack

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\net6.0\HelloWorld.dll
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\netstandard2.0\HelloWorld.dll
  Successfully created package 'C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\HelloWorld.1.0.0.nupkg'.

> dotnet pack

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\net6.0\HelloWorld.dll
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\netstandard2.0\HelloWorld.dll

You can see that on the second run, HelloWorld.1.0.0.nupkg is not generated. Because msbuild is able to figure out that via @(NuGetPackOutput) that HelloWorld.1.0.0.nupkg is up-to-date.

Now, add MinVer to the HelloWorld project, tag the commit with 2.3.4 and do 2 times a pack in a row:

> dotnet pack

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored C:\code\dotnet-releaser\tmp\HelloWorld\HelloWorld.csproj (in 94 ms).
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\net6.0\HelloWorld.dll
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\netstandard2.0\HelloWorld.dll
  Successfully created package 'C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\HelloWorld.2.3.4.nupkg'.

> dotnet pack

Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\net6.0\HelloWorld.dll
  HelloWorld -> C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\netstandard2.0\HelloWorld.dll
  Successfully created package 'C:\code\dotnet-releaser\tmp\HelloWorld\bin\Debug\HelloWorld.2.3.4.nupkg'.

You can see that on the second run, it still creates the HelloWorld.2.3.4.nupkg. That's because msbuild is assuming that the output filename of the package is still HelloWorld.1.0.0.nupkg for the reason that MinVer is running after the msbuild target _GetOutputItemsFromPack.

[adamralph]

@xoofx thanks! That's clear now. I'll take a closer look at the details a little later.

[adamralph]

@xoofx thanks again for raising this. I've extracted your reproduction steps into a new bug issue.