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

FUTDC doesn't check packed items when GeneratePackageOnBuild is true #9433

Open
MattKotsenas opened this issue Mar 30, 2024 · 6 comments · Fixed by #9437
Open

FUTDC doesn't check packed items when GeneratePackageOnBuild is true #9433

MattKotsenas opened this issue Mar 30, 2024 · 6 comments · Fixed by #9437
Assignees
Labels
Bug This is a functional issue in already written code. Feature-Up-to-date Build up-to-date check that avoids shelling out to MSBuild unless necessary. Triage-Approved Reviewed and prioritized
Milestone

Comments

@MattKotsenas
Copy link

MattKotsenas commented Mar 30, 2024

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

@JanKrivanek
Copy link
Member

Looks like another case of #9001

@JanKrivanek JanKrivanek transferred this issue from dotnet/msbuild Apr 2, 2024
@drewnoakes drewnoakes added Bug This is a functional issue in already written code. Feature-Up-to-date Build up-to-date check that avoids shelling out to MSBuild unless necessary. labels Apr 4, 2024
@drewnoakes
Copy link
Member

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 drewnoakes changed the title [Broken Build]: GeneratePackageOnBuild doesn't rebuild when only content files change (FUTDC issue?) FUTDC doesn't check packed items when GeneratePackageOnBuild is true Apr 4, 2024
@drewnoakes drewnoakes self-assigned this Apr 4, 2024
@drewnoakes
Copy link
Member

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):

// Never generate NuGet package during design time build.
_properties = Task.FromResult<IImmutableDictionary<string, string>>(Empty.PropertiesMap.Add(ConfigurationGeneralBrowseObject.GeneratePackageOnBuildProperty, "false"));

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

@drewnoakes
Copy link
Member

Fixed in #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 drewnoakes added this to the 17.11 milestone Apr 5, 2024
@MattKotsenas
Copy link
Author

MattKotsenas commented Apr 12, 2024

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
Copy link
Member

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 drewnoakes reopened this Apr 12, 2024
@tmeschter tmeschter added the Triage-Approved Reviewed and prioritized label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a functional issue in already written code. Feature-Up-to-date Build up-to-date check that avoids shelling out to MSBuild unless necessary. Triage-Approved Reviewed and prioritized
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants