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 with 3-character language codes are not copied to the output from NuGet packages #10898

Closed
NightOwl888 opened this issue Oct 28, 2024 · 6 comments

Comments

@NightOwl888
Copy link

Issue Description

When satellite assemblies are packaged in .nupkg files and then referenced by another project, none of the cultures with 3-character language codes are copied into the build output.

This is known to happen for build, publish, and pack as tool. There may be more cases, such as single file with embedded resources.

The behavior is the same in Visual Studio 2022 (Version 17.11.5) as well as the .NET SDK 8.0.403.

The following neutral cultures are known to be failing:

agq ars asa ast bas bem bez brx ccp cgg chr ckb dav dje dsb dua dyo ebu ewo fil fur gsw guz haw hsb jgo jmc kab kam kde kea khq kkj kln kok ksb ksf ksh lag lkt lrc luo luy mas mer mfe mgh mgo mua mzn naq nds nmg nnh nus nyn qu rof rwk sah saq sbp seh ses shi shi-Latn shi-Tfng smn teo twq tzm vai vai-Latn vai-Vaii vun wae xog yav zgh

Feature Request:

We have to patch this for all of the SDK versions in the wild. This is being done with our NuGet package using a buildTransitive .targets file. Please provide a way to detect when running on an SDK that you have fixed so we can skip our patch.

Steps to Reproduce

From the repro project: https://github.com/NightOwl888/IssueMSBuild3LetterLanguageCodes

  1. Build ProjectA. It will create a ProjectA.1.0.0.nupkg file in the repo root.
  2. Check the contents of ProjectA.1.0.0.nupkg. Both en and agq languages are present, as expected.
  3. Build ProjectB.
  4. Check the build output. Only en is present, agq is not.

Expected Behavior

MSBuild copies all .resources.dll files and their directory to the output.

Actual Behavior

MSBuild copies satellite assemblies for cultures with 2-letter language codes, but does not copy the ones for 3-letter language codes to the output.

Analysis

No response

Versions & Configurations

MSBuild version 17.11.9+a69bbaaf5 for .NET
17.11.9.46202


F:\Users\shad\source\repos\IssueMSBuild3LetterLanguageCodes>dotnet --version
8.0.403

This is on Windows 10 x64.

@baronfel
Copy link
Member

The problem seems to lie in how NuGet has created the project.assets.json. Here is the relevant section:

"targets": {
        "net8.0": {
            "ProjectA/1.0.0": {
                "type": "package",
                "compile": {
                    "lib/net8.0/ProjectA.dll": {}
                },
                "runtime": {
                    "lib/net8.0/ProjectA.dll": {}
                },
                "resource": {
                    "lib/net8.0/en/ProjectA.resources.dll": {
                        "locale": "en"
                    }
                }
            }
        }
    },

The SDK relies on the 'resource' key in this object to tag resources in the NuGet package with their locale - in this case the agq resource is not there. If you manually add the agq resource to the object like so:

"targets": {
        "net8.0": {
            "ProjectA/1.0.0": {
                "type": "package",
                "compile": {
                    "lib/net8.0/ProjectA.dll": {}
                },
                "runtime": {
                    "lib/net8.0/ProjectA.dll": {}
                },
                "resource": {
                    "lib/net8.0/en/ProjectA.resources.dll": {
                        "locale": "en"
                    },
                    "lib/net8.0/agq/ProjectA.resources.dll": {
                        "locale": "agq"
                    }
                }
            }
        }
    },

and then build or publish ProjectB with dotnet publish --no-restore -bl you can see that the file is recognized and copied to the publish directory as expected.

It turns out that this is a duplicate of an existing issue: NuGet/Home#12253. So I'm going to tag this appropriately and close this in favor of this issue.

@baronfel baronfel closed this as not planned Won't fix, can't repro, duplicate, stale Oct 28, 2024
@NightOwl888
Copy link
Author

After a bit of experimentation, it looks like this solution will work for publish only.

  • It doesn't copy the satellite assemblies to the build output for debugging, but I have found that updating the .deps.json file after the GenerateBuildDependencyFile target will make MSBuild copy the files.

  • I tested it with the single file deployment example (except changed to net8.0) and only en makes it to output there even though the obj/project.assets.json file was updated as you suggested.

  • PackAsTool also doesn't seem to work reliably with this approach, although it did work once and I am not sure what I was doing differently when it did.

We are running the patch for obj/project.assets.json after the GenerateBuildRuntimeConfigurationFiles target. Maybe there is another target that would work better? Or do PackAsTool and single-file deployment require another approach?

We need this to work 100% of the time regardless of how the end user is building their project. About 1/4 of the resource files we deploy have 3-letter language codes and it isn't obvious to our end users when they are missing.

Note that we also want our patch to respect SatelliteResourceLanguages, which is now how we are recommending users exclude resources from their distribution.

@baronfel
Copy link
Member

At this point there is a PR with a fix at the NuGet level (NuGet/NuGet.Client#6124) so the thing to do would be to get that merged and then either wait for the 9.0.200 SDK release early next year, or ask the NuGet team to backport the fix to the 9.0.100 releases, which requires them to gather justification and get approval.

@NightOwl888
Copy link
Author

Thanks.

Unfortunately, we don't control which SDK our users use and currently all supported .NET versions are broken in this way. Most of them may not even be completely aware that they depend on us through another dependency, so informing them to upgrade would probably need to be done through a build warning when you finally have a working SDK to upgrade to. Are you going to backport to .NET 8? Because I am sure requiring users to upgrade from an LTS to an STS SDK for their build is going to ruffle some feathers.

In the meantime, we will need a reliable way to patch the broken SDKs. Is it possible you could provide a shim .targets file for those who require a fix today for their users to be happy? Or document a complete workaround (even if that just means posting it here)?

@baronfel
Copy link
Member

Are you going to backport to .NET 8?

Your feedback is key to making this happen - do you have rough user numbers and/or any idea of which SDKs your users are using? At this point basically only .NET 8 SDKs are in support, so we wouldn't backport to 6.x or 7.x.

we will need a reliable way to patch the broken SDKs

Since this change is in NuGet, and inside the implementation of the Restore task, there is no easy way for you to patch the behavior.

@NightOwl888
Copy link
Author

do you have rough user numbers and/or any idea of which SDKs your users are using?

Which SDK versions, no.

Currently we are still in prerelease, so our numbers may be misleading. We are averaging 350 downloads per day, most through SaxonCS and Lucene.Net.ICU. Our project is gaining some momentum though, and it is possible we will be out of prerelease by the time your patch is available.

I am sure we are not the only project that is affected by this, and it is so subtle it is hard to notice so it may be under reported.

we wouldn't backport to 6.x or 7.x.

That is fine. We don't intend to support SDKs that you don't.

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

2 participants