project-system icon indicating copy to clipboard operation
project-system copied to clipboard

FUTDC doesn't check packed items when GeneratePackageOnBuild is true

Open MattKotsenas opened this issue 1 year ago • 6 comments

Issue Description

When including content files in a NuGet package built with <GeneratePackageOnBuild>true</GeneratePackageOnBuild>, if only the content files change, Visual Studio considers the build up to date and doesn't create a new .nupkg, which breaks incremental build.

Workaround

  • Works on dotnet build
  • Works when setting <DisableFastUpToDateCheck>true</DisableFastUpToDateCheck> in the .csproj

Steps to Reproduce

  1. dotnet new classlib
  2. Create a file build/Task.props (content doesn't matter)
  3. Update .csproj file so it looks like this
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <RootNamespace>repro_generatepackageonbuild_futdc</RootNamespace>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
+    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
  </PropertyGroup>
  
+  <ItemGroup>
+    <Content Include="build/Task.props" Pack="true" PackagePath="build/" />
+  </ItemGroup>
</Project>
  1. Build in Visual Studio
  2. Notice that .nupkg is created
  3. Edit Task.props so it has different content (actual content doesn't matter)
  4. Build in Visual Studio
  5. Notice that build is up-to-date and package is not updated

Expected Behavior

  • Content updates trigger a new Pack operation

Actual Behavior

  • Build is considered up-to-date and package is not updated

dotnet / Visual Studio info

dotnet: 8.0.202 Microsoft Visual Studio Enterprise 2022 Version 17.9.3 VisualStudio.17.Release/17.9.3+34701.34 Microsoft .NET Framework Version 4.8.09032 Installed Version: Enterprise

MattKotsenas avatar Mar 30 '24 04:03 MattKotsenas

Looks like another case of https://github.com/dotnet/project-system/issues/9001

JanKrivanek avatar Apr 02 '24 09:04 JanKrivanek

Looks like another case of #9001

I think this is unrelated to Build Acceleration.

With project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
  </PropertyGroup>

  <ItemGroup>
    <Content Include="build/Task.props" Pack="true" PackagePath="build/" />
  </ItemGroup>

</Project>

Enabling verbose FUTDC logs and building a few times shows:

Comparing timestamps of inputs and outputs:
    Adding UpToDateCheckBuilt outputs:
        D:\MyProject\bin\Debug\net8.0\MyProject.dll
        D:\MyProject\obj\Debug\net8.0\MyProject.dll
        D:\MyProject\obj\Debug\net8.0\MyProject.pdb
        D:\MyProject\bin\Debug\net8.0\MyProject.pdb
    Adding newest import input:
        D:\MyProject\MyProject.csproj
    Adding Compile inputs:
        D:\MyProject\Class1.cs
    No inputs are newer than earliest output 'D:\MyProject\obj\Debug\net8.0\MyProject.pdb' (2024-04-05 09:34:05.631). Newest input is 'D:\MyProject\MyProject.csproj' (2024-04-05 09:33:41.061).
Project is up-to-date.
Up-to-date check completed in 0.6 ms

The Content item is not checked at all, so therefore changes to it will never trigger a build.

drewnoakes avatar Apr 04 '24 22:04 drewnoakes

The SDK defines the _GetPackageFiles target that provides the items we need for this in the VS FUTDC:

https://github.com/dotnet/dotnet/blob/b722edc43e8710aae2f92eca898a7aae083eca74/src/nuget-client/src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets#L491

Ideally we would condition this on the presence of the GeneratePackageOnBuild property in the user's project. However we force this property to be false in design-time builds (to avoid accidentally packaging during such builds):

https://github.com/dotnet/project-system/blob/c69d9b1b5518b5dd9af323af858252a8cc4b4b23/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Build/GeneratePackageOnBuildDesignTimeBuildPropertyProvider.cs#L21-L22

I cannot think of a better way than always calling this target unconditionally before CollectUpToDateCheckInputDesignTime, then including items from _PackageFiles.

drewnoakes avatar Apr 05 '24 01:04 drewnoakes

Fixed in https://github.com/dotnet/project-system/pull/9437

With those changes:

Comparing timestamps of inputs and outputs:
    Adding UpToDateCheckBuilt outputs:
        D:\MyProject\bin\Debug\net8.0\MyProject.dll
        D:\MyProject\obj\Debug\net8.0\MyProject.dll
        D:\MyProject\obj\Debug\net8.0\MyProject.pdb
        D:\MyProject\bin\Debug\net8.0\MyProject.pdb
    Adding newest import input:
        D:\MyProject\MyProject.csproj
    Adding Compile inputs:
        D:\MyProject\Class1.cs
    Adding UpToDateCheckInput inputs:
        D:\MyProject\build\Task.props
    No inputs are newer than earliest output 'D:\MyProject\obj\Debug\net8.0\MyProject.pdb' (2024-04-05 12:20:22.680). Newest input is 'D:\MyProject\MyProject.csproj' (2024-04-05 12:20:14.320).
Project is up-to-date.
Up-to-date check completed in 5.4 ms

Specifically, Task.props is now checked here:

    Adding UpToDateCheckInput inputs:
        D:\MyProject\build\Task.props

drewnoakes avatar Apr 05 '24 01:04 drewnoakes

Verifying this behavior in the latest int.main and it appears the up-to-date might be broken in the other direction (i.e. unnecessary incremental builds).

Using the same sample project from the original repo, here's the set of actions I took, what I expected to happen, and what actually happened

Step Action Expected Result Actual Result Expected == Actual
1 Build / clean build Build Build
2 Build (again) Up-to-date Up-to-date
3 Edit the Task.props file
4 Build Build Build
5 Build (again) Up-to-date Build ⚠️
6 Touch csproj (no edits needed; just update timestamp)
7 Build Build Build
8 Build (again) Up-to-date Up-to-date

It seems like when the only reason for the build is a Pack item, the input to output stamp logic is getting messed up. Do something that causes the assembly to be newer than the pack item (step 6 in this case) fixes the issue. I'm also getting the following FUTDC warning, which may be related:

WARNING: Potential build performance issue in 'classlib.csproj'. The project does not appear up-to-date after a successful build: Input UpToDateCheckInput item 'D:\Projects\dotnet-project-system-9433\classlib\build\Task.props' is newer (2024-04-12 11:01:28.399) than earliest output 'D:\Projects\dotnet-project-system-9433\classlib\bin\Debug\net8.0\classlib.dll' (2024-04-12 10:44:33.133), not up-to-date. See https://aka.ms/incremental-build-failure.

MattKotsenas avatar Apr 12 '24 18:04 MattKotsenas

Re-opening. We will need to track the package output file as well, and isolate package inputs/outputs separately (probably via FUTDC "sets").

The challenge here will be knowing when GeneratePackageOnBuild is set, given we force it to false for DTBs.

drewnoakes avatar Apr 12 '24 23:04 drewnoakes