Refactor Bash Launcher

1. Extract some funtions into NativeLauncherUtil.java
   So that they can also be used by Python and Java launcher.

2. Derive runfiles directory from mainExectuable
   yielding "%{name}.cmd.runfiles" or "%{name}.exe.runfiles"

3. Removed bash_main_file key, we can assume the main file is next the
   launcher just without .exe suffix.
   By doing this, we don't have to worry about passing RunfilesPath or
   ExecPath. RunfilesPath has ../ prefix for external file, ExecPath
   contians ./bazel-out/... for generated file. If main file is a
   generated file from external repository, neither of them are correct.

Change-Id: Ie0c35f6030f6229c28d935d8fb3cb6a8af5af29c
PiperOrigin-RevId: 165699975

8 files changed, 140 insertions, 92 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/NativeLauncherUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/NativeLauncherUtil.java
new file mode 100644
index 0000000000..451c05a6fe
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/NativeLauncherUtil.java
@@ -0,0 +1,94 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.bazel.rules;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.io.ByteSource;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.BinaryFileWriteAction;
+import com.google.devtools.build.lib.analysis.actions.SpawnAction;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+
+/** Utility class to create Windows native launcher */
+public class NativeLauncherUtil {
+
+  /** Write a key-value pair launch info to buffer. */
+  public static void writeLaunchInfo(ByteArrayOutputStream launchInfo, String key, String value)
+      throws IOException {
+    launchInfo.write(key.getBytes(UTF_8));
+    launchInfo.write('=');
+    launchInfo.write(value.getBytes(UTF_8));
+    launchInfo.write('\0');
+  }
+
+  /**
+   * Write the size of all the launch info as a 64-bit integer at the end of the output stream in
+   * little endian.
+   */
+  public static void writeDataSize(ByteArrayOutputStream launchInfo) throws IOException {
+    long dataSize = launchInfo.size();
+    ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES);
+    // All Windows versions are little endian.
+    buffer.order(ByteOrder.LITTLE_ENDIAN);
+    buffer.putLong(dataSize);
+    launchInfo.write(buffer.array());
+  }
+
+  /**
+   * The launcher file consists of a base launcher binary and the launch information appended to the
+   * binary.
+   *
+   * @param ruleContext The rule context.
+   * @param launcher The exe launcher we are going to build.
+   * @param launchInfo The launch info to be appended.
+   */
+  public static void createNativeLauncherActions(
+      RuleContext ruleContext, Artifact launcher, ByteArrayOutputStream launchInfo) {
+    Artifact launchInfoFile =
+        ruleContext.getRelatedArtifact(launcher.getRootRelativePath(), ".launch_info");
+
+    ruleContext.registerAction(
+        new BinaryFileWriteAction(
+            ruleContext.getActionOwner(),
+            launchInfoFile,
+            ByteSource.wrap(launchInfo.toByteArray()),
+            /*makeExecutable=*/ false));
+
+    Artifact baseLauncherBinary = ruleContext.getPrerequisiteArtifact("$launcher", Mode.HOST);
+
+    ruleContext.registerAction(
+        new SpawnAction.Builder()
+            .addInput(baseLauncherBinary)
+            .addInput(launchInfoFile)
+            .addOutput(launcher)
+            .setShellCommand(
+                "cmd.exe /c \"copy /Y /B "
+                    + baseLauncherBinary.getExecPathString().replace('/', '\\')
+                    + "+"
+                    + launchInfoFile.getExecPathString().replace('/', '\\')
+                    + " "
+                    + launcher.getExecPathString().replace('/', '\\')
+                    + " > nul\"")
+            .useDefaultShellEnvironment()
+            .setMnemonic("BuildNativeLauncher")
+            .build(ruleContext));
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
index 4399f69347..ccada476a2 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
@@ -13,10 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.bazel.rules.sh;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
 import com.google.common.collect.ImmutableList;
-import com.google.common.io.ByteSource;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -26,20 +23,17 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
-import com.google.devtools.build.lib.analysis.actions.BinaryFileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.ExecutableSymlinkAction;
-import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 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.bazel.rules.BazelConfiguration;
+import com.google.devtools.build.lib.bazel.rules.NativeLauncherUtil;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.util.OS;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
 
 /**
  * Implementation for the sh_binary rule.
@@ -88,11 +82,11 @@ public class ShBinary implements RuleConfiguredTargetFactory {
             .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
             .build();
 
-    // Create the RunfilesSupport with the symlink's name, even on Windows. This way the runfiles
-    // directory's name is derived from the symlink (yielding "%{name}.runfiles) and not from the
-    // wrapper script (yielding "%{name}.cmd.runfiles").
+    // Create the RunfilesSupport with the mainExecutable's name. On Windows, this way the runfiles
+    // directory's name is derived from the launcher (yielding "%{name}.cmd.runfiles" or
+    // "%{name}.exe.runfiles").
     RunfilesSupport runfilesSupport =
-        RunfilesSupport.withExecutable(ruleContext, runfiles, symlink);
+        RunfilesSupport.withExecutable(ruleContext, runfiles, mainExecutable);
     return new RuleConfiguredTargetBuilder(ruleContext)
         .setFilesToBuild(filesToBuild)
         .setRunfilesSupport(runfilesSupport, mainExecutable)
@@ -100,84 +94,33 @@ public class ShBinary implements RuleConfiguredTargetFactory {
         .build();
   }
 
-  // Write launch info to buffer, return the number of bytes written.
-  private static int writeLaunchInfo(ByteArrayOutputStream buffer, String key, String value)
-      throws IOException {
-    byte[] keyBytes = key.getBytes(UTF_8);
-    byte[] valueBytes = value.getBytes(UTF_8);
-    buffer.write(keyBytes);
-    buffer.write('=');
-    buffer.write(valueBytes);
-    buffer.write('\0');
-    return keyBytes.length + valueBytes.length + 2;
-  }
-
   private static boolean isWindowsExecutable(Artifact artifact) {
     return artifact.getExtension().equals("exe")
         || artifact.getExtension().equals("cmd")
         || artifact.getExtension().equals("bat");
   }
 
-  private static Artifact createWindowsExeLauncher(RuleContext ruleContext, Artifact mainFile)
+  private static Artifact createWindowsExeLauncher(RuleContext ruleContext)
       throws RuleErrorException {
-    // The launcher file consists of a base launcher binary and the launch information appended to
-    // the binary. The length of launch info is a signed 64-bit integer written at the end of
-    // the binary in little endian.
-    Artifact launcher = ruleContext.getPrerequisiteArtifact("$launcher", Mode.HOST);
     Artifact bashLauncher =
         ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".exe");
-    Artifact launchInfoFile =
-        ruleContext.getRelatedArtifact(bashLauncher.getRootRelativePath(), ".launch_info");
 
     ByteArrayOutputStream launchInfo = new ByteArrayOutputStream();
-    Long dataSize = 0L;
     try {
-      dataSize += writeLaunchInfo(launchInfo, "binary_type", "Bash");
-      dataSize += writeLaunchInfo(launchInfo, "workspace_name", ruleContext.getWorkspaceName());
-      dataSize +=
-          writeLaunchInfo(
-              launchInfo,
-              "bash_bin_path",
-              ruleContext
-                  .getFragment(BazelConfiguration.class)
-                  .getShellExecutable()
-                  .getPathString());
-      dataSize += writeLaunchInfo(launchInfo, "bash_main_file", mainFile.getExecPathString());
-
-      ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES);
-      // All Windows versions are little endian.
-      buffer.order(ByteOrder.LITTLE_ENDIAN);
-      buffer.putLong(dataSize);
-
-      launchInfo.write(buffer.array());
+      NativeLauncherUtil.writeLaunchInfo(launchInfo, "binary_type", "Bash");
+      NativeLauncherUtil.writeLaunchInfo(
+          launchInfo, "workspace_name", ruleContext.getWorkspaceName());
+      NativeLauncherUtil.writeLaunchInfo(
+          launchInfo,
+          "bash_bin_path",
+          ruleContext.getFragment(BazelConfiguration.class).getShellExecutable().getPathString());
+      NativeLauncherUtil.writeDataSize(launchInfo);
     } catch (IOException e) {
       ruleContext.ruleError(e.getMessage());
       throw new RuleErrorException();
     }
 
-    ruleContext.registerAction(
-        new BinaryFileWriteAction(
-            ruleContext.getActionOwner(),
-            launchInfoFile,
-            ByteSource.wrap(launchInfo.toByteArray()),
-            /*makeExecutable=*/ false));
-
-    ruleContext.registerAction(
-        new SpawnAction.Builder()
-            .addInput(launcher)
-            .addInput(launchInfoFile)
-            .addOutput(bashLauncher)
-            .setShellCommand(
-                "cmd.exe /c \"copy /Y /B "
-                    + launcher.getExecPathString().replace('/', '\\')
-                    + "+"
-                    + launchInfoFile.getExecPathString().replace('/', '\\')
-                    + " "
-                    + bashLauncher.getExecPathString().replace('/', '\\')
-                    + " > nul\"")
-            .useDefaultShellEnvironment()
-            .setMnemonic("BuildBashLauncher")
-            .build(ruleContext));
+    NativeLauncherUtil.createNativeLauncherActions(ruleContext, bashLauncher, launchInfo);
 
     return bashLauncher;
   }
@@ -198,7 +141,7 @@ public class ShBinary implements RuleConfiguredTargetFactory {
     }
 
     if (ruleContext.getConfiguration().enableWindowsExeLauncher()) {
-      return createWindowsExeLauncher(ruleContext, mainFile);
+      return createWindowsExeLauncher(ruleContext);
     }
 
     Artifact wrapper =
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/sh_stub_template_windows.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/sh_stub_template_windows.txt
index 175985fac6..10b2f8306f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/sh_stub_template_windows.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/sh_stub_template_windows.txt
@@ -28,7 +28,7 @@ set launcher=%~dp0%~n0
 set sh_path=%launcher:\=/%
 
 set RUNFILES_MANIFEST_ONLY=1
-set RUNFILES_MANIFEST_FILE=%sh_path%.runfiles/MANIFEST
+set RUNFILES_MANIFEST_FILE=%sh_path%.cmd.runfiles/MANIFEST
 set all_args=%*
 rem replaces $ with \$ in $*, then puts it on the command line
 rem Cribbed from here: http://ss64.com/nt/syntax-replace.html
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 6f6c846a88..8b08168b2e 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -553,7 +553,12 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "   tools=ruleContext.attr.tools)");
     @SuppressWarnings("unchecked")
     List<Artifact> inputs = (List<Artifact>) (List<?>) (MutableList) lookup("inputs");
-    assertArtifactFilenames(inputs, "mytool.sh", "mytool", "foo_Smytool-runfiles", "t.exe");
+    assertArtifactFilenames(
+        inputs,
+        "mytool.sh",
+        "mytool",
+        "foo_Smytool" + OsUtils.executableExtension() + "-runfiles",
+        "t.exe");
     @SuppressWarnings("unchecked")
     CompositeRunfilesSupplier runfilesSupplier =
         new CompositeRunfilesSupplier((List<RunfilesSupplier>) lookup("input_manifests"));
diff --git a/src/test/py/bazel/launcher_test.py b/src/test/py/bazel/launcher_test.py
index 92f93e552e..dec66400d5 100644
--- a/src/test/py/bazel/launcher_test.py
+++ b/src/test/py/bazel/launcher_test.py
@@ -88,12 +88,15 @@ class LauncherTest(test_base.TestBase):
                                          launcher_flag)
     self.AssertExitCode(exit_code, 0, stderr)
 
-    bin1 = os.path.join(bazel_bin, 'foo', 'bin1.sh.%s' % bin1_suffix
-                        if self.IsWindows() else 'bin1.sh')
+    if not self.IsWindows():
+      bin1_suffix = ''
+
+    bin1 = os.path.join(bazel_bin, 'foo', 'bin1.sh%s' % bin1_suffix)
 
     self.assertTrue(os.path.exists(bin1))
     self.assertTrue(
-        os.path.isdir(os.path.join(bazel_bin, 'foo/bin1.sh.runfiles')))
+        os.path.isdir(
+            os.path.join(bazel_bin, 'foo/bin1.sh%s.runfiles' % bin1_suffix)))
 
     exit_code, _, stderr = self.RunBazel(['build', '//foo:bin2.cmd'] +
                                          launcher_flag)
@@ -126,7 +129,8 @@ class LauncherTest(test_base.TestBase):
 
     if self.IsWindows():
       self.AssertRunfilesManifestContains(
-          os.path.join(bazel_bin, 'foo/bin1.sh.runfiles/MANIFEST'),
+          os.path.join(bazel_bin,
+                       'foo/bin1.sh%s.runfiles/MANIFEST' % bin1_suffix),
           '__main__/bar/bar.txt')
       self.AssertRunfilesManifestContains(
           os.path.join(bazel_bin, 'foo/bin2.cmd.runfiles/MANIFEST'),
@@ -151,9 +155,10 @@ class LauncherTest(test_base.TestBase):
     self.assertEqual(stdout[0], 'hello shell')
     if self.IsWindows():
       self.assertEqual(stdout[1], 'runfiles_manifest_only=1')
-      self.assertRegexpMatches(stdout[2],
-                               (r'^runfiles_manifest_file='
-                                r'[a-zA-Z]:/.*/foo/bin1.sh.runfiles/MANIFEST$'))
+      self.assertRegexpMatches(
+          stdout[2],
+          (r'^runfiles_manifest_file='
+           r'[a-zA-Z]:/.*/foo/bin1.sh%s.runfiles/MANIFEST$' % bin1_suffix))
     else:
       # TODO(laszlocsomor): Find out whether the runfiles-related envvars should
       # be set on Linux (e.g. $RUNFILES, $RUNFILES_MANIFEST_FILE). Currently
@@ -210,8 +215,8 @@ class LauncherTest(test_base.TestBase):
     self.AssertExitCode(exit_code, 0, stderr)
     bazel_bin = stdout[0]
 
-    self._buildShBinaryTargets(bazel_bin, ['--windows_exe_launcher=0'], 'cmd')
-    self._buildShBinaryTargets(bazel_bin, [], 'exe')
+    self._buildShBinaryTargets(bazel_bin, ['--windows_exe_launcher=0'], '.cmd')
+    self._buildShBinaryTargets(bazel_bin, [], '.exe')
 
   def testShBinaryArgumentPassing(self):
     self.ScratchFile('WORKSPACE')
@@ -242,10 +247,13 @@ class LauncherTest(test_base.TestBase):
         ['build', '--windows_exe_launcher', '//foo:bin'])
     self.AssertExitCode(exit_code, 0, stderr)
 
-    bin1 = os.path.join(bazel_bin, 'foo', 'bin.exe'
-                        if self.IsWindows() else 'bin')
+    bin_suffix = '.exe' if self.IsWindows() else ''
+
+    bin1 = os.path.join(bazel_bin, 'foo', 'bin%s' % bin_suffix)
     self.assertTrue(os.path.exists(bin1))
-    self.assertTrue(os.path.isdir(os.path.join(bazel_bin, 'foo/bin.runfiles')))
+    self.assertTrue(
+        os.path.isdir(
+            os.path.join(bazel_bin, 'foo/bin%s.runfiles' % bin_suffix)))
 
     arguments = ['a', 'a b', '"b"', 'C:\\a\\b\\', '"C:\\a b\\c\\"']
     exit_code, stdout, stderr = self.RunProgram([bin1] + arguments)
diff --git a/src/tools/launcher/bash_launcher.cc b/src/tools/launcher/bash_launcher.cc
index af0047d45a..3e41ac152a 100644
--- a/src/tools/launcher/bash_launcher.cc
+++ b/src/tools/launcher/bash_launcher.cc
@@ -36,8 +36,7 @@ ExitCode BashBinaryLauncher::Launch() {
 
   vector<string> origin_args = this->GetCommandlineArguments();
   ostringstream bash_command;
-  string bash_main_file =
-      this->Rlocation(this->GetLaunchInfoByKey(BASH_MAIN_FILE));
+  string bash_main_file = GetBinaryPathWithoutExtension(origin_args[0]);
   bash_command << GetEscapedArgument(bash_main_file);
   for (int i = 1; i < origin_args.size(); i++) {
     bash_command << ' ';
diff --git a/src/tools/launcher/bash_launcher.h b/src/tools/launcher/bash_launcher.h
index 8070016800..659c144c39 100644
--- a/src/tools/launcher/bash_launcher.h
+++ b/src/tools/launcher/bash_launcher.h
@@ -21,7 +21,6 @@ namespace bazel {
 namespace launcher {
 
 static constexpr const char* BASH_BIN_PATH = "bash_bin_path";
-static constexpr const char* BASH_MAIN_FILE = "bash_main_file";
 
 class BashBinaryLauncher : public BinaryLauncherBase {
  public:
diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc
index 9aaa8a5924..8e82542fb4 100644
--- a/src/tools/launcher/launcher.cc
+++ b/src/tools/launcher/launcher.cc
@@ -46,7 +46,7 @@ BinaryLauncherBase::BinaryLauncherBase(
 
 string BinaryLauncherBase::FindManifestFile(const char* argv0) {
   // Get the name of the binary
-  string binary = GetBinaryPathWithoutExtension(argv0);
+  string binary = GetBinaryPathWithExtension(argv0);
 
   // The path will be set as the RUNFILES_MANIFEST_FILE envvar and used by the
   // shell script, so let's convert backslashes to forward slashes.
@@ -64,7 +64,7 @@ string BinaryLauncherBase::FindManifestFile(const char* argv0) {
     return manifest_file;
   }
 
-  die("Couldn't find MANIFEST file %s.runfiles\\", binary.c_str());
+  die("Couldn't find MANIFEST file under %s.runfiles\\", binary.c_str());
 }
 
 void BinaryLauncherBase::ParseManifestFile(ManifestFileMap* manifest_file_map,