System-specific environment variables on Windows pollute remote cache

[sebjmxn]

Description of the problem / feature request / question:

When building C++ rules on Windows with MSVC, system-specific environment variables are part of the inputs, i.e.

SET TEMP=C:\Users\<my_user>\AppData\Local\Temp
SET TMP=C:\Users\<my_user>\AppData\Local\Temp
SET PATH="...;C:\Program Files (x86)\Skype\Phone\;C:\Program Files (x86)\Bitvise SSH Client;..."

For remote caching, this prevents sharing outputs between different systems.

Environment info

• Operating System: Windows 10

• Bazel version (output of bazel info release): release 0.5.4rc1

Have you found anything relevant by searching the web?

https://groups.google.com/forum/#!topic/bazel-discuss/4GV8lJzfeGQ

[ulfjack]

I consider this a dupe of #2574. Can you try setting --experimental_strict_action_env? It may not work yet if you have actions that try to write to TMP, which defaults to c:\windows, which is not user-writable. We'd like to hear back whether or not it works.

[laszlocsomor]

This affects other platforms too, not just Windows.

Bazel copies envvars from the client environment to the action's environment, and if those envvars are different for each user, there's no chance for cross-user caching.

The right solution is to set user-independent values for these somewhere in the execution machinery, as part of setting up the environment just before executing a spawn.

[sebjmxn]

Note that I tried
bazel build //examples/cpp:hello-world --experimental_strict_action_env -s, but the console output produced by the -s flag did not change. It still contains the full PATH and TMP, TEMP. Is that to be expected?

I have yet to test the impact on the actual caching, I will post the results later.

(E: Still running release 0.5.4rc1)

[ulfjack]

Can you paste the console output you're seeing?

[sebjmxn]

Yes. See the attached file.
bazel_output.txt

[ulfjack]

I don't see any of these env variables on MacOS. I wonder if the ./tools/cpp/windows_cc_configure.bzl is setting them?

[ulfjack]

Reopening - there might be an additional Windows-specific issue here.

[ulfjack]

Without --experimental_strict_action_env:

    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=10.12 \
    PATH=/Users/ulfjack/bin:/Users/ulfjack/homebrew/bin:/Users/ulfjack/bin:/usr/local/git/current/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin \
    TMPDIR=/var/folders/zl/pndhy4k906s372ln2spnnrnw002vrk/T/ \
    XCODE_VERSION_OVERRIDE=8.3.3 \

With --experimental_strict_action_env:

    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=10.12 \
    PATH=/bin:/usr/bin \
    XCODE_VERSION_OVERRIDE=8.3.3 \

[ulfjack]

@laszlocsomor , can you reproduce this on Windows?

[ulfjack]

$ bazel build //src/main/cpp/... --experimental_strict_action_env=1 -s

[dslomov]

we do set INCLUDE and LIB for C++ toolchain, to help linker locate system libraries, such as kernel32.lib and such. That is sure unhappy, we need a better solution for those /cc @meteorcloudy

[meteorcloudy]

@ulfjack You are right. It is indeed windows_cc_configure.bzl who set PATH, INCLUDE, LIB, TEMP and TMP environment variables.
See https://github.com/bazelbuild/bazel/blob/master/tools/cpp/CROSSTOOL.tpl#L260
and https://github.com/bazelbuild/bazel/blob/master/tools/cpp/windows_cc_configure.bzl#L369

We calculate these env value by running a batch script.

I think the first step to solve this problem is by cleaning PATH, INCLUDE, LIB before running the script, so that irrelevant system paths like C:\Program Files (x86)\Skype\Phone\; don't get leaked.

But we still need to figure out what to do if MSVC is installed in different places.

[laszlocsomor]

I suppose this is still a problem with Bazel 0.23. @meteorcloudy , is that correct?

[meteorcloudy]

Yes, I think so

[kkpattern]

Looks lik bazel 4.2.2 still has this problem. The PATH is ok as long as all the team members install MSVC in the same path. But the TEMP and TMP are trickier. Force all the members have a same TEMP path doesn't seem a good idea. Any chance this can be fixed in the future?

[kkpattern]

Another problem we encounter is case-senstive. On Windows, path is not case-senstive. So C:\Windows\xxx is equal to C:\WINDOWS\xxx. However bazel will treat these two differently, cause remote cache miss. Maybe we can convert PATH to lower case? This sounds a different issue.