py_runtime not added to sandbox

[abergmeier-dsfishlabs]

Description of the problem / feature request / question:

In our repository we have a hermetic py_runtime and py_binary. When this py_binary is used in an action, the resulting sandbox does not get linked any py_runtime file:

Traceback (most recent call last):
  File "bazel-out/host/bin/external/testworkspace/mypybinary", line 178, in <module>
    Main()
  File "bazel-out/host/bin/external/testworkspace/mypybinary", line 168, in Main
os.execv(args[0], args)

OSError: [Errno 2] No such file or directory: '/dev/shm/bazel-sandbox.6db27142db1a9dda6ab345b217b92239/5712676529500419816/execroot/testworkspace/third_party/python3/linux_x86_64/bin/python3.6'

Environment info

• Operating System: Ubuntu 16.04.2
• Bazel version (output of bazel info release): 0.5.3

[iirina]

@philwo @lberki can you take a look at this?

[philwo]

@abergmeier-dsfishlabs I'm not familiar at all with creating a hermetic py_runtime. :( Andreas, could you provide a minimal example for reproducing this? Maybe copy & pastable instructions or a tar.gz of a repo where I just have to "bazel build" something to see the error?

[abergmeier-dsfishlabs]

@philwo There you go: https://github.com/abergmeier-dsfishlabs/bazel_hermetic_test

Run bazel build //:bar

[amlinux]

+1 to the importance of this bug.
A workaround is to selectively disable sandboxing: amlinux/bazel_hermetic_test@7f8a1a9

[AustinSchuh]

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
index 1aed86d..3ba95b9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -75,6 +75,7 @@ public class BazelPythonSemantics implements PythonSemantics {
 
   @Override
   public void collectDefaultRunfilesForBinary(RuleContext ruleContext, Builder builder) {
+    addRuntime(ruleContext, builder);
   }
 
   @Override
@@ -358,7 +359,11 @@ public class BazelPythonSemantics implements PythonSemantics {
         pythonBinary = provider.interpreterPath();
       } else {
         // checked in Python interpreter in py_runtime
-        pythonBinary = provider.interpreter().getExecPathString();
+        PathFragment workspaceName = PathFragment.create(
+            ruleContext.getRule().getPackage().getWorkspaceName());
+        pythonBinary =
+            workspaceName.getRelative(provider.interpreter().getRunfilesPath())
+                .getPathString();
       }
     } else  {
       // make use of the Python interpreter in an absolute path
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
index 2aacd41..faa5c19 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
@@ -50,7 +50,7 @@ def IsRunningFromZip():
   return %is_zipfile%
 
 # Find the real Python binary if it's not a normal absolute path
-def FindPythonBinary():
+def FindPythonBinary(module_space):
   if PYTHON_BINARY.startswith('//'):
     # Case 1: Path is a label. Not supported yet.
     raise AssertionError(
@@ -60,7 +60,7 @@ def FindPythonBinary():
     return PYTHON_BINARY
   elif '/' in PYTHON_BINARY:
     # Case 3: Path is relative to current working directory.
-    return os.path.join(os.getcwd(), PYTHON_BINARY)
+    return os.path.join(module_space, PYTHON_BINARY)
   else:
     # Case 4: Path has to be looked up in the search path.
     return SearchPath(PYTHON_BINARY)
@@ -146,7 +146,7 @@ def Main():
   assert os.access(main_filename, os.R_OK), \
          'Cannot exec() %r: file not readable.' % main_filename
 
-  program = python_program = FindPythonBinary()
+  program = python_program = FindPythonBinary(module_space)
   if python_program is None:
     raise AssertionError('Could not find python binary: ' + PYTHON_BINARY)
   args = [python_program, main_filename] + args

Should add it to the sandbox and find it in the .runfiles folder. I haven't tested what the other side-effects are so beware.

[philwo]

@AustinSchuh Do you mind sending this as a pull-request? We can just run it through our test suite to see if its fine. :)

[AustinSchuh]

@philwo A gerrit code review, not a pull request, right? Or is the process changing?

[philwo]

@AustinSchuh Feel free to pick whichever you prefer. :) Our import / merge script supports both and there's no difference in the workflow.

I think GitHub pull requests are totally fine for smaller patches, while Gerrit is preferred for bigger code reviews. But AFAIK that's not a hard rule, just a recommendation.

[AustinSchuh]

@philwo done! Maybe I should file another bug, but I'm struggling to find the command I should be using to run all the tests applicable to my platform before submitting. The command that I found in the docs seems to be out of date since it tries to build Windows on Linux.

[greggdonovan]

How's the review going? We've been hitting this same issue. Any chance this makes it to Bazel 0.8.0 or 0.9.0?

[AustinSchuh]

@greggdonovan Likely too late for 0.8.0 since there's already a RC, but 0.9.0 seems possible. https://bazel-review.googlesource.com/c/bazel/+/19750

[benley]

Any word on this? I keep running into awkward cases that would be nicely improved by this fix.

[AustinSchuh]

It's merged and we've been using it for a quarter.

[benley]

Ha, great. Probably time to close this issue then?

[lberki]

Yep. Let's do it.