Msbuild: Support resource naming based on DependentUpon source by convention

Created on 3 Jul 2019  路  14Comments  路  Source: dotnet/msbuild

Related issues:

https://github.com/dotnet/winforms/issues/638
https://github.com/dotnet/project-system/issues/4807
https://github.com/dotnet/corefx/issues/34190

In SDK projects and new project system, we have eliminated the need for DependentUpon to make the appropriate file nesting in the IDE tree. However, there is a place where the build actually uses DependentUpon to:

  1. Locate a source file, and _parse (!)_ it to get first class name and namespace
  2. Generate .resources accordingly.

Let first me say that this whole design is terrible and it is absolutely insane that we have code to find the class and namespace name in msbuild:

https://github.com/microsoft/msbuild/blob/f309171ed821eb66cb635403e1fbff41ea22ae9c/src/Tasks/CSharpParserUtilities.cs#L15-L20

But people and features in VS rely on this and it has been a consistent source of feedback in moving to .NET Core 3.0. So I give up and propose:

  1. Add a boolean property that, when true, causes us to look for the source file by convention when DependentUpon metadata is not found.

  2. Set that property by default for .NET Core App 3.0 TFM.

cc @rainersigwald @davkean @livarcocc

.NET Core Tasks

All 14 comments

Proposed property name: EmbeddedResourceUseDependentUponConvention

Proposed new argument to CreateManifestResourceName: UseDependentUponConvention

The new code would go after this if it didn't return anything (so explicit in-the-project metadata should continue to win):
https://github.com/microsoft/msbuild/blob/07d3c25a1461dfba3fcc1cc9b28cb8accd4e32b9/src/Tasks/CreateManifestResourceName.cs#L143

Sounds good.

@BenVillalobos can you take a look at this once you're feeling better?

Let first me say that this whole design is terrible and it is absolutely insane that we have code to find the class and namespace name in msbuild

This was for compatibility with the pre-MSBuild behavior, FWIW (build was only supported within devenv, where the IDE could supply the information).

That's worth a lot. Fascinating.

I just hit this while porting EF6 to .NET Core. We drop two files into the project:

  • Migrations\201907170000000_MigrationName.cs // Contains a class named MigrationName
  • Migrations\201907170000000_MigrationName.resx

The resource gets embedded as RootNamespace.Migrations.201907170000000_MigrationName.resources instead of just RootNamespace.Migrations.MigrationName.resources causing ResourceManager to throw.

The work around, of course, is to explicitly set the item's DependentUpon metadata:

<EmbeddedResource Update="Migrations\201907170000000_MigrationName.resx">
  <DependentUpon>201907170000000_MigrationName.cs</DependentUpon>
</EmbeddedResource>

cc @ajcvickers @divega @Pilchie

  1. Set that property by default for .NET Core App 3.0 TFM.

@nguerrera should this apply to .NET Standard 2.1 TFM as well?

Yes. Good catch.

Shouldn't this be turned on almost always with new format?

I'm hesitant to make a breaking change here.

Maybe turned off for known existing TFMs but on by default so that it is picked up by .NET Core 3.0 and up, .NET Standard 2.1 and up, new Mono versions, etc?

@BenVillalobos This is fixed, right? It still doesn't appear to be working in 3.0-preview9/16.3-preview3. https://github.com/aspnet/EntityFramework6/issues/1225

@bricelam It's fixed . . . unless the resx file is in a folder: #4695. Your workaround in the linked bug is good.

Now that the rules for manifest resource name selection have changed, refer to this dotnet docs issue for a detailed explanation on how this selection works now.

Was this page helpful?
0 / 5 - 0 ratings