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

Packing with ContinuePackingAfterGeneratingNuspec=false should not fail when there are no errors #9786

Closed
wli3 opened this issue Jul 12, 2020 · 22 comments · Fixed by NuGet/NuGet.Client#3610
Assignees
Labels

Comments

@wli3
Copy link

wli3 commented Jul 12, 2020


Issue moved from dotnet/sdk#12292


From @aggieben on Tuesday, June 30, 2020 7:41:03 PM

Using the 5.0.0-preview.5.20279.10 SDK on Windows 10, the tooling fails to pack a nupkg for the project located here: https://github.com/aggieben/ConstrainedTypes (master branch).

I used the following command:
dotnet pack -o pkg ConstrainedTypes.fsproj -v diag. The error message reported is:

"C:\Users\Ben\proj\oss\constrainedtypes\src\ConstrainedTypes\ConstrainedTypes.fsproj" (pack target) (1:7) ->
                   (GenerateNuspec target) ->
                     C:\Program Files\dotnet\sdk\5.0.100-preview.5.20279.10\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(198,5): error MSB4181: The "PackTask" task returned false but did not log an error. [C:\Users\Ben\proj\oss\constrainedtypes\src\ConstrainedTypes\ConstrainedTypes.fsproj]

The exact same command works just fine with SDK version 3.1.300.

@heng-liu
Copy link
Contributor

Hi @aggieben, I tried the following steps to repro, but failed:

  1. git clone https://github.com/aggieben/ConstrainedTypes
  2. install dotnet SDK 5.0.0-preview.5.20279.10
  3. go to constrainedtypes\src\ConstrainedTypes folder
  4. run dotnet pack -o pkg ConstrainedTypes.fsproj -v diag
    The error I encountered is:
Build FAILED.

                   "C:\repos\ConstrainedTypes\src\ConstrainedTypes\ConstrainedTypes.fsproj" (Restore target) (1) ->
                   "C:\repos\ConstrainedTypes\src\ConstrainedTypes\ConstrainedTypes.fsproj" (_GenerateRestoreGraphProjectEntry target) (1:5) ->
                   (PaketRestore target) ->
                     C:\repos\ConstrainedTypes\.paket\Paket.Restore.targets(171,5): error MSB3073: The command "dotnet paket restore" exited with code 1. [C:\repos\ConstrainedTypes\src\ConstrainedTypes\ConstrainedTypes.fsproj]

    0 Warning(s)
    1 Error(s)

I also tried to install dotnet SDK 3.1.400, but the result is the same as above.

Can you pls provide the repro steps if the above is not accurate? Thanks!

@heng-liu heng-liu added WaitingForCustomer Applied when a NuGet triage person needs more info from the OP Product:dotnet.exe labels Jul 15, 2020
@lanfeust69
Copy link

Same thing here, using SDK 3.1.401 to build FSharp.Formatting.

No problem when forcing 3.1.302 through global.json.

@AlexVallat
Copy link

Also been reported to Paket: fsprojects/Paket#3897

@AlexVallat
Copy link

This appears to be because if ContinuePackingAfterGeneratingNuspec task parameter to PackTask is set false, PackCommandRunner.BuildPackage will return null (as there is no package built, as GenerateNugetPackage is false). This in turn means that PackCommandRunner.RunPackageBuild returns false, which is treated by PackTask.Execute as failure.

Not generating a package when the generation of a package is not requested should not be considered failure.

@baronfel
Copy link

This has completely killed any project that uses Paket from updating to newer SDKs, or even participating in testing the 5.x previews. Paket calls the Pack Task twice:

  • once with ContinuePackingAfterGeneratingNuspec set to false, to generate the nuget-derived nuspec (to ensure that many project properties/shared properties are set in a manner consistent with Nuget itself). once this nuspec is generated it then modifies the generated reference sections of the nuspec in line with paket's version bounds.
  • once with ContinuePackingAfterGeneratingNuspec set to true, providing the new-patched nuspec in order to actually generate the package.

@zivkan
Copy link
Member

zivkan commented Aug 26, 2020

@heng-liu previously attempted to reproduce the problem, but was unable to, as she got a different error. Can someone provide us with a simple repro?

@baronfel
Copy link

The linked paket issue contains a zip of a project that triggers this, along with reproduction steps.

@TheAngryByrd
Copy link

I think she didn't do dotnet tool restore since the error stated it couldn't run dotnet paket restore

@zivkan
Copy link
Member

zivkan commented Aug 26, 2020

I'm taking a quick look, but it seems I need to use the paket and fake CLIs, both products that the NuGet team doesn't support, so first I have to figure out how these tools are using NuGet. If someone has a repro that minimises usage of 3rd party tools, that could speed up our investigation.

edit: never mind. I just noticed that the issue reporter said that running dotnet pack reproduces the problem, which I can confirm.

@baronfel
Copy link

@zivkan the entire point is the usage of third-party tools :) The way the targets delegated to the PackTask relies on an invariant that has changed between 3.1.30x and 3.1.40x, and the contention is that this invariant change is a bug.

The core question is: should not making a nuget package when the user explicitly requested that behavior cause a failing build?

@zivkan
Copy link
Member

zivkan commented Aug 26, 2020

3rd party tools still invoke NuGet somehow. If someone can peel away those layers and report just the NuGet command that fails, I can investigate that. Otherwise, I have to spend time learning a new tool before I can even begin investigating the reported problem. If the problem is how the 3rd party tools use NuGet, I don't know how to write this without sounding like a jerk, but it's out of my job description.

Anyway, I've figured out how to invoke NuGet in a way that reports the error. Debugging the .NET SDK is not easy and requires setup, but I'll take a quick look to see if I spot something obvious.

@zivkan
Copy link
Member

zivkan commented Aug 26, 2020

The problem is an unintended consequence from fixing #7404 (when a pack warning is elevated to error via WarnAsError, NuGet was not telling MSBuild there was an error, even though it logged errors).

It looks the implementation was to: delete the zip file on error, and report to MSBuild whether the zip file exists. This doesn't take into account ContinuePackingAfterGeneratingNuspec="false", hence this bug.

@zivkan zivkan added Functionality:Pack Triage:NeedsTriageDiscussion Type:Bug and removed WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Aug 26, 2020
@baronfel
Copy link

Nice investigation work! Thanks for narrowing down the root cause.

@zivkan zivkan changed the title error MSB4181: The "PackTask" task returned false but did not log an error. Packing with ContinuePackingAfterGeneratingNuspec=false should not fail when there are no errors Aug 26, 2020
@zivkan zivkan added Category:Quality Week Issues that should be considered for quality week and removed Triage:NeedsTriageDiscussion labels Aug 26, 2020
@zivkan zivkan self-assigned this Aug 26, 2020
@zivkan zivkan added this to the Sprint 175 - 2020.08.17 milestone Aug 26, 2020
@zivkan
Copy link
Member

zivkan commented Sep 2, 2020

I expect the fix will be available in NuGet 5.8 and the .NET 5 SDK (rc2 maybe. I'm not sure if there's time for us to make rc1 still)

@Danl2620
Copy link

Are there any workarounds in the meantime? I.e. any other way to produce a .nupkg file from a .net core project?

@AlexVallat
Copy link

I've been editing my .paket\packet.restore.targets file and commenting out the <ContinuePackagingAfterGeneratingNuspec>false line

@Danl2620
Copy link

Nice, that works, thanks @AlexVallat !

@aggieben
Copy link

I think she didn't do dotnet tool restore since the error stated it couldn't run dotnet paket restore

Yep, this was it. Sorry - I forgot about this issue. The repro would need:

  1. dotnet tool restore
  2. dotnet paket install
  3. dotnet pack -o pkg ConstrainedTypes.fsproj -v diag

Hopefullly I didn't leave out a step this time 😬

@Danl2620
Copy link

Danl2620 commented Sep 19, 2020

Could this issue result in a .lock file no longer containing the transitive dependencies of the packaged .nupkg file?

i.e. if I used the workaround of commenting out the ContinuePackagingAfterGeneratingNuspec line in Packet.Restore.Targets (or setting it to true) and do the full restore the .lock file is now missing dependencies and my code doesn't compile (although everything restores properly). Nasty issue...

@ModernRonin
Copy link

I can confirm this goes away with net5-rc2.

@ScottShingler
Copy link

Just had a fun day dealing with this in .NET Core 3.1.407 while trying to address CVE-2021-26701. The workaround @AlexVallat suggested was very helpful, but I found that Paket.Restore.targets would get overwritten every time paket restore is run. To work around this, I ended up adding the following to our affected fsproj and csproj files just below <Import Project="..\..\.paket\Paket.Restore.targets" />:

<Target Name="PaketDisableDirectPack" AfterTargets="_IntermediatePack" BeforeTargets="GenerateNuspec" Condition="('$(IsPackable)' == '' Or '$(IsPackable)' == 'true') And Exists('$(PaketIntermediateOutputPath)/$(MSBuildProjectFile).references')" >
</Target>

Is there any chance of this being fixed in .NET Core 3.1?

@enricosada
Copy link

enricosada commented Mar 11, 2022

Something similar to set the ContinuePackingAfterGeneratingNuspec to true can be in a Directory.Build.targets in the root dir (or any parent dir)

  <!-- Workaround for https://github.com/NuGet/Home/issues/9786 -->
  <Target Name="PaketDisableDirectPackWorkaround" AfterTargets="PaketDisableDirectPack">
    <PropertyGroup>
      <ContinuePackingAfterGeneratingNuspec>true</ContinuePackingAfterGeneratingNuspec>
    </PropertyGroup>
  </Target>

so paket install doesnt override it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.