Windows: Make Python binary work with runfiles symlink tree

Closes #6036.

Change-Id: I1f8ffc6cf675c86b4799682282398dd847babcf0
PiperOrigin-RevId: 211333043

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java

@@ -144,19 +144,28 @@ public Artifact createExecutable(
     String pythonBinary = getPythonBinary(ruleContext, config);
 
     if (!ruleContext.getFragment(PythonConfiguration.class).buildPythonZip()) {
+      Artifact stubOutput = executable;
+      if (OS.getCurrent() == OS.WINDOWS
+          && ruleContext.getConfiguration().enableWindowsExeLauncher()) {
+        // On Windows, use a Windows native binary to launch the python launcher script (stub file).
+        stubOutput = common.getPythonLauncherArtifact(executable);
+        executable =
+            createWindowsExeLauncher(ruleContext, pythonBinary, executable, /*useZipFile*/ false);
+      }
+
       ruleContext.registerAction(
           new TemplateExpansionAction(
               ruleContext.getActionOwner(),
-              executable,
+              stubOutput,
               STUB_TEMPLATE,
               ImmutableList.of(
                   Substitution.of("%main%", main),
                   Substitution.of("%python_binary%", pythonBinary),
                   Substitution.of("%imports%", Joiner.on(":").join(imports)),
                   Substitution.of("%workspace_name%", ruleContext.getWorkspaceName()),
                   Substitution.of("%is_zipfile%", "False"),
-                  Substitution.of("%import_all%",
-                      config.getImportAllRepositories() ? "True" : "False")),
+                  Substitution.of(
+                      "%import_all%", config.getImportAllRepositories() ? "True" : "False")),
               true));
     } else {
       Artifact zipFile = common.getPythonZipArtifact(executable);
@@ -194,7 +203,7 @@ public Artifact createExecutable(
                 .build(ruleContext));
       } else {
         if (ruleContext.getConfiguration().enableWindowsExeLauncher()) {
-          return createWindowsExeLauncher(ruleContext, pythonBinary, executable);
+          return createWindowsExeLauncher(ruleContext, pythonBinary, executable, true);
         }
 
         ruleContext.registerAction(
@@ -212,13 +221,14 @@ public Artifact createExecutable(
   }
 
   private static Artifact createWindowsExeLauncher(
-      RuleContext ruleContext, String pythonBinary, Artifact pythonLauncher)
+      RuleContext ruleContext, String pythonBinary, Artifact pythonLauncher, boolean useZipFile)
       throws InterruptedException {
     LaunchInfo launchInfo =
         LaunchInfo.builder()
             .addKeyValuePair("binary_type", "Python")
             .addKeyValuePair("workspace_name", ruleContext.getWorkspaceName())
             .addKeyValuePair("python_bin_path", pythonBinary)
+            .addKeyValuePair("use_zip_file", useZipFile ? "1" : "0")
             .build();
     LauncherFileWriteAction.createAndRegister(ruleContext, pythonLauncher, launchInfo);
     return pythonLauncher;

src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt

@@ -55,10 +55,10 @@ def FindPythonBinary(module_space):
     # Case 1: Path is a label. Not supported yet.
     raise AssertionError(
       'Bazel does not support execution of Python interpreters via labels yet')
-  elif PYTHON_BINARY.startswith('/'):
+  elif os.path.isabs(PYTHON_BINARY):
     # Case 2: Absolute path.
     return PYTHON_BINARY
-  elif '/' in PYTHON_BINARY:
+  elif os.sep in PYTHON_BINARY:
     # Case 3: Path is relative to the repo root.
     return os.path.join(module_space, PYTHON_BINARY)
   else:
@@ -72,7 +72,7 @@ def CreatePythonPathEntries(python_imports, module_space):
 # Find the runfiles tree
 def FindModuleSpace():
   stub_filename = os.path.abspath(sys.argv[0])
-  module_space = stub_filename + '.runfiles'
+  module_space = stub_filename + ('.exe' if IsWindows() else '') + '.runfiles'
   if os.path.isdir(module_space):
     return module_space
 
@@ -190,7 +190,12 @@ def Main():
       shutil.rmtree(os.path.dirname(module_space), True)
       exit(retCode)
     else:
-      os.execv(args[0], args)
+      if IsWindows():
+        # On Windows, os.execv doesn't handle arguments with spaces correctly,
+        # and it actually starts a subprocess just like subprocess.call.
+        exit(subprocess.call(args))
+      else:
+        os.execv(args[0], args)
   except EnvironmentError:
     # This works from Python 2.4 all the way to 3.x.
     e = sys.exc_info()[1]

src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java

@@ -138,6 +138,10 @@ public void initBinary(List<Artifact> srcs) {
 
     if (ruleContext.getFragment(PythonConfiguration.class).buildPythonZip()) {
       filesToBuildBuilder.add(getPythonZipArtifact(executable));
+    } else if (OS.getCurrent() == OS.WINDOWS) {
+      // TODO(bazel-team): Here we should check target platform instead of using OS.getCurrent().
+      // On Windows, add the python stub launcher in the set of files to build.
+      filesToBuildBuilder.add(getPythonLauncherArtifact(executable));
     }
 
     filesToBuild = filesToBuildBuilder.build();
@@ -154,6 +158,11 @@ public Artifact getPythonZipArtifact(Artifact executable) {
     return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ".zip");
   }
 
+  /** @return An artifact next to the executable file with no suffix, only used on Windows */
+  public Artifact getPythonLauncherArtifact(Artifact executable) {
+    return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), "");
+  }
+
   public void addCommonTransitiveInfoProviders(RuleConfiguredTargetBuilder builder,
       PythonSemantics semantics, NestedSet<Artifact> filesToBuild) {
 

src/test/shell/bazel/bazel_windows_example_test.sh

@@ -196,6 +196,31 @@ function test_native_python() {
   assert_test_fails //examples/py_native:fail
 }
 
+function test_native_python_with_runfiles() {
+  BUILD_FLAGS="--experimental_enable_runfiles --build_python_zip=0"
+  bazel build -s --verbose_failures $BUILD_FLAGS //examples/py_native:bin \
+    || fail "Failed to build //examples/py_native:bin with runfiles support"
+  (
+    # Clear runfiles related envs
+    unset RUNFILES_MANIFEST_FILE
+    unset RUNFILES_MANIFEST_ONLY
+    unset RUNFILES_DIR
+    # Run the python package directly
+    ./bazel-bin/examples/py_native/bin >& $TEST_log \
+      || fail "//examples/py_native:bin execution failed"
+    expect_log "Fib(5) == 8"
+    # Use python <python file> to run the python package
+    python ./bazel-bin/examples/py_native/bin >& $TEST_log \
+      || fail "//examples/py_native:bin execution failed"
+    expect_log "Fib(5) == 8"
+  )
+  bazel test --test_output=errors $BUILD_FLAGS //examples/py_native:test >> $TEST_log 2>&1 \
+    || fail "Test //examples/py_native:test failed while expecting success"
+  bazel test --test_output=errors $BUILD_FLAGS //examples/py_native:fail >> $TEST_log 2>&1 \
+    && fail "Test //examples/py_native:fail succeed while expecting failure" \
+    || true
+}
+
 function test_native_python_with_python3() {
   PYTHON3_PATH=${PYTHON3_PATH:-/c/Program Files/Anaconda3}
   if [ ! -x "${PYTHON3_PATH}/python.exe" ]; then

src/test/shell/integration/runfiles_test.sh

@@ -179,14 +179,15 @@ EOF
 
   # that accounts for everything
   cd ../..
-  # For shell binary, we build both `bin` and `bin.exe`, but on Linux we only build `bin`
-  # That's why we have one more symlink on Windows.
+  # For shell binary and python binary, we build both `bin` and `bin.exe`,
+  # but on Linux we only build `bin`.
+  # That's why we have two more symlinks on Windows.
   if "$is_windows"; then
-    assert_equals 10 $(find ${WORKSPACE_NAME} -type l | wc -l)
+    assert_equals 11 $(find ${WORKSPACE_NAME} -type l | wc -l)
     assert_equals  4 $(find ${WORKSPACE_NAME} -type f | wc -l)
     assert_equals  9 $(find ${WORKSPACE_NAME} -type d | wc -l)
-    assert_equals 23 $(find ${WORKSPACE_NAME} | wc -l)
-    assert_equals 14 $(wc -l < MANIFEST)
+    assert_equals 24 $(find ${WORKSPACE_NAME} | wc -l)
+    assert_equals 15 $(wc -l < MANIFEST)
   else
     assert_equals  9 $(find ${WORKSPACE_NAME} -type l | wc -l)
     assert_equals  4 $(find ${WORKSPACE_NAME} -type f | wc -l)

src/tools/launcher/python_launcher.cc

@@ -25,6 +25,7 @@ using std::vector;
 using std::wstring;
 
 static constexpr const char* PYTHON_BIN_PATH = "python_bin_path";
+static constexpr const char* USE_ZIP_FILE = "use_zip_file";
 
 ExitCode PythonBinaryLauncher::Launch() {
   wstring python_binary = this->GetLaunchInfoByKey(PYTHON_BIN_PATH);
@@ -35,10 +36,16 @@ ExitCode PythonBinaryLauncher::Launch() {
   }
 
   vector<wstring> args = this->GetCommandlineArguments();
-  wstring python_zip_file = GetBinaryPathWithoutExtension(args[0]) + L".zip";
+  wstring use_zip_file = this->GetLaunchInfoByKey(USE_ZIP_FILE);
+  wstring python_file;
+  if (use_zip_file == L"1") {
+    python_file = GetBinaryPathWithoutExtension(args[0]) + L".zip";
+  } else {
+    python_file = GetBinaryPathWithoutExtension(args[0]);
+  }
 
-  // Replace the first argument with python zip file path
-  args[0] = python_zip_file;
+  // Replace the first argument with python file path
+  args[0] = python_file;
 
   // Escape arguments that has spaces
   for (int i = 1; i < args.size(); i++) {