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

Satellite assemblies are not produced for registered custom cultures when using .NET Core msbuild #1454

Closed
nguerrera opened this issue Dec 7, 2016 · 31 comments

Comments

@nguerrera
Copy link
Contributor

Moving from dotnet/sdk#387 (originally https://github.com/dotnet/cli/issues/4050) on behalf of @ryanbrandenburg.


Steps to reproduce

  1. Register a custom culture using something like sysglobl.dll and:
var car1 = new CultureAndRegionInfoBuilder("ru-US", CultureAndRegionModifiers.None);
car1.LoadDataFromCultureInfo(CultureInfo.CreateSpecificCulture("ru-RU"));
car1.LoadDataFromRegionInfo(new RegionInfo("en-US"));

car1.CultureEnglishName = "Russion (United States)";
car1.CultureNativeName = "русский (США)";
car1.CurrencyNativeName = "Доллар (США)";
car1.RegionNativeName = "США";

// Register the culture.
try
{
    car1.Register();
}
catch (InvalidOperationException)
{
    // Swallow the exception: the culture already is registered.
}
  1. Create a new ASP.NET Core Web Application (.NET Core)
  2. Create two resources, one named Resource.en-US.resx and one named Resource.ru-US.resx
  3. Build the project.
  4. Open bin/Debug/net451
  5. Observe that a en-US folder is present and has a dll and that ru-US is missing

Expected behavior

Any culture which will work with new CultureInfo("culture-NAME") (which custom cultures do once registered as above) should create a folder and dll which can be used in localization.

Actual behavior

Satellite assemblies are only created for original cultures, not for custom cultures.

Environment data

dotnet --info output:
.NET Command Line Tools (1.0.0-preview2-003121)

Product Information:
Version: 1.0.0-preview2-003121
Commit SHA-1 hash: 1e9d529

Runtime Environment:
OS Name: Windows
OS Version: 10.0.10586
OS Platform: Windows
RID: win10-x64

@nguerrera
Copy link
Contributor Author

nguerrera commented Dec 7, 2016

(Moving dotnet/sdk#387 (comment))

This will work fine if you build using desktop MSBuild / Visual Studio, but .NET Core MSBuild has this.

This can be worked around with a custom target that lets any culture through:

  <Target Name="AssignCustomCultures" AfterTargets="SplitResourcesByCulture">
    <ItemGroup>
      <EmbeddedResource Condition="$([System.IO.Path]::HasExtension(%(Filename)))">
        <Culture>$([System.IO.Path]::GetExtension(%(Filename)).TrimStart('.'))</Culture>
        <WithCulture>true</WithCulture>
      </EmbeddedResource>
    </ItemGroup>
  </Target>

This approach has the advantage that it is not dependent on what is registered on the build machine.

@nguerrera
Copy link
Contributor Author

@rainersigwald Thoughts? Should msbuild be more lenient about culture names to allow for custom cultures? I'm not too keen on relying on the build machine having registered a custom culture, but that does at least work with desktop msbuild. However, I don't know if .NET Core BCL provides enough to replicate that.

@rainersigwald
Copy link
Member

I remember talking about this when @cdmihai implemented the current mechanism but I don't know enough about the problem space to have a good opinion off the bat.

Speaking from the hip, it seems like we should either always allow arbitrary cultures or stick to some sort of list. I agree that registering the culture by changing OS state on the build machine is kind of an icky way to enable this.

@cdmihai
Copy link
Contributor

cdmihai commented Dec 8, 2016

On Full Framework MSBuild populates a list of all valid culture names via CultureInfo.GetCultures. That method does not exist on .Net Core, so we just use the hardcoded list FF gave us.

We hit this in MSBuild's own build because we have a resource named Strings.Shared.Resx. This caused a runtime crash with missing resources on .Net Core because the resource was assigned the "Shared" culture. It works on FF because FF successfully rejects "Shared" as a culture.

Here's one solution: accept everything on .net core, and introduce a ThisIsNotACulture metadata for EmbeddedResource which tells the AssignCulture task to interpret it as neutral.

@cdmihai
Copy link
Contributor

cdmihai commented Dec 8, 2016

P.S.: We went with the hardcoded list to mimic the FF behaviour as close as possible. If universal strings failed for our build, then it could fail for others too. Maybe we'll get the API back with ns2.0

@nguerrera
Copy link
Contributor Author

How about making the list a property or items that the user can amend.

@nguerrera
Copy link
Contributor Author

Btw, the first thing I tried was to explicitly give WithCulture=true, Culture=ru-US to a static item but AssignCulture wacks that, which is why my workaround evolved in to a target.

Another approach would be to respect explicit WithCulture metadata as implying that no check is required.

@cdmihai
Copy link
Contributor

cdmihai commented Dec 8, 2016

Yup, sounds like we have to implement something that respects current metadata to ignore / enforce file name based culture inference. Or as you said, look into extending the hardcoded list of cultures.

In terms of planning, when should this change go in? How often is it that users create their own custom cultures?

@nguerrera
Copy link
Contributor Author

I don't know about the usage but I suspect it's low. Given that there is a workaround of setting metadata in a custom rather, I'd think we could get away with vNext.

@adsurg
Copy link

adsurg commented Mar 13, 2017

Is there a bug here? GenerateSatelliteAssemblies only runs where MSBuildRuntimeType != Core but ComputeIntermediateSatelliteAssemblies which is dependent on it runs in all cases.

I've tried following the above bit am still hitting the same issue.

@adsurg
Copy link

adsurg commented Mar 13, 2017

@nguerrera, ping in case you're not still following this.

@adsurg
Copy link

adsurg commented Mar 13, 2017

It's an evil, hacky workaround, but dropping the line
<Target Name="ComputeIntermediateSatelliteAssemblies"></Target>
into the problem project file seems to get past this for now.

@nguerrera
Copy link
Contributor Author

nguerrera commented Mar 13, 2017

@adsurg It's not clear what you are working around. Can you open a new issue with repro steps and the precise failure that you're seeing. It is not clear that this is the same as the root issue, which is about custom cultures. There are targets that are core runtime only for satellite only because core msbuild doesn't support building satellites with its own targets, but anything that does not run on core has an alternate on full framework msbuild that does run.

@alberto-riggi
Copy link

@nguerrera
Hi Nick,
I tried with vs build and it doesn't generate the custom culture dll.
I tried the workaround you proposed. Adding that target on the xproj and it gives me
"Invalid static method invocation syntax:
"[System.IO.Path]::HasExtension()". Method 'System.IO.Path.HasExtension' not found. "

I think I am missing something. Do you have any idea?

@tarekgh
Copy link
Member

tarekgh commented Oct 16, 2018

This issue is not limited to the custom cultures but also to the cultures that can be created by the framework but not enumerated with CultureInfo.GetCultures. for example, on Windows 10, you can create any culture even if the OS doesn't have data for (something like yy-YY). also on Linux if using one of the aliased cultures.

@Forgind
Copy link
Member

Forgind commented May 25, 2020

Can you use the Copy task to move them right after they're generated?

@tarekgh
Copy link
Member

tarekgh commented May 25, 2020

to fix this, msbuild has to change the way depending on culture enumerated list and instead try to validate the culture by trying to create it. CultureInfo.GetCultureInfo can be used as it caches the previously created culture anyway.

@keab42
Copy link

keab42 commented Jun 8, 2020

Given that the issue has been present for 3 1/2 years, how likely is a fix at this point?

@marcoregueira
Copy link

Can you use the Copy task to move them right after they're generated?

Not really well. I can make it to work partially. But as far as I have seen, I need to add the task from nguerrera above, to every project that has those resources, then add the copy command to all projects in cascade, and a command for each dependency using project paths.

I made the test for a couple of projects in a solution and for some reason it worked for one of them but not for the other with identical configurations. No idea why, but the process seems so time and work consuming and prone to failure that doesn't seem a good idea to go ahead putting it to work.

Maybe I missed something, and is easier than it looks...

@DamianEdwards
Copy link
Member

Came across another case of folks running into this issue: https://docs.microsoft.com/en-us/answers/questions/611607/how-to-add-custom-culture-ex-en-mxenglish-mexico-i.html

I'd really like to see us do a focused push on tackling the issues folks are having in this area (localization in .NET Core) as it's definitely a recurring topic every release.

@DamianEdwards
Copy link
Member

DamianEdwards commented Nov 4, 2021

I've posted an example repro ASP.NET Core project that demonstrates this issue at https://github.com/DamianEdwards/CustomCultureExample

The repro site is running at https://dedward-customcultureissue.azurewebsites.net/ on Linux and it repros there too.

@BartoszCichecki
Copy link

Any news on this one?

@rainersigwald
Copy link
Member

There are some conflicting constraints that will keep us from getting to a perfect solution, but we can do better than requiring the workaround above.

The ideal solution would have no project-file impact, and a target would determine the culture for each .resx file.

The general pattern is ResourceFilename.culture.resx. But it's perfectly acceptable to have neutral resources in ResourceFilename.resx, and it's also ok for the name to include multiple segments/namespaces/whatever.

So if you have A.B.C.resx, it's ambiguous between:

  1. Resources for A.B.C, with no culture, and
  2. Resources for A.B in culture C.

The existing AssignCulture task uses a heuristic to disambiguate: if C is a valid culture, assume it's a culture. That's great unless you have resources with C# code in them in a Foo.cs.resx which looks like a valid alias for cs-CZ, so you can explicitly specify WithCulture="false" as of #5824 to explicitly say "don't detect a culture here, treat as neutral".

But there's no means to do the opposite and specify that "C is not a culture known to the system at build time, but it will be a culture registered and usable at runtime for this app".

I can see two options for that, not mutually exclusive:

  1. Extend AssignCulture to respect a manually-specified culture, and require custom culture resource files to manually specify that culture, for example
diff --git a/WebApplication116/CustomCultureWebApp.csproj b/WebApplication116/CustomCultureWebApp.csproj
index e0b8c8f..b99c807 100644
--- a/WebApplication116/CustomCultureWebApp.csproj
+++ b/WebApplication116/CustomCultureWebApp.csproj
@@ -18,4 +18,10 @@
     </AssemblyAttribute>
   </ItemGroup>
   </Target>
+
+  <ItemGroup>
+    <EmbeddedResource Update="Pages\Index.en-MX.resx">
+      <Culture>en-MX</Culture>
+    </EmbeddedResource>
+  </ItemGroup>
 </Project>
  1. Define a list of known custom cultures, and respect it in AssignCulture, for example
diff --git a/WebApplication116/CustomCultureWebApp.csproj b/WebApplication116/CustomCultureWebApp.csproj
index e0b8c8f..e1e7a6c 100644
--- a/WebApplication116/CustomCultureWebApp.csproj
+++ b/WebApplication116/CustomCultureWebApp.csproj
@@ -4,6 +4,7 @@
     <TargetFramework>net6.0</TargetFramework>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
+    <KnownCustomCultures>en-MX;ja-MX</KnownCustomCultures>
   </PropertyGroup>

I think doing both would have the best UX. The normal project diff could look like the latter there but enable more flexibility.

rainersigwald added a commit to rainersigwald/msbuild that referenced this issue May 9, 2023
@DamianEdwards
Copy link
Member

@rainersigwald these proposals look good. Curious though, what would be problematic about just checking whether the C in your example above (i.e. that last dot-separated segment before the .resx extension) is a semantically valid culture identifier, and if so, assume it's a culture, with the same workaround (WithCulture="false") for cases where it isn't?

@blanchardglen
Copy link

Would you accept a PR for this?
I could nearly match @rainersigwald example except in option 1 for backwards compatibility you would need to specify <WithCulture>true</WithCulture>

  <ItemGroup>
    <EmbeddedResource Update="Pages\Index.en-MX.resx">
      <Culture>en-MX</Culture>
+     <WithCulture>true</WithCulture>
    </EmbeddedResource>
  </ItemGroup>

By doing that the existing unit tests pass without changes and custom cultures are supported

<KnownCustomCultures> works great as well

@DaleCam
Copy link

DaleCam commented May 13, 2024

@danroth27 You mentioned I could ping you if somethings an issue for my organization that developing heavily with Blazor. This one is a blocking issue for a non profit application my organization built helping thousands of users in smaller cultures. It would help us immensely if this ticket could get some love. Thanks Dan!

@rainersigwald
Copy link
Member

@f-alizada this will be addressed by #10095 right?

@f-alizada
Copy link
Contributor

Recent PR that got merged: #10026 introduces the possibility to specify the property for the Task RespectAlreadyAssignedItemCulture. When set to true the metadata Culture recognized by the MSBuild.
however the default value of this property is false.
Currently work in progress to enable this by default for SDK.

@FatTigerWang
Copy link

FatTigerWang commented Jul 3, 2024

(Moving dotnet/sdk#387 (comment))

This will work fine if you build using desktop MSBuild / Visual Studio, but .NET Core MSBuild has this.

This can be worked around with a custom target that lets any culture through:

  <Target Name="AssignCustomCultures" AfterTargets="SplitResourcesByCulture">
    <ItemGroup>
      <EmbeddedResource Condition="$([System.IO.Path]::HasExtension(%(Filename)))">
        <Culture>$([System.IO.Path]::GetExtension(%(Filename)).TrimStart('.'))</Culture>
        <WithCulture>true</WithCulture>
      </EmbeddedResource>
    </ItemGroup>
  </Target>

This approach has the advantage that it is not dependent on what is registered on the build machine.

Using this method will cause ManifestResourceName to generate unexpected names in .NET 8 and .NET Framework, for example:

Controllers.WeatherForecastController.en-CN.resx => Controllers.WeatherForecastController.en-CN.en-CN.resx

It is speculated that this is because MSBUILD cannot distinguish whether en-CN is ordinary characters or Culture.

Therefore, you can use <LogicalName> to re-specify ManifestResourceName

<Target Name="AssignCustomCultures" AfterTargets="SplitResourcesByCulture">
  <ItemGroup>
    <EmbeddedResource Condition="'%(Filename)%(Extension)' == 'Controllers.WeatherForecastController.en-CN.resx'">
      <Culture>$([System.IO.Path]::GetExtension(%(Filename)).TrimStart('.'))</Culture>
      <WithCulture>true</WithCulture>
      <LogicalName>$(AssemblyName).Resources.Controllers.WeatherForecastController.$([System.IO.Path]::GetExtension(%(Filename)).TrimStart('.')).resources</LogicalName>
    </EmbeddedResource>
  </ItemGroup>
</Target>

@JanKrivanek
Copy link
Member

Fixed by dotnet/sdk#41042

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests