sdk icon indicating copy to clipboard operation
sdk copied to clipboard

ResolvePackageAssets target is skipped for multiple target frameworks and Rebuild target in 17.2.0

Open alexandersorokin opened this issue 3 years ago • 5 comments

Issue Description

The ResolvePackageAssets target is skipped if three conditions are met:

  1. The feature "multiple target frameworks" is used in csproj;
  2. The Rebuild target is selected;
  3. The version of msbuild is 17.2.0 or newer.

That results in skipping code generation from source-only nuget packages with pp-files. That results in failed rebuilds.

The Build target runs the ResolvePackageAssets target in such conditions. Also, some older versions of the msbuild run the ResolvePackageAssets target is such conditions.

Steps to Reproduce

  1. Create an ClassLibrary1.csproj file in an empty folder with the following content:
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netstandard2.0</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="StackTraceParser.Source" Version="1.3.1" />
  </ItemGroup>
</Project>
  1. Add an cs-file to the folder:
namespace ClassLibrary1
{
    public class GeneratedFileContainer
    {
        StackTraceParser x;
    }
}
  1. Run dotnet build /t:rebuild in the folder.

Expected Behavior

Build succeeded.

Actual Behavior

Build FAILED.

d:\work\CheckNullaRebuild\ClassLibrary1\GeneratedFileContainer.cs(5,9): error CS0246: The type or namespace name 'StackTraceParser' could not be found (are you missing a using directive or an assembly reference?)

Analysis

dotnet build /t:build finishes successfully. So there is an issue the Rebuilt target.

The type StackTraceParser is generated by the ResolvePackageAssets target from pp-file. See https://github.com/atifaziz/StackTraceParser/blob/master/StackTraceParser.csproj#L66

The reason why the ResolvePackageAssets target is skipped during the Rebuild target run is value of the _CleaningWithoutRebuilding parameter. In order for the target to be run, the parameter should not have a true value.

If the selected target is Build, the _CleaningWithoutRebuilding parameter is undefined. So the ResolvePackageAssets target is not skipped.

If the feature "multiple target frameworks" is not used in csproj and the selected target is Rebuild, the _CleaningWithoutRebuilding parameter is set to true by the _SdkBeforeClean target and then set to false by the_SdkBeforeRebuild target. So the ResolvePackageAssets target is not skipped.

If the feature "multiple target frameworks" is used in csproj and the selected target is Rebuild, the _CleaningWithoutRebuilding parameter is set to true by the _SdkBeforeClean target. But it is never set to false because the DispatchToInnerBuilds runs the Build target instead of the Rebuild target for each target framework. So the _CleaningWithoutRebuilding parameter value remains true and the ResolvePackageAssets target is not run.

Versions & Configurations

d:\work\CheckNullaRebuild\ClassLibrary1>dotnet msbuild --version
Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

17.2.0.20702

alexandersorokin avatar May 18 '22 09:05 alexandersorokin

I tried this with 17.2.0.17804, and dotnet build /t:Rebuild failed with the exception above, but so did dotnet build /t:Build and dotnet build. I looked at binlogs for both cases, and it seemed to be building the ResolvePackageAssets target, but its build time was unusually short for the Rebuild case, so I'm a little suspicious about that.

Forgind avatar May 19 '22 20:05 Forgind

Thank you for trying to reproduce the bug!

The reason why you get the same exception in all three cases can be the difference between the namespace of the generated StackTraceParser class and the namespace of the GeneratedFileContainer class.

The StackTraceParser class is generated with the root namespace of a project. Could you try renaming csproj-file to ClassLibrary1.csproj or add using to cs-file and then try again?


I have edited the original Steps to Reproduce. So csproj-file now has name ClassLibrary1.csproj.

Also I have attached ClassLibrary1.zip generated by the new Steps to Reproduce block with the project name fix. It produces the following log:

d:\work\CheckNullaRebuild\ClassLibrary1>dotnet build
Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored d:\work\CheckNullaRebuild\ClassLibrary1\ClassLibrary1.csproj (in 163 ms).
d:\work\CheckNullaRebuild\ClassLibrary1\1.cs(5,26): warning CS0169: The field 'GeneratedFileContainer.x' is never used
[d:\work\CheckNullaRebuild\ClassLibrary1\ClassLibrary1.csproj]
  ClassLibrary1 -> d:\work\CheckNullaRebuild\ClassLibrary1\bin\Debug\netstandard2.0\ClassLibrary1.dll

Build succeeded.

d:\work\CheckNullaRebuild\ClassLibrary1\1.cs(5,26): warning CS0169: The field 'GeneratedFileContainer.x' is never used
[d:\work\CheckNullaRebuild\ClassLibrary1\ClassLibrary1.csproj]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.32

d:\work\CheckNullaRebuild\ClassLibrary1>dotnet msbuild /t:build
Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  ClassLibrary1 -> d:\work\CheckNullaRebuild\ClassLibrary1\bin\Debug\netstandard2.0\ClassLibrary1.dll

d:\work\CheckNullaRebuild\ClassLibrary1>dotnet msbuild /t:rebuild
Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

d:\work\CheckNullaRebuild\ClassLibrary1\1.cs(5,9): error CS0246: The type or namespace name 'StackTraceParser' could no
t be found (are you missing a using directive or an assembly reference?) [d:\work\CheckNullaRebuild\ClassLibrary1\Class
Library1.csproj]

ClassLibrary1.zip

alexandersorokin avatar May 19 '22 21:05 alexandersorokin

It looks like in the success case, we're passing on extra parameter to the csc task: obj\Debug\netstandard2.0\NuGet\D219894B6304F8B67F0125410EAFB780514626B1\StackTraceParser.Source\1.3.1\StackTraceParser/StackTraceParser.cs

Looking at roslyn/Microsoft.CSharp.Core.targets at 079474b518dc3684408b1d3f848113d901b0b0c9 · dotnet/roslyn (github.com), it actually sounds like csc calculates its own command line from information we give it, including the Compile item.

Looking further, the issue behind the wrong command line seems to be that the Compile Item doesn't get StackTraceParser.cs, apparently because the RunProduceContentAssets target (defined here) doesn't execute because its condition is false.

That happened because _CleaningWithoutRebuilding was true. We set it to true in the _SdkBeforeClean then false again in the _SdkBeforeRebuild. _SdkBeforeClean is in CleanDependsOn, so it executes before Clean. _SdkBeforeRebuild similarly executes just before the Rebuild target.

The problem is that Rebuild executes after both Clean and Build. That means _SdkBeforeRebuild isn't "needed" until after Build and Clean complete, except the RunProduceContentAssets target then knows we're cleaning but doesn't realize we're rebuilding, causing all the above problems.

The correct solution to this is not immediately obvious to me. We can't move Rebuild before Build. We don't have a built-in way to say that if a target will eventually execute that we should execute it at a particular point. It's possible we could change setting _CleaningWithoutRebuilding in _SdkBeforeClean to take into consideration whether we're rebuilding vs. just cleaning? That's my best idea, though I don't love it, since I don't think Clean should know that.

Forgind avatar May 26 '22 18:05 Forgind

Great repro, by the way, @alexandersorokin. Very helpful in pointing me to the underlying issue.

Forgind avatar May 26 '22 19:05 Forgind

MSBuild Team Triage: The targets for this issue don't exist in the msbuild repo. The flags that control the build ordering seem to exist in the SDK repo: _SdkBeforeRebuild _SdkBeforeClean , etc. Moving to the sdk repo

benvillalobos avatar Jul 28 '22 16:07 benvillalobos