Mitigate python user site-packages

[jwnimmer-tri]

Closes #8475 as best we can right now. Hopefully upstream will provide a more comprehensive solution.

Do not merge until:

• macOS builds pass.

---

This change is 

[jwnimmer-tri]

+@soonho-tri for all review, please.

Could you also test locally on macOS if possible?

[jwnimmer-tri]

This can be tested using the repro in bazelbuild/bazel#4939. When bug is present the BAD line is shown; with the fix, it should not be shown.

[soonho-tri]

On macOS, I've checked that without this fix, ${HOME}/Library/Python/2.7/lib/python/site-packages is included while it's not the case anymore with this PR.

---

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[soonho-tri]

For the simple example in the upstream issue, using --action_env=PYTHONNOUSERSITE=1 does the trick.

But somehow, it's not the case for the Drake. This is what I did to test it:

1. pip uninstall pyyaml
2. bazel clean
3. bazel test //tools/vector_gen/... -c dbg gives:

    import yaml
ImportError: No module named yaml

which is expected.
4. pip install pyyaml --user. It install pyyaml under ${HOME}/Library/Python/2.7/lib/python/site-packages.
5. bazel test //tools/vector_gen/... -c dbg works, while it should fail.
6. I've added print sys.path in tools/vector_gen/lcm_vector_gen.py to debug. FYI, it still includes ${HOME}/Library/Python/2.7/lib/python/site-packages.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jwnimmer-tri]

Are there any cases where this breaks the macOS build? If not, then my proposal would be to rework it slightly to include macOS TODOs but then merge so at least Ubuntu is fixed (and leave the issue open). What do you think?

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[soonho-tri]

Our setup script will not install a PIP package if it already exists locally. Then Drake will not pick it up (if this PR get merged) and it will cause a problem.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jamiesnape]

FWIW I install everything with pip --user locally and our builds are already broken with Sphinx. lxml is currently only for Director so is unaffected by this, leaving just matplotlib, pygame,PyYAML.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jwnimmer-tri]

@soonho-tri I am having trouble reconciling these two statements:

[if a PIP package exists locally] Then Drake will not pick it up (if this PR get merged) and it will cause a problem.

I've added print sys.path in tools/vector_gen/lcm_vector_gen.py to debug. FYI, it still includes ${HOME}/Library/Python/2.7/lib/python/site-packages.

Maybe I will try some things on macsim and see what I can find out.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[soonho-tri]

I've added print sys.path in tools/vector_gen/lcm_vector_gen.py to debug. FYI, it still includes ${HOME}/Library/Python/2.7/lib/python/site-packages.

It's about the current macOS issue that I have. I think it's a Drake-specific issue but I have no clues at the same time.

[if a PIP package exists locally] Then Drake will not pick it up (if this PR get merged) and it will cause a problem.

Even if we resolve my issue (macOS + Drake-specific?), I think we will have a problem because of this local python packages. I guess it's not macOS-specific.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jwnimmer-tri]

Well, on Ubuntu we should not use$HOME nor grab things directly from pip, so I don't think this makes Ubuntu any worse.

I guess I buy that for macOS, if users have previously done pip install --user PyYAML and then later run our setup script, they would expect our build to still work. So indeed if this PR breaks that workflow, then I will need to find another solution.

Possibly mac's install_prereqs_user_environment.sh could patch this up, but I'm not in love with adding more items there, since I don't really want Drake to own (very much of) that file.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[soonho-tri]

I've added print sys.path in tools/vector_gen/lcm_vector_gen.py to debug. FYI, it still includes ${HOME}/Library/Python/2.7/lib/python/site-packages.

FYI, it seems that we have the same problem on Ubuntu. Could you check?

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[jwnimmer-tri]

Aha. So I guess --action_env only affects genrule, but not rule()? A rule() has a default, small environment? At least, once I add the following, then it seems to remove the ~/.local from the sys.path:

jwnimmer@call-cc:~/jwnimmer-tri/drake$ git diff tools/vector_gen/vector_gen.bzl
diff --git a/tools/vector_gen/vector_gen.bzl b/tools/vector_gen/vector_gen.bzl
index c1ba33c..a7559c9 100644
--- a/tools/vector_gen/vector_gen.bzl
+++ b/tools/vector_gen/vector_gen.bzl
@@ -78,6 +78,7 @@ def _vector_gen_impl(ctx):
         ] + [
             "--out=%s" % out.path for out in ctx.outputs.outs
         ],
+        env = {"PYTHONNOUSERSITE": "1"},
         executable = ctx.executable.lcm_vector_gen,
     )
     return struct()

I was testing with tools/workspace/godotengine/main_make_binders.py which is a genrule(), and is affected by this patch.

[jwnimmer-tri]

This really makes me think that the virtualenv proposal is going to be a better long-term answer (#8392). Then we can make a python that does what we want, and even use pip to make the virtualenv (as part of a repository rule). In the meantime, I'll consider whether I can rework this patch to be a stop-gap.

[jwnimmer-tri]

I've pushed a revision that sets PYTHONNOUSERSITE when calling vector_gen, and vector_gen is the only user of PyYAML. I assume this means if PyYAML is pip --user only, then this would make it the build fail.

[soonho-tri]

Now the negative testing (pyyaml installed locally) works as expected (FAIL) on mac and Ubuntu.

Well, on Ubuntu we should not use$HOME nor grab things directly from pip, so I don't think this makes Ubuntu any worse.

Correct. I just forget that we do not use PIP at all in those install scripts.

@jamiesnape , can we have something like the following to warn mac users if they have installed the packages used by Drake using --user?

import pip
from os.path import expanduser
home = expanduser("~")
for item in pip.get_installed_distributions():
    # Need to check if item is in the requirement list.
    if item.location.startswith(home):
        print(("Error: {} is installed locally (--user) "
              "and Drake will ignore this package")
              .format(item.project_name))

---

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jwnimmer-tri]

I am still pondering how best to support macOS users who do pip install --user. Ideally I would find a way to make this PR only in effect for Ubuntu, but that's doesn't seem easy.

In the alternative, I am looking at how to let macOS users opt-in to the user site packages, which I guess Soonho's sample code above could suggest as a resolution, if we go that route.

[jwnimmer-tri]

I decided to focus on the codegen rules, and not worry about tests and stuff yet. I retracted the tools/bazel.rc change, since it is difficult to customize for, or override by users, on macOS.

I fixed godotengine by changing the imports; it was the only real genrule at risk. The other codegen genrules are the cps stuff that doesn't really import enough to be worrisome yet.

I fixed the vector_gen rule to allow for user site-packages for PyYAML on macOS; the cmake_configure_file rule doesn't need anything from pip so I locked it down fully.

This is no longer a trivial PR, so needs at least a second reviewer.
+@jamiesnape for feature review, please.

[soonho-tri]

Reviewed 5 of 6 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jamiesnape]

---

Reviewed 1 of 2 files at r2, 5 of 6 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

tools/vector_gen/vector_gen.bzl, line 106 at r3 (raw file):

def _vector_gen_env():
    # In general, we do not want to use $HOME/.local because it's not hermetic.

Since the directory is different on Mac, something likethe Python user install directory (e.g., $HOME/.local) maybe better.

---

tools/workspace/cmake_configure_file.bzl, line 17 at r3 (raw file):

        outputs = [ctx.outputs.out],
        arguments = arguments,
        # Never use $HOME/.local; it's not hermetic.

BTW This is a different behavior to vector_gen.bzl.

---

Comments from Reviewable

[jwnimmer-tri]

Review status: 3 of 7 files reviewed at latest revision, 2 unresolved discussions.

---

tools/vector_gen/vector_gen.bzl, line 106 at r3 (raw file):

Previously, jamiesnape (Jamie Snape) wrote…

Since the directory is different on Mac, something likethe Python user install directory (e.g., $HOME/.local) maybe better.

Done (in new file).

---

tools/workspace/cmake_configure_file.bzl, line 17 at r3 (raw file):

Previously, jamiesnape (Jamie Snape) wrote…

BTW This is a different behavior to vector_gen.bzl.

Done.

My initial thought was that since this program doesn't use anything from pip, we didn't need the complexity. But on second thought, why try to parse that distinction -- better to just reuse the same settings for everything, so I've factored into a helper.

---

Comments from Reviewable

[jamiesnape]

Reviewed 4 of 5 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[soonho-tri]

Reviewed 4 of 5 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jwnimmer-tri]

@drake-jenkins-bot mac-sierra-clang-bazel-experimental-everything please
@drake-jenkins-bot mac-highsierra-clang-bazel-experimental please