Runtime: System.Private.CoreLib assembly missing `NeutralResourcesLanguage("en-US")`

[ericstj]

See https://github.com/dotnet/coreclr/issues/26472#issuecomment-536176848

Appears to be a regression in 3.0. Not sure if this is intentional or not, but it does seem to be observable.

/cc @tarekgh

[tarekgh]

This looks to me a regression from the tools. in 2.x we used to have

mscorlib\Tools\Versioning\GenerateVersionInfo.targets:      <AssemblyInfoLines Include="[assembly:System.Resources.NeutralResourcesLanguage(&quot;en-US&quot;)]" />      

I think this attribute is important to have for msbuild.

Note, this issue in not specific to System.Private.Corelib but it looks a generic for all corefx assemblies.

[ericstj]

In 2.x we did this with https://github.com/dotnet/buildtools/blob/6736870b84e06b75e7df32bb84d442db1b2afa10/src/Microsoft.DotNet.Build.Tasks/PackageFiles/AssemblyInfoPartial.cs, arcade doesn't have this.

@tarekgh do you think this is important for 3.1?

[tarekgh]

do you think this is important for 3.1?

My understanding is msbuild can fail when not finding this attribute in GenerateResource target. At least this happened on the desktop builds. We need msbuild guys confirm that as I believe it is important to fix it in 3.1.

[danmosemsft]

If it's important to fix in 3.1, should it get a fix in 3.0? (I don't have enough context to know whether it meets the higher 3.0 bar)

[tarekgh]

CC @rainersigwald as if he may know exactly what scenarios maybe broken of the missing attribute.

I recall one scenario was related to the .Net Native builds when msbuild was trying to merge the framework resources into the PRI file and generate a warning for missing this attribute.

[rainersigwald]

That sounds like

MSB3817: The assembly "{0}" does not have a NeutralResourcesLanguageAttribute on it. To be used in an app package, portable libraries must define a NeutralResourcesLanguageAttribute on their main assembly (ie, the one containing code, not a satellite assembly).

That's the only obvious error I see, and it looks like it applies only in the extract-resources-from-assembly case, which isn't enabled on .NET Core.

Based on that, I'm more worried about the runtime failure mentioned in the linked OP in

C# ResourceManager.GetNeutralResourcesLanguage(typeof(object).Assembly)

[tarekgh]

GetNeutralResourcesLanguage is a protected method so it will be interesting only to whoever subclass ResourceManager which is I believe is not a common case. Also, I couldn't find any code inside our libraries calling this method.

The code has a comment saying this is obsolete but unfortunately the public docs is not saying so.

C# // This method should be obsolete - replace it with the one below. // Unfortunately, we made it protected.

based on this, I would say let's fix it for 3.1 and wait if anyone wanted it for 3.0 so we can service it at that time.

[buyaa-n]

@tarekgh is this fixed in 3.1? Or do we have to fix this for 5.0?

[ericstj]

We should first fix in 5.0 then backport via servicing if we need it. I checked and we're still missing this attribute in 5.0. I would expect we need a louder/stronger ask to do this in servicing. This issue was opened because a single report of a testcase breaking.

[ericstj]

I think you can fix this by adding an entry here:
https://github.com/dotnet/runtime/blob/200b197559fc49f91b8db3f20fd6df6b5660d7bf/eng/versioning.targets#L56

Something like:

      <AssemblyAttribute Include="System.Resources.NeutralResourcesLanguage">
        <_Parameter1>en-US</_Parameter1>
      </AssemblyAttribute>

[tarekgh]

I checked and I am seeing this is not fixed either in 3.x nor 5.0. I agree we should fix it for 5.0.

@buyaa-n do you want to submit a PR for that? U can do it if you don't have cycles.

[buyaa-n]

Thanks for looking @ericstj , @tarekgh, unfortunately i am a bit tight with the analyzers, please fix it if you have capacity

[tarekgh]

I'll try to get this soon as @ericstj suggestion should be simple.

[tarekgh]

@ericstj I tried your suggestion and worked very well. The only thing is it didn't work for the corelib. do you know how we can add this attribute there? I hope we'll not need any change from the build tools.

[ericstj]

CoreLib doesn't include Versioning.targets it seems. Only libraries build does. It looks like SPC had decided to define such attributes directly:
https://github.com/dotnet/runtime/blob/master/src/coreclr/src/System.Private.CoreLib/src/System/Internal.cs
https://github.com/dotnet/runtime/blob/master/src/mono/netcore/System.Private.CoreLib/src/System/Internal.cs

So I'd imagine we just add to those files.