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

Windows: Case sensitivity issue of environment variables while creating processes with system API #5285

Closed
meteorcloudy opened this issue May 28, 2018 · 1 comment
Assignees
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug

Comments

@meteorcloudy
Copy link
Member

meteorcloudy commented May 28, 2018

Bazel builds environment variables map for creating Windows process by the following function:

private byte[] convertEnvToNative(Map<String, String> env) throws IOException {

Windows environment variable names are case-insensitive, this function didn't take this into consideration. Therefore it could pass the same environment variable with different cases to the Windows API. For example, {"FOO": "bar1", "foo" : "bar2", "Foo": "bar3"}. Surprisingly, Windows will allow it!
A simple reproduce with Bazel 0.13.0:
https://github.com/meteorcloudy/my_tests/blob/master/simple_repo_rule/repo.bzl#L3

pcloudy@tensorlow-jenkins-win2-slave MSYS ~/workspace/my_tests/simple_repo_rule
$ bazel-0.13.0 build @env_test//...
DEBUG: C:/tools/msys64/home/pcloudy/workspace/my_tests/simple_repo_rule/repo.bzl:3:3:
C:\users\pcloudy\_bazel_pcloudy\1e3v36rp\external\env_test>set FOO
FOO=awef
Foo=123
foo=a
INFO: Analysed 0 targets (0 packages loaded).
INFO: Found 0 targets...
INFO: Elapsed time: 0.185s, Critical Path: 0.01s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

This causes problems when we will try to set or clean environment variables before executing an action. For example,

env = _add_system_root(repository_ctx,
{"PATH": "", "INCLUDE": "", "LIB": "", "WINDOWSSDKDIR": ""})

If in user's environment, envs like Path or WindowsSdkDir are already set, this won't be able to clean the expected envs.

@meteorcloudy meteorcloudy self-assigned this May 28, 2018
@meteorcloudy meteorcloudy added type: bug P1 I'll work on this now. (Assignee required) platform: windows labels May 28, 2018
@laszlocsomor
Copy link
Contributor

Facepalm. Nice analysis @meteorcloudy .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug
Projects
None yet
Development

No branches or pull requests

2 participants