NuGet / Home

Repo for NuGet Client issues
Other
1.5k stars 252 forks source link

NuGet package contentFiles don't get copied when downgrading package on incremental build #12578

Open JanKrivanek opened 1 year ago

JanKrivanek commented 1 year ago

NuGet Product Used

dotnet.exe, MSBuild.exe

Product Version

This can be reproduced in Visual Studio 2022 version 17.2, using NuGet 6.2

Worked before?

No response

Impact

None

Repro Steps & Context

Copying from the issue in MSBuild repo (https://github.com/dotnet/msbuild/issues/8276):

Issue Description

When using incremental builds, the NuGet contentFiles out of a NuGet package may not be copied when downgrading to an older version of the package. NuGet marks the files as PreserveNewest, which in this scenario is causing problems, as the "newer" files that we don't one during our downgrade, is preserved over the file we do want, the older version.

Steps to Reproduce

Using a simple C# project, reference a NuGet package that contains contentFiles that have copyToOutput=true set. In our case the NuGet package has a .dll in lib\net472 and a contentFile in contentFiles\any\net472 (lets call it hash.txt, containing the hash of the dll, so it needs to remain in sync)

  1. Reference version 1.0 of the NuGet package and build the project Notice the output contains version 1.0 of the .dll and the content file
  2. Update the NuGet reference to version 2.0 of the NuGet package and build the project Notice the output contains version 2.0 of the .dll and the content file
  3. Revert back to version 1.0 of the NuGet package and build the project and build the project Notice the output contains version 1.0 of the .dll and version 2.0 of the content file

We have narrowed the issue down to the generated nuget.g.props file, which correctly mentions the "hash.txt" file with its metadata, of which the important one is

<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>

MSBuild is not copying the file, because the file in the output is already newer and also does not consider this file during the IncrementalClean, as this target does not seem to do a timestamp check, only check for Orphans.

For simplicity, lets say our nuspec files look like below, the work fine in general.

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
  <metadata>
    <id>PackageName</id>
    <version>2.0.0</version>
    <contentFiles>
      <!-- Include Assets as Content -->
      <files include="**\*.*" buildAction="None" copyToOutput="true" />
    </contentFiles>
    <dependencies>
      <group targetFramework=".NETFramework4.7.2" />
    </dependencies>
  </metadata>
  <files>
    <file src="bin\Release\AssemblyName.dll" target="lib\net472" />
    <file src="bin\Release\Hash.txt" target="contentFiles\any\net472" />
  </files>
</package>

Expected Behavior

We would expect MSBuild to understand the contentFiles need to be replaced with the previous version

Actual Behavior

MSBuild ignores the copy step as the target file is newer

Analysis

We believe this problem goes back to the generated nuget.g.props file, when setting copyToOutput="true" in the nuspec file, NuGet always flags the files with "PreserveNewest", while we believe it should treat it the same as the libraries, these are correctly copied when downgrading a package version. Giving the package builder an option to say the copy step should use "Always" it would help, but this would add unnecessary build time as well. While only None, PreserveNewest and Always are available in MSBuild, an option to specify "copy if different" would be best, skip if the file is the same, otherwise copy. Which is basically what "Always" is supposed to do, only always does an overwrite even if the file is the same, which depending on the amount of files comes with a price and IO load.

Versions & Configurations

This can be reproduced in Visual Studio 2022 version 17.2, using NuGet 6.2

Workarround

We managed to find a "hack" to influence the behaviour in 2 ways, both using Always instead of PreserveNewest, which for our 1200 csproj files, add some additional build time.

  1. Update all our NuGet packages .props files to set a different value for the CopyToOutputDirectory
    <ItemGroup> 
    <None Update="@(None)">
    <CopyToOutputDirectory Condition="%(CopyToOutputDirectory) == 'PreserveNewest'">Always</CopyToOutputDirectory>
    </None>
    </ItemGroup>
  2. Update all None includes from our NuGet packages to always copy to the output direct. Put this in the Directory.Build.Targets
    <ItemGroup> 
    <None Update="@(None)">
    <CopyToOutputDirectory Condition="%(CopyToOutputDirectory) == 'PreserveNewest' AND $([System.String]::new('%(NuGetPackageId)').Contains('CompanyPrefix.'))">Always</CopyToOutputDirectory>
    </None>
    </ItemGroup>

Verbose Logs

No response

JanKrivanek commented 1 year ago

Discussed offline with @zivkan:

erdembayar commented 1 year ago

@JanKrivanek Could you able to give me name of nuget package can used to repro this problem? It looks it could be any package with content files. But if you know one what would be great.

JanKrivanek commented 1 year ago

@Nico-1987 (the original issue creator) - is there any publicly available nuget that can be used to quickly repro the issue?

bh-sijtnic commented 1 year ago

We use contentFiles with copyToOutput a lot in our internal NuGet packages, so i had to search for a public package that does the same. I actually found a nice example. The ContentFilesExample package.

Steps to reproduce:

Hope this helps. Thanks for picking this up!

erdembayar commented 1 year ago

I can repro this and there's workaround is available.

jeffkl commented 1 year ago

Team Triage: Ideally NuGet would keep track of all of the content files and which package they came from. From build to build, it would detect if the same package was in use and if not, delete the content files that were copied in a previous build. That way if a new package was referenced, the existing files would be deleted and the same MSBuild logic would run to copy the files to the output directory. If the package version did not change, MSBuild's Copy task would only copy the files if they were newer.

tHesTx commented 1 year ago

I have sort of a same problem, except that it happens every time I install the bootstrap package - files are only show as reference in my project. I have a post here if allowed.