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

.NET 9 SDK WPF build broken with AssemblyName in Directory.Build.props #10068

Open
candritzky opened this issue Nov 13, 2024 · 12 comments
Open
Assignees
Labels
Investigate Requires further investigation by the WPF team.

Comments

@candritzky
Copy link

candritzky commented Nov 13, 2024

Description

We have a pretty complex solution with a central Directory.Build.props file.
This Directory.Build.props file contains - among other stuff - a RootNamespace property that sets a default root namespace according to our conventions.

Something similar to this:

  <PropertyGroup>
    <RootNamespace>MyCompanyName.MyProductName.$(MSBuildProjectName)</RootNamespace>
    <AssemblyName>$(RootNamespace)</AssemblyName>
  </PropertyGroup>

The goal of this is that our .csproj files have short names without a MyCompanyName.MyProductName. prefix, but the namespaces and generated assemblies should have this prefix.

This worked since years up to .NET SDK 8.0.403. But it's broken since 2024-11-13 when Visual Studio 17.12 (which brings .NET SDK 9.0.100) was installed. It breaks WPF builds if the Directory.Build.props file sets a RootNamespace and/or a custom AssemblyName MSBuild property. It works if the property is moved to the WPF project's .csproj file. But it really needs to be moved, i.e. removed from the central Directory.Build.props file.

Something seems to be broken in the WPF XAML compile process.

As a workaround, I would have to comment out the <AssemblyName> property in Directory.Build.props, but this would break (or at least change) the output of all our other projects.

Reproduction Steps

Please see that attached ZIP file for a minimal repro.
Uncomment the <AssemblyName> tag in Directory.Build.props and the (clean) build will fail.

MyWpfApp.zip

Expected behavior

WPF apps should build without errors in .NET SDK 9 as they did in .NET SDK 8.

Actual behavior

Building with .NET SDK 9 is broken if there is a Directory.Build.props file with a custom <AssemblyName> property.

Regression?

Yes, it worked up to .NET SDK 8.0.403. Broken since 9.0.100.

Known Workarounds

No response

Impact

No response

Configuration

No response

Other information

No response

@zlatanov
Copy link

zlatanov commented Nov 14, 2024

I can reproduce the same issue as well. We use AssemblyName identically to how @candritzky explained.

WORKAROUND

Instead of overriding the AssemblyName in the csproj file, create a Directory.Build.props inside the WPF project and set the AssemblyName there.

<Project>
    <PropertyGroup>
        <AssemblyName>my-wpf-app</AssemblyName>
    </PropertyGroup>
</Project>

@zlatanov
Copy link

For the WPF team:

The task GenerateTemporaryTargetAssembly generates assembly with name which comes from the parent Directory.Build.props and not the one that is defined in the WPF csproj file.

@zlatanov
Copy link

zlatanov commented Nov 14, 2024

<IncludePackageReferencesDuringMarkupCompilation>false</IncludePackageReferencesDuringMarkupCompilation>

Also fixes the problem. Which means that something in ExecuteGenerateTemporaryTargetAssemblyWithPackageReferenceSupport method is different than ExecuteLegacyGenerateTemporaryTargetAssembly.

The only real difference I see is how AssemblyName is passed to BuildEngine.BuildProjectFile. In the legacy version it is passed as properties, whereas in the new version it is written to the temp csproj file, and as far as I can see it is written correctly, but the BuildEngine ignores it for some reason.

@candritzky
Copy link
Author

candritzky commented Nov 14, 2024

Instead of overriding the AssemblyName in the csproj file, create a Directory.Build.props inside the WPF project and set the AssemblyName there.

Thank you so much!

Just wanted to confirm that this workaround also works for me. I refined the solution a bit with an important of the ancestor Directory.Build.props file as shown below, so that I don't loose all my other global settings.

<Project>

  <Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$([System.IO.File.Path]::Combine('..', $(MSBuildThisFileDirectory)'))" />

  <PropertyGroup>
    <AssemblyName>my-wpf-app</AssemblyName>
  </PropertyGroup>
</Project>

@candritzky
Copy link
Author

false
Also fixes the problem. Which means that something in ExecuteGenerateTemporaryTargetAssemblyWithPackageReferenceSupport method is different than ExecuteLegacyGenerateTemporaryTargetAssembly.

Unfortunately this workaround does not work for me. It breaks other things in the build process.
For example, it breaks the Compile-time logging source generation feature because the source generator seems to be effectively disabled.

public static partial class Log
{
    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Critical,
        Message = "Could not open socket to `{HostName}`")]
    public static partial void CouldNotOpenSocket(
        ILogger logger, string hostName);
}

Results in

error CS8795: Partial method 'Log.CouldNotOpenSocket(ILogger, string)' must have an implementation part because it has accessibility modifiers.

@zlatanov
Copy link

Unfortunately this workaround does not work for me. It breaks other things in the build process.
For example, it breaks the Compile-time logging source generation feature because the source generator seems to be effectively disabled.

Yeah, I know. I just noted that, so the code owners have a starting point when trying to find out the root cause.

@Kuldeep-MS
Copy link
Member

@candritzky - I have been able to replicate the issue with .NET 8.0 as well. Upon investigation, I discovered that the problem stems from an incorrect resultant DLL name for the TemporaryTargetAssembly task. I confirmed that we are sending the correct assembly name for the temporaryTargetAssembly csproj, and found that the issue does not occur if we use RootNamespace and AssemblyName in the csproj file of the project instead of directory.build.props. In my opinion, the issue is related to MSBuild rather than WPF.

Additionally, regarding the use of IncludePackageReferencesDuringMarkupCompilation, this option sends globalProperties containing AssemblyName and other details to MSBuild instead of null. While I am unsure how MSBuild handles global properties, I suspect that the presence of global properties ensures the resultant DLL for TemporaryTargetAssembly has the correct name.

@Kuldeep-MS Kuldeep-MS added the 📭 waiting-author-feedback To request more information from author. label Nov 19, 2024
@candritzky
Copy link
Author

@Kuldeep-MS As described, the problem occurs for us only since the upgrade to Visual Studio 17.12 (which includes .NET 9 SDK). I could not see it with .NET 8 SDK. Of course, I can see it also when I target net8.0-windows, but use .NET 9 SDK to build.

You might be right that this is a MSBuild issue. I cannot tell.

What else do you need from me? Why did you tag this issue with "waiting-author-feedback"?

@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback To request more information from author. label Nov 19, 2024
@MichaeIDietrich
Copy link
Contributor

MichaeIDietrich commented Nov 19, 2024

Maybe to make the problem more clear, here are the temporary projects that are created (left: .NET 8, right: .NET 9):

Image

With #7557 only the first AssemblyName is preserved in the .csproj file and any others are removed.

My opinion on that, it's just luck that this worked in .NET 8. I don't really see a use case in setting the AssemblyName in Directory.Build.props and .csproj to different values.

The main problem seems to be again, that MSBuildProjectName cannot be safely used in a WPF projects due to its ugly markup compilation behavior using a temporary .csproj file. 😬

If there is really need to use MSBuildProjectName, then something like the following should better be used to drop the _wpftmp suffix when compiling the temporary .csproj file:

$([System.Text.RegularExpressions.Regex]::Replace($(MSbuildProjectName), '_[a-z0-9]+_wpftmp', ''))

So when you change the RootNamespace assignment in the Directory.Build.props to
<RootNamespace>MyCompanyName.MyProductName.$([System.Text.RegularExpressions.Regex]::Replace($(MSbuildProjectName), '_[a-z0-9]+_wpftmp', ''))</RootNamespace>
then it should work for .NET 8 and .NET 9.

@Kuldeep-MS
Copy link
Member

@rainersigwald - I would like to draw your attention to this issue. To give you more context, when building a WPF application, if we define the AssemblyName in Directory.Build.props using the $(MSBuildProjectName) variable. During the build process for the GenerateTemporaryTarget Assembly, WPF provides MSBuild with a .csproj file that has a resolved AssemblyName, where $(MSBuildProjectName) has already been replaced with its value. However, MSBuild ends up using the AssemblyName from the Directory.Build.props file, which still contains the $(MSBuildProjectName) variable, leading to a DLL name that differs from what is expected for the TemporaryTarget Assembly.

GenerateTemporaryTarget is an intermediate step for XAML compilation in WPF where a temporary project is created. However, the output DLL is expected to retain the original AssemblyName rather than using the temporary project's name.

Could you please have a look at this and share your insights on whether this issue lies with MSBuild, or if there is something that can be adjusted on the WPF side?

@candritzky
Copy link
Author

candritzky commented Nov 20, 2024

@MichaeIDietrich Thanks for your suggestion for stripping off the nasty _wpftmp suffix. So far (up to .NET 8 SDK), it was not needed. And I still don't see why this broke in .NET 9 SDK. Obviously the first AssemblyName property (at the beginning of the .csproj) file gets overwritten with some default. In .NET 8 SDK this was then "fixed" by redefining the property later again. This 2nd AssemblyName is now missing.

Regarding this:

I don't really see a use case in setting the AssemblyName in Directory.Build.props and .csproj to different values.

Our use case is the following:

  1. We have a large solution with a lot of projects and we want them to use namespace and assembly names that are prefixed with MyCompanyName.MyProductName. by default, without having to put this prefix onto each and every .csproj file. That's why we put this convention into the central Directory.Build.props file.
  2. The few executables that our solution produces, should have a different, shorter assembly name so that they are easier to handle from the command line (e.g. using tab completion), or to locate in the process list (Task Manager, attach debugger etc.). For this reason we override the (convention based) AssemblyName in the EXEs' .csproj files.

Does this make sense? Is there a better way to achieve the same result (without renaming all .csproj files to contain the MyCompanyName.MyProductName. prefix)?

@MichaeIDietrich
Copy link
Contributor

MichaeIDietrich commented Nov 20, 2024

@candritzky In fact, I think your use case is actually reasonable, so I am wrong about this.
Then the actual problem stays with using MSBuildProjectName in WPF projects.
I think we cannot override the value of it as it is a built-in property and changing the semantics seems to be the wrong path here.

But as a simple workaround we could introduce another property to the Microsoft.NET.Sdk that contains the expected project name and so there would be a simple way to fix such issues by replacing MSBuildProjectName with BetterMSBuildProjectName. (maybe someone could come up with a better name :D)

The implementation could look something like this:

  <PropertyGroup>
    <BetterMSBuildProjectName>$(_TargetAssemblyProjectName)</BetterMSBuildProjectName>
    <BetterMSBuildProjectName Condition=" '$(BetterMSBuildProjectName)' == '' ">$(MSBuildProjectName)</BetterMSBuildProjectName>
  </PropertyGroup>

This would work for all project types not only WPF.


As a mid term / long term change we could think about no longer creating a customized temporary .csproj file, but instead do this with the current .csproj file and inject the logic using MSBuild, in a similar fashion like inner builds work when a project has multiple TargetFrameworks defined. That way MSBuildProjectName would stay the same for inner temporary builds and the issue would no longer occur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Investigate Requires further investigation by the WPF team.
Projects
None yet
Development

No branches or pull requests

5 participants