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

Support executing plugins with invariant culture (as happens in docker) #7223

Closed
nkolev92 opened this issue Aug 22, 2018 · 4 comments
Closed
Assignees
Labels
Area:Authentication Area:Plugin V2 plugin w/ cross platform support Type:Bug
Milestone

Comments

@nkolev92
Copy link
Member

nkolev92 commented Aug 22, 2018

Create a basic repro image such as https://github.com/nkolev92/docker-images/blob/4501913d62526441e8bca40b9d01ba96821082aa/dotnetapp/Dockerfile.
When a restore is run with the plugin configured, the plugin exits after the handshake.

The reason why the plugin is terminated is a client side exception caused by https://github.com/NuGet/NuGet.Client/blob/62916a48fe61fc7d94c2f436c7d4569878705dd4/src/NuGet.Core/NuGet.Protocol/Plugins/PluginManager.cs#L358
thrown in https://github.com/NuGet/NuGet.Client/blob/62916a48fe61fc7d94c2f436c7d4569878705dd4/src/NuGet.Core/NuGet.Protocol/Plugins/Messages/InitializeRequest.cs.

The root cause of this is that the default culture for the dotnet docker images is invariant.

An invariant culture has a name that's an empty string, thus causing the above mentioned code to throw.
Docs https://docs.microsoft.com/en-us/dotnet/api/system.globalization.cultureinfo?view=netframework-4.7.2#culture-names-and-identifiers and https://docs.microsoft.com/en-us/dotnet/api/system.globalization.cultureinfo?view=netframework-4.7.2#invariant-neutral-and-specific-cultures.

The code as currently written simply cannot handle an invariant culture.

//cc @rrelyea @dtivel

@nkolev92
Copy link
Member Author

nkolev92 commented Aug 22, 2018

There are a couple of ways we can approach this.

Whatever approach we make, ideally we would be backward compatible and not break old plugins and old scenarios.

  1. Because NuGet and the plugin don't really do anything fancy with localization other than the language displayed and the invariant culture default will be English, we can specify EN-us if the culture is invariant.
    The advantage of this is that it's simple, and should have minimal repercussions. The disadvantage is that it's incomplete.

  2. We can rev up the protocol version and introduce a different InitialeRequest2 message.
    That message would have the LCID instead of the the culture name. https://docs.microsoft.com/en-us/dotnet/api/system.globalization.cultureinfo.lcid?redirectedfrom=MSDN&view=netframework-4.7.2#System_Globalization_CultureInfo_LCID
    This means we would not fix old plugins ofc. 4.8/2.1.400 would require that the container owners set the locale in order to make it work.

Personally I am ok with option 1, as it's simpler and things will just work.

//cc @NuGet/nuget-client, @rrelyea , @dtivel
I'd love to get your input on this.

//cc @loic-sharma as I know you're curious :)

@nkolev92 nkolev92 added Area:Authentication Area:Plugin V2 plugin w/ cross platform support labels Aug 22, 2018
@nkolev92 nkolev92 added this to the 4.9 milestone Aug 22, 2018
@nkolev92 nkolev92 self-assigned this Aug 22, 2018
@nkolev92 nkolev92 changed the title Docker plugins execution Executing plugins with invariant culture Aug 24, 2018
@nkolev92
Copy link
Member Author

We opted for the first approach.

@dtretyakov
Copy link

@nkolev92, thanks for fixing it. In which .NET CLI / NuGet versions it will be available?

@nkolev92
Copy link
Member Author

4.9 NuGet <=> 15.9 Visual Studio <=> 2.1.500 CLI version.

@rrelyea rrelyea changed the title Executing plugins with invariant culture Support executing plugins with invariant culture (as happens in docker) Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Authentication Area:Plugin V2 plugin w/ cross platform support Type:Bug
Projects
None yet
Development

No branches or pull requests

3 participants