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

RootNamespace should be set explicitly in WPF inner builds #5697

Closed
AArnott opened this issue Nov 16, 2021 · 3 comments · Fixed by #6535
Closed

RootNamespace should be set explicitly in WPF inner builds #5697

AArnott opened this issue Nov 16, 2021 · 3 comments · Fixed by #6535
Assignees
Labels
Bug Product bug (most likely) 🚧 work in progress

Comments

@AArnott
Copy link

AArnott commented Nov 16, 2021

  • .NET Core Version: 6.0.100
  • Windows version: Windows 11 (21H2, 22000.318)
  • Does the bug reproduce also in WPF for .NET Framework 4.8?: Yes/No
  • Is this bug related specifically to tooling in Visual Studio (e.g. XAML Designer, Code editing, etc...)? No

Problem description:

A typical WPF project file does not explicitly set the RootNamespace MSBuild property but rather inherits it from the $(MSBuildProjectName).

WPF's inner build creates a temporary project file with a random suffix added to the project name. This means that the default RootNamespace changes to not match the actual RootNamespace from the outer build.
Two problems with that:

  1. The RootNamespace in the inner build does not match the value used by the outer build. This may not be a customer facing problem if the inner build does not ever include targets that care what that property is set to. EmbeddedResource items are embedded using logical names based on RootNamespace, so as one example if that step is done within an inner build, then the inner build has a corrupted result.
  2. The RootNamespace changes to a new random value with each and every build. This breaks incremental build and forces WPF's inner build to actually compile the result with each and every build (with no source changes) when Nerdbank.GitVersioning is installed in the project, since this property serves as an input to a generated source file.

See #5458 for other work targeting improving incremental build performance.

Actual behavior:

The RootNamespace property is not set in the generated tmpproj.csproj file.

Expected behavior:

If RootNamespace is explicitly set in the project file, its definition should be copied into the tmpproj
If RootNamespace is not explicitly set in the project file, the generated tmpproj should set it explicitly to $(_TargetAssemblyProjectName).

For example we were able to workaround the problem by adding this to our Directory.Build.targets file:

  <!-- Incremental build perf improvements -->
  <PropertyGroup>
    <!-- Fix RootNamespace in WPF inner builds so they are constant across builds. -->
    <RootNamespace Condition=" $(RootNamespace.EndsWith('_wpftmp')) ">$(_TargetAssemblyProjectName)</RootNamespace>
  </PropertyGroup>
@Kuldeep-MS
Copy link
Member

@AArnott - I have added a fix here. Can you please give it a try to see if the issue is fixed or not?

@AArnott
Copy link
Author

AArnott commented Mar 7, 2023

I just tested with 7.0.300-preview.23122.5, and only part of the original problem repros.

If RootNamespace is explicitly set in the project file, its definition should be copied into the tmpproj

The above now works.

But this part:

If RootNamespace is not explicitly set in the project file, the generated tmpproj should set it explicitly to $(_TargetAssemblyProjectName).

is not yet done.

Here is my repro project:
wpftest.zip

Just run dotnet build repeatedly inside it. You can tell the problem exists because you see 2 compilation warnings every time you dotnet build. But if you add a RootNamespace property definition to the project file, dotnet build only emits the 2 warnings the first time you run it. On subsequent runs no warnings are emitted. This demonstrates how incremental build is broken by default for WPF projects (when NB.GV is installed anyway) because the RootNamespace keeps changing.

@Kuldeep-MS thank you for the proposed fix. I don't know how to apply your PR build to my local machine (and sadly, the PR build has been deleted by now anyway). But hopefully the repro project and instructions will help you to validate.

@Kuldeep-MS
Copy link
Member

@AArnott - I ran dotnet build on the repro project you provided with my fix enabled and I can see the project builds fine with two warnings every time. Looks like the issue is still not resolved.
image
(see image).
Thanks for the repro.

Reg: Applying fix to your local machine, Please follow the following steps

  1. Build the WPF having the Changes in Fixing the AssemblyName in temporary project file for GenerateTemporaryTargetAssembly task #7557
  2. Replace the Binaries at C:\Program Files\dotnet\sdk\<your sdk version>\Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472 with <your WPF clone Directory>\wpf\artifacts\packaging\Debug\Microsoft.NET.Sdk.WindowsDesktop.Debug\tools\net472

@ghost ghost added the 🚧 work in progress label Aug 1, 2023
@ThomasGoulet73 ThomasGoulet73 added Bug Product bug (most likely) and removed Investigate Requires further investigation by the WPF team. labels Aug 1, 2023
@ghost ghost removed the 🚧 work in progress label Aug 21, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Product bug (most likely) 🚧 work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants