-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[csharp] Fix error loading library grpc_csharp_ext.*.dll on windows with non-ASCII encoding #26762
Conversation
…Windows with charset on non ANSI encoding. Example library path: C:\Program Files (x86)\тест\grpc_csharp_ext.x64.dll
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once all the tests pass.
@apolcyn can you please add another LGTM? |
string.Join(",", libraryPathAlternatives))); | ||
} | ||
|
||
private static class Windows | ||
{ | ||
[DllImport("kernel32.dll")] | ||
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Net Standard not supported CharSet.Auto, CharSet.Unicode supported all .net versions
https://docs.microsoft.com/ru-ru/dotnet/api/system.runtime.interopservices.charset?view=netstandard-1.6
Is there anything else i need to do, to complete my first pull request to merge? Sorry for my english |
Nothing is needed from you, we're just running some tests and if they all look good, we'll merge. |
Getting a good run of the macos artifact build is proving difficult and without that it's not possible to get a good distribtest run. I'm going to merge and check that distribtests are green afterwards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
FTR I checked the distribtests results after merging and everything seemed green. |
…ith non-ASCII encoding (grpc#26762) * [csharp net472] Fix error loading library grpc_csharp_ext.x64.dll on Windows with charset on non ANSI encoding. Example library path: C:\Program Files (x86)\тест\grpc_csharp_ext.x64.dll * cleanup * Change charset to Unicode Co-authored-by: Jan Tattermusch <jtattermusch@google.com>
Fix error loading library grpc_csharp_ext.x64.dll on desktop Windows application with charset on non ANSI encoding. Example library path: C:\тест\grpc_csharp_ext.x64.dll
lang/csharp
.NET Framework 4.7.2
@nicolasnoble