NB.GV defeats incremental build for .NET SDK style WPF projects

[jviau]

Nerdbank gitversion is incompatible with WPF targets and SDK projects. Either ThisAssembly will be inaccessible due to protection level, or the generation of Version.cs in the WPF temp project breaks incremental build.

From my investigation, I believe 3 things are needed:

1. Make GenerateAssemblyVersionInfo run before MarkupCompilePass1
2. Do not run GenerateAssemblyVersionInfo inside the WPF temporary project.
3. Re-include the Version.cs generated from the original project in the WPF temporary project.

See https://devdiv.visualstudio.com/DevDiv/VS%20IDE%20CPS/_git/CPS/pullrequest/120052?_a=overview for the workarounds I performed to get this to work.

[AArnott]

I think the first thing that needs to be fixed for this scenario to work is NuGet/Home#5894. Without PackageReference working properly for WPF projects, it's going to fail regardless of what we do in the NB.GV package.

[jviau]

I opened dotnet/msbuild#3254 for that issue.

They both need to be fixed IMO, order doesn't matter. In my experience it was much harder to discover and solve this NB.GV issue than the WPF import issue. That one was relatively simple. WPF tmp_proj gets the same $(AssemblyName) as the original project, and my assembly names match the project name. So I used that to build the imports in Directory.Build.props/targets when I am in a WPF project - which can be detected by testing for '.tmp_proj'.

[AArnott]

They both need to be fixed IMO

No argument there. Just that it's a low priority for me to fix an issue in a scenario Microsoft doesn't support, whose workaround is not straightforward. I'm still investigating, but at this stage I'm just trying to get a repro (with the workaround applied) so I can see the problem. And the nuget bug makes it difficult.

[AArnott]

OK, so I've finished applying a workaround for the WPF nuget import issue. After that, NB.GV works. This matches the experience I have in the VS-Platform repo where we have several such WPF projects that use NB.GV.
I don't understand the claim that it's incompatible.

You mentioned an incremental build regression with NB.GV. Is that really the only issue?

[jviau]

It is the incremental build issue. MarkupCompilePass1 has an incremental build step that checks the timestamp of all input files. Version.cs is the one that always fails that check. The WPF temp project will touch the version.cs and version.cs.new file, and then the real project will touch it again after. So every build causes a full build, as MarkupCompilePass1 will return true for needing to compile main assembly. Disabling generation of this file within WPF isn't sufficient either, as then it won't exist until after WPF runs, causing build errors.

[AArnott]

I create version.cs.new first and compare it with version.cs before overwriting version.cs specifically to keep incremental build working. How does touching version.cs.new break incremental build, seeing as it's not a compile input? And I don't touch version.cs at all unless it changed.

[jviau]

I updated my comment above. I debugged into MarkupCompilePass1, and its incremental build check always failed when checking the timestamp of Version.cs. The fix I linked of ensuring Version.cs is created before wpf targets run, and then not generating that file within the wpf temp proj resolved it for me.

[clairernovotny]

@AArnott FWIW, the SDK Extras 1.5.4 includes a fix for the WPF imports stuff based on a new variable they set.

[KirillOsenkov]

Related discussion in #642 (comment)

[AArnott]

The problem is the WPF inner build and outer build define different root namespace, so the generated assemblyinfo file is different from inner to outer builds:

31c31
<     internal const string RootNamespace = "fix642_clrmgcrv_wpftmp";
---
>     internal const string RootNamespace = "fix642";

So a workaround would be to set RootNamespace in the WPF project file explicitly so it doesn't inherit from the project filename.
This should be fixed with dotnet/wpf#5697, but for folks using older SDKs, NB.GV may be able to do something clever here.

[KirillOsenkov]

Ah, interesting! Thanks for digging in!

[KirillOsenkov]

Confirmed setting RootNamespace fixes the incremental build. Thanks again!