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.

Sign up for GitHub

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

Jump to bottom

windows,launcher: use the Unicode Windows API #4473

Closed
laszlocsomor opened this issue Jan 18, 2018 · 3 comments
Closed

windows,launcher: use the Unicode Windows API #4473

laszlocsomor opened this issue Jan 18, 2018 · 3 comments
Assignees
@meteorcloudy
Labels
category: misc > misc P2 We'll consider working on this in future. (Assignee optional) platform: windows type: feature request

Comments

@laszlocsomor
Copy link
Contributor

Description of the problem / feature request:

The native launcher for {sh,java,py}_binary rules should use the Unicode Window API, e.g. CreateProcessW instead of CreateProcessA here:

bazel/src/tools/launcher/launcher.cc

Line 170 in adecfb2

BOOL ok = CreateProcessA(

Feature requests: what underlying problem are you trying to solve with this feature?

To support non-ASCII characters in the path.

Though Bazel doesn't support non-ASCII characters in paths, the user may copy/deploy the built binaries to non-ASCII paths and I think they could reasonably expect that the binaries run.

What operating system are you running Bazel on?

Windows 10.

What's the output of bazel info release?

0.9.0

/cc @meteorcloudy
@meteorcloudy meteorcloudy self-assigned this May 15, 2018
@meteorcloudy meteorcloudy added the P2 We'll consider working on this in future. (Assignee optional) label May 15, 2018
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Jun 15, 2018
@meteorcloudy
Make blaze_util::AsAbsoluteWindowsPath support wstring as input
9b3ced5 
Now we have:
bool AsAbsoluteWindowsPath(const std::wstring& path, std::wstring* result, std::string* error);

This change helps making the C++ native launcher work with UTF-16.

See bazelbuild#4473

Change-Id: I7eaf55f9fe5a4d41e3dd09edc2a21e9b3cc9277c
@meteorcloudy meteorcloudy mentioned this issue Jun 15, 2018
Closed
bazel-io pushed a commit that referenced this issue Jun 20, 2018
@meteorcloudy
Make blaze_util::AsAbsoluteWindowsPath support wstring as input
4e578e6 
Now we have:
bool AsAbsoluteWindowsPath(const std::wstring& path, std::wstring* result, std::string* error);

This change helps making the C++ native launcher work with UTF-16.

See #4473

Closes #5406.

Change-Id: I7eaf55f9fe5a4d41e3dd09edc2a21e9b3cc9277c
PiperOrigin-RevId: 201352866
bazel-io pushed a commit that referenced this issue Jun 25, 2018
@meteorcloudy
Windows: Native launcher now works with unicode.
40f5a77 
The native launcher can now launch Java and Bash binary in
directory with non-English characters.

Unfortunately, python doesn't support running python zip file under
directory with non-English characters. eg. python ./??/bin.zip will
still fail.

See #4473

Change-Id: I77fe9cdaabffc2e0d25c7097da5c0c9333a9c4a3
PiperOrigin-RevId: 201939391
@laszlocsomor
Copy link
Contributor Author

@meteorcloudy : Is there any work remaining?

@meteorcloudy
Copy link
Member

This should be done.
But note that it still doesn't work for python because python doesn't support running python zip file under
directory with non-ascii character

@laszlocsomor
Copy link
Contributor Author

laszlocsomor commented Jul 3, 2018 •
edited
Loading

But note that it still doesn't work for python because python doesn't support running python zip file under directory with non-ascii character

Do you mean it's a limitation of the Python interpreter?

werkt pushed a commit to werkt/bazel that referenced this issue Aug 2, 2018
@meteorcloudy
Make blaze_util::AsAbsoluteWindowsPath support wstring as input
8bcfae7 
Now we have:
bool AsAbsoluteWindowsPath(const std::wstring& path, std::wstring* result, std::string* error);

This change helps making the C++ native launcher work with UTF-16.

See bazelbuild#4473

Closes bazelbuild#5406.

Change-Id: I7eaf55f9fe5a4d41e3dd09edc2a21e9b3cc9277c
PiperOrigin-RevId: 201352866
werkt pushed a commit to werkt/bazel that referenced this issue Aug 2, 2018
@meteorcloudy
Windows: Native launcher now works with unicode.
40fcc9f 
The native launcher can now launch Java and Bash binary in
directory with non-English characters.

Unfortunately, python doesn't support running python zip file under
directory with non-English characters. eg. python ./??/bin.zip will
still fail.

See bazelbuild#4473

Change-Id: I77fe9cdaabffc2e0d25c7097da5c0c9333a9c4a3
PiperOrigin-RevId: 201939391
werkt pushed a commit to werkt/bazel that referenced this issue Oct 19, 2018
@meteorcloudy
Make blaze_util::AsAbsoluteWindowsPath support wstring as input
c254c8b 
Now we have:
bool AsAbsoluteWindowsPath(const std::wstring& path, std::wstring* result, std::string* error);

This change helps making the C++ native launcher work with UTF-16.

See bazelbuild#4473

Closes bazelbuild#5406.

Change-Id: I7eaf55f9fe5a4d41e3dd09edc2a21e9b3cc9277c
PiperOrigin-RevId: 201352866
werkt pushed a commit to werkt/bazel that referenced this issue Oct 19, 2018
@meteorcloudy
Windows: Native launcher now works with unicode.
ae775cd 
The native launcher can now launch Java and Bash binary in
directory with non-English characters.

Unfortunately, python doesn't support running python zip file under
directory with non-English characters. eg. python ./??/bin.zip will
still fail.

See bazelbuild#4473

Change-Id: I77fe9cdaabffc2e0d25c7097da5c0c9333a9c4a3
PiperOrigin-RevId: 201939391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meteorcloudy meteorcloudy

Labels
category: misc > misc P2 We'll consider working on this in future. (Assignee optional) platform: windows type: feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@buchgr @meteorcloudy @laszlocsomor