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

Proper fix utf8 command line arguments (#14253) #15519

Merged
1 commit merged into from
Jan 20, 2024
Merged

Proper fix utf8 command line arguments (#14253) #15519

1 commit merged into from
Jan 20, 2024

Conversation

copybara-service[bot]
Copy link

Proper fix utf8 command line arguments (#14253)

#14197
Tried to fix utf-8 issue, but it didnt handle multibyte chars.
Only way I found that works constantly is using CommandLineToArgvW.
To not ripple out wchar_t, I convert to and from where needed

Closes #14253

COPYBARA_INTEGRATE_REVIEW=#14253 from hknielsen:proper-fix-none-ascii-issue cad753e
FUTURE_COPYBARA_INTEGRATE_REVIEW=#14253 from hknielsen:proper-fix-none-ascii-issue cad753e

#14197
Tried to fix utf-8 issue, but it didnt handle multibyte chars.
Only way I found that works constantly is using `CommandLineToArgvW`.
To not ripple out `wchar_t`, I convert to and from where needed

Closes #14253

COPYBARA_INTEGRATE_REVIEW=#14253 from hknielsen:proper-fix-none-ascii-issue cad753e
PiperOrigin-RevId: 599990369
@copybara-service copybara-service bot closed this pull request by merging all changes into main in 1eff9d7 Jan 20, 2024
@copybara-service copybara-service bot deleted the test_599826579 branch January 20, 2024 02:47
copybara-service bot pushed a commit that referenced this pull request Aug 29, 2024
This relates to:

Issues:
- #17036
- grpc/grpc#36518
PRs:
- #15519

The fix in #15519 has one line missing that breaks `Grpc.Tools` and doesn't actually fix the problem it proports to fix.

This PR fixes this.

This fix needs to  also be applied to the version of protobuf that is used by gRPC.

There are various scenarios that need to be tested on Windows:

**A. Non-ascii command line arguments, e.g.**
```
protoc.exe --csharp_out=out --proto_path=E:\work\grpc\protoctest\test-Dré E:\work\grpc\protoctest\test-Dré\helloworld.proto
```

Failed output:
```
E:\work\grpc\protoctest\test-DrΘ: warning: directory does not exist.
Could not make proto path relative: E:\work\grpc\protoctest\test-DrΘ\helloworld.proto: No such file or directory
```

Success output:
- no errors printed
- generated `.cs` file in the `out` directory

**B. Non-ascii arguments in a file containing the protoc arguments (no path to file) e.g.:**
```
protoc.exe @test.rsp
```
where `test.rsp` contains:
```
--plugin=protoc-gen-grpc=E:\work\grpc\protoctest\tools\grpc_csharp_plugin.exe
--csharp_out=E:\work\grpc\protoctest\test-Dré\out
--grpc_out=E:\work\grpc\protoctest\test-Dré\out
--proto_path=E:\work\grpc\protoctest\test-Dré
helloworld.proto
```

Success output for both old and fixed code:
- no errors printed
- generated `.cs` file in the `out` directory

**C. Non-ascii arguments in a file containing the protoc arguments (with non-ascii path to file).**

(This is what `Grpc.Tools` uses.) e.g.
```
protoc.exe @e:\work\grpc\protoctest\test-Dré\test.rsp
```

Failed output:
```
Failed to open argument file: E:\work\grpc\protoctest\test-DrΘ\test.rsp
```

Success output:
- no errors printed
- generated `.cs` file in the `out` directory

Closes #17854

COPYBARA_INTEGRATE_REVIEW=#17854 from tonydnewell:windows-utf8-command-fix de9ec54
PiperOrigin-RevId: 669083804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant