Windows: Implement python native launcher

Now Bazel build a Windows exe to launch the python self-extracting zip file by default, using --windows_exe_launcher=0 to switch back to cmd wrapper. The extra zip file with shebang preprended is not built on Windows anymore, even when using cmd wrapper. Change-Id: Ic7060326f19ca6e2e73ea8d8211afd1c7618083c PiperOrigin-RevId: 165707076
kinarashah · Aug 21, 2017 · 3e0277a · 3e0277a
1 parent a05cda4
commit 3e0277a
Show file tree

Hide file tree

Showing 10 changed files with 163 additions and 89 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryRule.java
@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.bazel.rules.python.BazelPyRuleClasses.PyBinaryBaseRule;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.rules.python.PythonConfiguration;
 
@@ -37,6 +38,10 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
     minus the extension.  For example, if your entry point is called
     <code>main.py</code>, then your name should be <code>main</code>.
     <!-- #END_BLAZE_RULE.NAME --> */
+    Label launcher = env.getLauncherLabel();
+    if (launcher != null) {
+      builder.add(attr("$launcher", LABEL).cfg(HOST).value(launcher));
+    }
     return builder
         .requiresConfigurationFragments(PythonConfiguration.class, BazelPythonConfiguration.class)
         .add(attr("$zipper", LABEL).cfg(HOST).exec().value(env.getToolsLabel("//tools/zip:zipper")))

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyTestRule.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.bazel.rules.python.BazelPyRuleClasses.PyBinaryBaseRule;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TriState;
@@ -35,6 +36,10 @@
 public final class BazelPyTestRule implements RuleDefinition {
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
+    Label launcher = env.getLauncherLabel();
+    if (launcher != null) {
+      builder.add(attr("$launcher", LABEL).cfg(HOST).value(launcher));
+    }
     return builder
         .requiresConfigurationFragments(PythonConfiguration.class, BazelPythonConfiguration.class)
         .add(attr("$zipper", LABEL).cfg(HOST).exec().value(env.getToolsLabel("//tools/zip:zipper")))

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
@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Substitution;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Template;
 import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec;
+import com.google.devtools.build.lib.bazel.rules.NativeLauncherUtil;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -43,6 +44,8 @@
 import com.google.devtools.build.lib.util.FileTypeSet;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -115,11 +118,6 @@ public List<PathFragment> getImports(RuleContext ruleContext) {
     return result;
   }
 
-  /** @return An artifact next to the executable file with ".zip" suffix */
-  public Artifact getPythonZipArtifact(RuleContext ruleContext, Artifact executable) {
-    return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ".zip");
-  }
-
   /** @return An artifact next to the executable file with ".temp" suffix */
   public Artifact getPythonTemplateMainArtifact(RuleContext ruleContext, Artifact executable) {
     return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ".temp");
@@ -153,7 +151,7 @@ public Artifact createExecutable(
                       config.getImportAllRepositories() ? "True" : "False")),
               true));
     } else {
-      Artifact zipFile = getPythonZipArtifact(ruleContext, executable);
+      Artifact zipFile = common.getPythonZipArtifact(executable);
       Artifact templateMain = getPythonTemplateMainArtifact(ruleContext, executable);
       // The executable zip file will unzip itself into a tmp directory and then run from there
       ruleContext.registerAction(
@@ -171,35 +169,59 @@ public Artifact createExecutable(
                       config.getImportAllRepositories() ? "True" : "False")),
               true));
 
-      ruleContext.registerAction(
-          new SpawnAction.Builder()
-              .addInput(zipFile)
-              .addOutput(executable)
-              .setShellCommand(
-                  "echo '#!/usr/bin/env python' | cat - "
-                      + zipFile.getExecPathString()
-                      + " > "
-                      + executable.getExecPathString())
-              .useDefaultShellEnvironment()
-              .setMnemonic("BuildBinary")
-              .build(ruleContext));
-
-      if (OS.getCurrent() == OS.WINDOWS) {
-        Artifact executableWrapper = common.getExecutableWrapper();
+      if (OS.getCurrent() != OS.WINDOWS) {
+        ruleContext.registerAction(
+            new SpawnAction.Builder()
+                .addInput(zipFile)
+                .addOutput(executable)
+                .setShellCommand(
+                    "echo '#!/usr/bin/env python' | cat - "
+                        + zipFile.getExecPathString()
+                        + " > "
+                        + executable.getExecPathString())
+                .useDefaultShellEnvironment()
+                .setMnemonic("BuildBinary")
+                .build(ruleContext));
+      } else {
+        if (ruleContext.getConfiguration().enableWindowsExeLauncher()) {
+          return createWindowsExeLauncher(ruleContext, pythonBinary, executable);
+        }
+
         ruleContext.registerAction(
             new TemplateExpansionAction(
                 ruleContext.getActionOwner(),
-                executableWrapper,
+                executable,
                 STUB_TEMPLATE_WINDOWS,
                 ImmutableList.of(Substitution.of("%python_path%", pythonBinary)),
                 true));
-        return executableWrapper;
+        return executable;
       }
     }
 
     return executable;
   }
 
+  private static Artifact createWindowsExeLauncher(
+      RuleContext ruleContext, String pythonBinary, Artifact pythonLauncher)
+      throws InterruptedException {
+    ByteArrayOutputStream launchInfo = new ByteArrayOutputStream();
+    try {
+      NativeLauncherUtil.writeLaunchInfo(launchInfo, "binary_type", "Python");
+      NativeLauncherUtil.writeLaunchInfo(
+          launchInfo, "workspace_name", ruleContext.getWorkspaceName());
+      NativeLauncherUtil.writeLaunchInfo(launchInfo, "python_bin_path", pythonBinary);
+
+      NativeLauncherUtil.writeDataSize(launchInfo);
+    } catch (IOException e) {
+      ruleContext.ruleError(e.getMessage());
+      throw new InterruptedException();
+    }
+
+    NativeLauncherUtil.createNativeLauncherActions(ruleContext, pythonLauncher, launchInfo);
+
+    return pythonLauncher;
+  }
+
   @Override
   public void postInitBinary(RuleContext ruleContext, RunfilesSupport runfilesSupport,
       PyCommon common) throws InterruptedException {
@@ -210,7 +232,7 @@ public void postInitBinary(RuleContext ruleContext, RunfilesSupport runfilesSupp
         createPythonZipAction(
             ruleContext,
             executable,
-            getPythonZipArtifact(ruleContext, executable),
+            common.getPythonZipArtifact(executable),
             getPythonTemplateMainArtifact(ruleContext, executable),
             zipper,
             runfilesSupport);
@@ -265,7 +287,7 @@ private static void createPythonZipAction(
     // Read each runfile from execute path, add them into zip file at the right runfiles path.
     // Filter the executable file, cause we are building it.
     for (Artifact artifact : runfilesSupport.getRunfilesArtifactsWithoutMiddlemen()) {
-      if (!artifact.equals(executable)) {
+      if (!artifact.equals(executable) && !artifact.equals(zipFile)) {
         argv.addDynamicString(
             getZipRunfilesPath(artifact.getRunfilesPath(), workspaceName)
                 + "="

diff --git a/...in/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template_windows.txt b/...in/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template_windows.txt
@@ -17,7 +17,7 @@
 
 @SETLOCAL ENABLEEXTENSIONS
 
-@rem launcher=${$0%.cmd}
-@set launcher=%~dp0%~n0
+@rem launcher=${$0%.cmd}.zip
+@set launcher=%~dp0%~n0.zip
 
 @call "%python_path%" "%launcher%" %*
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBinary.java
@@ -128,9 +128,6 @@ private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon
     Runfiles.Builder builder = new Runfiles.Builder(
         ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles());
     builder.addArtifact(common.getExecutable());
-    if (common.getExecutableWrapper() != null) {
-      builder.addArtifact(common.getExecutableWrapper());
-    }
     if (common.getConvertedFiles() != null) {
       builder.addSymlinks(common.getConvertedFiles());
     } else {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
@@ -81,7 +81,6 @@ public void collectMetadataArtifacts(Iterable<Artifact> artifacts,
   private final RuleContext ruleContext;
 
   private Artifact executable = null;
-  private Artifact executableWrapper = null;
 
   private NestedSet<Artifact> transitivePythonSources;
 
@@ -115,10 +114,18 @@ public void initBinary(List<Artifact> srcs) {
     Preconditions.checkNotNull(version);
 
     validatePackageName();
-    executable = ruleContext.createOutputArtifact();
     if (OS.getCurrent() == OS.WINDOWS) {
-      executableWrapper =
-          ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".cmd");
+      String executableSuffix;
+      if (ruleContext.getConfiguration().enableWindowsExeLauncher()) {
+        executableSuffix = ".exe";
+      } else {
+        executableSuffix = ".cmd";
+      }
+      executable =
+          ruleContext.getImplicitOutputArtifact(
+              ruleContext.getTarget().getName() + executableSuffix);
+    } else {
+      executable = ruleContext.createOutputArtifact();
     }
     if (this.version == PythonVersion.PY2AND3) {
       // TODO(bazel-team): we need to create two actions
@@ -127,9 +134,11 @@ public void initBinary(List<Artifact> srcs) {
 
     NestedSetBuilder<Artifact> filesToBuildBuilder =
         NestedSetBuilder.<Artifact>stableOrder().addAll(srcs).add(executable);
-    if (executableWrapper != null) {
-      filesToBuildBuilder.add(executableWrapper);
+
+    if (ruleContext.getConfiguration().buildPythonZip()) {
+      filesToBuildBuilder.add(getPythonZipArtifact(executable));
     }
+
     filesToBuild = filesToBuildBuilder.build();
 
     if (ruleContext.hasErrors()) {
@@ -139,6 +148,11 @@ public void initBinary(List<Artifact> srcs) {
     addPyExtraActionPseudoAction();
   }
 
+  /** @return An artifact next to the executable file with ".zip" suffix */
+  public Artifact getPythonZipArtifact(Artifact executable) {
+    return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ".zip");
+  }
+
   public void addCommonTransitiveInfoProviders(RuleConfiguredTargetBuilder builder,
       PythonSemantics semantics, NestedSet<Artifact> filesToBuild) {
 
@@ -456,10 +470,6 @@ public Artifact getExecutable() {
     return executable;
   }
 
-  public Artifact getExecutableWrapper() {
-    return executableWrapper;
-  }
-
   public Map<PathFragment, Artifact> getConvertedFiles() {
     return convertedFiles;
   }

diff --git a/src/test/py/bazel/launcher_test.py b/src/test/py/bazel/launcher_test.py
@@ -172,6 +172,51 @@ def _buildShBinaryTargets(self, bazel_bin, launcher_flag, bin1_suffix):
       self.AssertExitCode(exit_code, 0, stderr)
       self.assertEqual(stdout[0], 'hello batch')
 
+  def _buildPyTargets(self, bazel_bin, launcher_flag, win_suffix):
+    # Verify that the build of our py_binary succeeds.
+    exit_code, _, stderr = self.RunBazel(['build', '//foo:foo'] + launcher_flag)
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    binary_suffix = win_suffix if self.IsWindows() else ''
+
+    # Verify that generated files exist.
+    foo_bin = os.path.join(bazel_bin, 'foo', 'foo%s' % binary_suffix)
+    self.assertTrue(os.path.isfile(foo_bin))
+    self.assertTrue(
+        os.path.isdir(
+            os.path.join(bazel_bin, 'foo/foo%s.runfiles' % binary_suffix)))
+
+    # Verify contents of runfiles (manifest).
+    if self.IsWindows():
+      self.AssertRunfilesManifestContains(
+          os.path.join(bazel_bin,
+                       'foo/foo%s.runfiles/MANIFEST' % binary_suffix),
+          '__main__/bar/bar.txt')
+    else:
+      self.assertTrue(
+          os.path.islink(
+              os.path.join(bazel_bin, 'foo/foo.runfiles/__main__/bar/bar.txt')))
+
+    # Try to run the built py_binary.
+    exit_code, stdout, stderr = self.RunProgram([foo_bin])
+    self.AssertExitCode(exit_code, 0, stderr)
+    self.assertEqual(stdout[0], 'Hello World!')
+
+    # Try to use the py_binary as an executable in a Skylark rule.
+    exit_code, stdout, stderr = self.RunBazel(['build', '//foo:hello'] +
+                                              launcher_flag)
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    # Verify that the Skylark action generated the right output.
+    hello_path = os.path.join(bazel_bin, 'foo', 'hello.txt')
+    self.assertTrue(os.path.isfile(hello_path))
+    with open(hello_path, 'r') as f:
+      self.assertEqual(f.read(), 'Hello World!')
+
+    # Verify that running py_test succeeds.
+    exit_code, _, stderr = self.RunBazel(['test', '//foo:test'] + launcher_flag)
+    self.AssertExitCode(exit_code, 0, stderr)
+
   def testShBinaryLauncher(self):
     self.ScratchFile('WORKSPACE')
     self.ScratchFile(
@@ -282,18 +327,10 @@ def testPyBinaryLauncher(self):
         ')',
     ])
     self.ScratchFile('foo/BUILD', [
-        'load(":foo.bzl", "helloworld")',
-        '',
-        'py_binary(',
-        '  name = "foo",',
-        '  srcs = ["foo.py"],',
-        '  data = ["//bar:bar.txt"],',
-        ')',
-        '',
-        'helloworld(',
-        '  name = "hello",',
-        '  out = "hello.txt",',
-        ')'
+        'load(":foo.bzl", "helloworld")', '', 'py_binary(', '  name = "foo",',
+        '  srcs = ["foo.py"],', '  data = ["//bar:bar.txt"],', ')', '',
+        'py_test(', '  name = "test",', '  srcs = ["test.py"],', ')', '',
+        'helloworld(', '  name = "hello",', '  out = "hello.txt",', ')'
     ])
     foo_py = self.ScratchFile('foo/foo.py', [
         '#!/usr/bin/env python',
@@ -304,48 +341,26 @@ def testPyBinaryLauncher(self):
         'else:',
         '  print("Hello World!")',
     ])
+    test_py = self.ScratchFile('foo/test.py', [
+        '#!/usr/bin/env python',
+        'import unittest',
+        'class MyTest(unittest.TestCase):',
+        '  def test_dummy(self):',
+        '      pass',
+        'if __name__ == \'__main__\':',
+        '  unittest.main()',
+    ])
     self.ScratchFile('bar/BUILD', ['exports_files(["bar.txt"])'])
     self.ScratchFile('bar/bar.txt', ['hello'])
     os.chmod(foo_py, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
+    os.chmod(test_py, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
 
     exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
     self.AssertExitCode(exit_code, 0, stderr)
     bazel_bin = stdout[0]
 
-    # Verify that the build of our py_binary succeeds.
-    exit_code, _, stderr = self.RunBazel(['build', '//foo:foo'])
-    self.AssertExitCode(exit_code, 0, stderr)
-
-    # Verify that generated files exist.
-    foo_bin = os.path.join(bazel_bin, 'foo', 'foo.cmd'
-                           if self.IsWindows() else 'foo')
-    self.assertTrue(os.path.isfile(foo_bin))
-    self.assertTrue(os.path.isdir(os.path.join(bazel_bin, 'foo/foo.runfiles')))
-
-    # Verify contents of runfiles (manifest).
-    if self.IsWindows():
-      self.AssertRunfilesManifestContains(
-          os.path.join(bazel_bin, 'foo/foo.runfiles/MANIFEST'),
-          '__main__/bar/bar.txt')
-    else:
-      self.assertTrue(
-          os.path.islink(
-              os.path.join(bazel_bin, 'foo/foo.runfiles/__main__/bar/bar.txt')))
-
-    # Try to run the built py_binary.
-    exit_code, stdout, stderr = self.RunProgram([foo_bin])
-    self.AssertExitCode(exit_code, 0, stderr)
-    self.assertEqual(stdout[0], 'Hello World!')
-
-    # Try to use the py_binary as an executable in a Skylark rule.
-    exit_code, stdout, stderr = self.RunBazel(['build', '//foo:hello'])
-    self.AssertExitCode(exit_code, 0, stderr)
-
-    # Verify that the Skylark action generated the right output.
-    hello_path = os.path.join(bazel_bin, 'foo', 'hello.txt')
-    self.assertTrue(os.path.isfile(hello_path))
-    with open(hello_path, 'r') as f:
-      self.assertEqual(f.read(), 'Hello World!')
+    self._buildPyTargets(bazel_bin, ['--windows_exe_launcher=0'], '.cmd')
+    self._buildPyTargets(bazel_bin, [], '.exe')
 
   def AssertRunfilesManifestContains(self, manifest, entry):
     with open(manifest, 'r') as f: