FUTDC doesn't check packed items when GeneratePackageOnBuild is true

MattKotsenas commented 3 months ago

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

dotnet new classlib
Create a file build/Task.props (content doesn't matter)
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>

Build in Visual Studio
Notice that .nupkg is created
Edit Task.props so it has different content (actual content doesn't matter)
Build in Visual Studio
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

JanKrivanek commented 3 months ago

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

drewnoakes commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

MattKotsenas commented 2 months ago

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.

drewnoakes commented 2 months ago

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.

dotnet / project-system