Bazel, Windows: sh_binary now builds a .cmd file

On Linux/MacOS, sh_binary creates an output file
with the same name as the rule. The file is a
symlink pointing to the main script of the rule
(sh_binary.srcs only allows one file.)

On Windows sh_binary also creates an output file
called the same as the rule, but it's a copy of
the main script file (due to lack of symlink
support on Windows). However the rule now also
creates the <rulename>.cmd output, which is a
wrapper script similar to the
java_binary-generated launcher.

If however the sh_binary rule's name ends with
".exe", ".cmd", or ".bat", and its main file also
ends with the same extension, then sh_binary will
not create the launcher .cmd file, and will copy
the main file to the output tree instead.

Change-Id: Idcf92ce3bb254bd6d9a1fb5c659a52220efe19aa
PiperOrigin-RevId: 160805720

6 files changed, 221 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 2a469a4a55..19158cfcd0 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -739,6 +739,7 @@ java_library(
         "bazel/rules/java/java_stub_template.txt",
         "bazel/rules/java/java_stub_template_windows.txt",
         "bazel/rules/python/stub_template.txt",
+        "bazel/rules/sh/sh_stub_template_windows.txt",
     ],
     deps = [
         ":bazel",
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 6da83aaa12..66b0ec4a1d 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
@@ -23,15 +23,22 @@ 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.ExecutableSymlinkAction;
+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.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.util.OS;
 
 /**
  * Implementation for the sh_binary rule.
  */
 public class ShBinary implements RuleConfiguredTargetFactory {
+  private static final Template STUB_SCRIPT_WINDOWS =
+      Template.forResource(ShBinary.class, "sh_stub_template_windows.txt");
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext) throws RuleErrorException {
@@ -53,21 +60,65 @@ public class ShBinary implements RuleConfiguredTargetFactory {
     ruleContext.registerAction(
         new ExecutableSymlinkAction(ruleContext.getActionOwner(), src, symlink));
 
-    NestedSet<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
-        .add(src)
-        .add(symlink)
-        .build();
-    Runfiles runfiles = new Runfiles.Builder(
-        ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles())
-        .addTransitiveArtifacts(filesToBuild)
-        .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES)
-        .build();
-    RunfilesSupport runfilesSupport = RunfilesSupport.withExecutable(
-        ruleContext, runfiles, symlink);
+    NestedSetBuilder<Artifact> filesToBuildBuilder =
+        NestedSetBuilder.<Artifact>stableOrder().add(src).add(symlink);
+    Runfiles.Builder runfilesBuilder =
+        new Runfiles.Builder(
+            ruleContext.getWorkspaceName(),
+            ruleContext.getConfiguration().legacyExternalRunfiles());
+
+    Artifact mainExecutable =
+        (OS.getCurrent() == OS.WINDOWS) ? wrapperForWindows(ruleContext, symlink, src) : symlink;
+    if (symlink != mainExecutable) {
+      filesToBuildBuilder.add(mainExecutable);
+      runfilesBuilder.addArtifact(symlink);
+    }
+    NestedSet<Artifact> filesToBuild = filesToBuildBuilder.build();
+    Runfiles runfiles =
+        runfilesBuilder
+            .addTransitiveArtifacts(filesToBuild)
+            .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").
+    RunfilesSupport runfilesSupport =
+        RunfilesSupport.withExecutable(ruleContext, runfiles, symlink);
     return new RuleConfiguredTargetBuilder(ruleContext)
         .setFilesToBuild(filesToBuild)
-        .setRunfilesSupport(runfilesSupport, symlink)
+        .setRunfilesSupport(runfilesSupport, mainExecutable)
         .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
         .build();
   }
+
+  private static Artifact wrapperForWindows(
+      RuleContext ruleContext, Artifact primaryOutput, Artifact mainFile) {
+    if (primaryOutput.getFilename().endsWith(".exe")
+        || primaryOutput.getFilename().endsWith(".bat")
+        || primaryOutput.getFilename().endsWith(".cmd")) {
+      String suffix =
+          primaryOutput.getFilename().substring(primaryOutput.getFilename().length() - 4);
+      if (mainFile.getFilename().endsWith(suffix)) {
+        return primaryOutput;
+      }
+    }
+
+    Artifact wrapper =
+        ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".cmd");
+    ruleContext.registerAction(
+        new TemplateExpansionAction(
+            ruleContext.getActionOwner(),
+            wrapper,
+            STUB_SCRIPT_WINDOWS,
+            ImmutableList.of(
+                Substitution.of(
+                    "%bash_exe_path%",
+                    ruleContext
+                        .getFragment(BazelConfiguration.class)
+                        .getShellExecutable()
+                        .getPathString())),
+            true));
+    return 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
new file mode 100644
index 0000000000..5c30e7fb8c
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/sh_stub_template_windows.txt
@@ -0,0 +1,33 @@
+@rem Copyright 2016 The Bazel Authors. All rights reserved.
+@rem
+@rem Licensed under the Apache License, Version 2.0 (the "License");
+@rem you may not use this file except in compliance with the License.
+@rem You may obtain a copy of the License at
+@rem
+@rem    http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing, software
+@rem distributed under the License is distributed on an "AS IS" BASIS,
+@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+@rem See the License for the specific language governing permissions and
+@rem limitations under the License.
+@rem
+@rem This script was generated from sh_stub_template_windows.txt.  Please
+@rem don't edit it directly.
+@rem See the header comments in the accompanying shell script (same path as that
+@rem of this file, minus the ".cmd" extension) for available command line flags.
+
+@echo off
+SETLOCAL ENABLEEXTENSIONS
+
+set bash_path=%bash_exe_path%
+
+rem launcher=${$0%.cmd}
+set launcher=%~dp0%~n0
+
+set sh_path=%launcher:\=/%
+
+rem replaces $ with \$ in $*, then puts it on the command line
+rem Cribbed from here: http://ss64.com/nt/syntax-replace.html
+set all_args=%*
+call %bash_path% -c "'%sh_path%' %all_args:$=\$%"
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidMultidexBaseTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidMultidexBaseTest.java
index 93e273c923..7d4d6ec7a5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidMultidexBaseTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidMultidexBaseTest.java
@@ -89,12 +89,10 @@ public class AndroidMultidexBaseTest extends BuildViewTestCase {
       assertThat(mainDexList).isNull();
     }
 
-    Artifact dexMerger = getFirstArtifactEndingWith(artifacts, "dexmerger");
     Artifact dexMergerInput = getFirstArtifactEndingWith(artifacts, "classes.jar");
     SpawnAction dexMergerAction = getGeneratingSpawnAction(finalDexOutput);
     ImmutableList.Builder<String> argsBuilder = ImmutableList.<String>builder()
         .add(
-            dexMerger.getExecPathString(),
             "--input",
             dexMergerInput.getExecPathString(),
             "--output",
@@ -105,7 +103,9 @@ public class AndroidMultidexBaseTest extends BuildViewTestCase {
     if (multidexMode == MultidexMode.LEGACY || multidexMode == MultidexMode.MANUAL_MAIN_DEX) {
       argsBuilder.add("--main-dex-list", mainDexList.getExecPathString());
     }
-    assertThat(dexMergerAction.getArguments()).containsExactlyElementsIn(argsBuilder.build()).inOrder();
+    assertThat(dexMergerAction.getRemainingArguments())
+        .containsExactlyElementsIn(argsBuilder.build())
+        .inOrder();
   }
 
   /**
@@ -115,19 +115,18 @@ public class AndroidMultidexBaseTest extends BuildViewTestCase {
   protected void internalTestNonMultidexBuildStructure(String ruleLabel) throws Exception {
     ConfiguredTarget binary = getConfiguredTarget(ruleLabel);
     Set<Artifact> artifacts = actionsTestUtil().artifactClosureOf(getFilesToBuild(binary));
-    Artifact dexMerger = getFirstArtifactEndingWith(artifacts, "dexmerger");
     Artifact dexInput = getFirstArtifactEndingWith(artifacts, "classes.jar");
     Artifact dexOutput = getFirstArtifactEndingWith(artifacts, "classes.dex.zip");
     SpawnAction dexAction = getGeneratingSpawnAction(dexOutput);
 
-    assertThat(dexAction.getArguments())
+    assertThat(dexAction.getRemainingArguments())
         .containsAllOf(
-            dexMerger.getExecPathString(),
             "--input",
             dexInput.getExecPathString(),
             "--output",
             dexOutput.getExecPathString(),
-            "--multidex=off").inOrder();
+            "--multidex=off")
+        .inOrder();
   }
 }
 
diff --git a/src/test/py/bazel/launcher_script_test.py b/src/test/py/bazel/launcher_script_test.py
index dff588242e..38bc557f91 100644
--- a/src/test/py/bazel/launcher_script_test.py
+++ b/src/test/py/bazel/launcher_script_test.py
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import os
+import stat
 import unittest
 from src.test.py.bazel import test_base
 
@@ -53,8 +54,9 @@ class LauncherScriptTest(test_base.TestBase):
     self.assertTrue(os.path.isdir(os.path.join(bazel_bin, 'foo/foo.runfiles')))
 
     if self.IsWindows():
+      self.assertTrue(os.path.isfile(main_binary))
       self.AssertRunfilesManifestContains(
-          os.path.join(bazel_bin, 'foo', 'foo.runfiles', 'MANIFEST'),
+          os.path.join(bazel_bin, 'foo/foo.runfiles/MANIFEST'),
           '__main__/bar/bar.txt')
     else:
       self.assertTrue(
@@ -65,6 +67,117 @@ class LauncherScriptTest(test_base.TestBase):
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertEqual(stdout[0], 'hello java')
 
+  def testShBinaryLauncher(self):
+    self.ScratchFile('WORKSPACE')
+    self.ScratchFile(
+        'foo/BUILD',
+        [
+            # On Linux/MacOS, all sh_binary rules generate an output file with
+            # the same name as the rule, and this is a symlink to the file in
+            # `srcs`. (Bazel allows only one file in `sh_binary.srcs`.)
+            # On Windows, if the rule's name and the srcs's name end with the
+            # same extension, and this extension is one of ".exe", ".cmd", or
+            # ".bat", then sh_binary makes a copy of the output file, with the
+            # same name as the rule. Otherwise (if the rule's name doesn't end
+            # with such an extension, or the extension of it doesn't match the
+            # main file's) then Bazel creates a %{rulename}.cmd output which is
+            # a similar launcher script to that generated by java_binary rules.
+            'sh_binary(',
+            '  name = "bin1.sh",',
+            '  srcs = ["foo.sh"],',
+            '  data = ["//bar:bar.txt"],',
+            ')',
+            'sh_binary(',
+            '  name = "bin2.cmd",',  # name's extension matches that of srcs[0]
+            '  srcs = ["foo.cmd"],',
+            '  data = ["//bar:bar.txt"],',
+            ')',
+            'sh_binary(',
+            '  name = "bin3.bat",',  # name's extension doesn't match srcs[0]'s
+            '  srcs = ["foo.cmd"],',
+            '  data = ["//bar:bar.txt"],',
+            ')',
+        ])
+    foo_sh = self.ScratchFile('foo/foo.sh', [
+        '#!/bin/bash',
+        'echo hello shell',
+    ])
+    foo_cmd = self.ScratchFile('foo/foo.cmd', ['@echo hello batch'])
+    self.ScratchFile('bar/BUILD', ['exports_files(["bar.txt"])'])
+    self.ScratchFile('bar/bar.txt', ['hello'])
+    os.chmod(foo_sh, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
+    os.chmod(foo_cmd, 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]
+
+    exit_code, _, stderr = self.RunBazel(['build', '//foo:all'])
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    bin1 = os.path.join(bazel_bin, 'foo', 'bin1.sh.cmd'
+                        if self.IsWindows() else 'bin1.sh')
+    self.assertTrue(os.path.exists(bin1))
+    self.assertTrue(
+        os.path.isdir(os.path.join(bazel_bin, 'foo/bin1.sh.runfiles')))
+
+    bin2 = os.path.join(bazel_bin, 'foo/bin2.cmd')
+    self.assertTrue(os.path.exists(bin2))
+    self.assertTrue(
+        os.path.isdir(os.path.join(bazel_bin, 'foo/bin2.cmd.runfiles')))
+
+    bin3 = os.path.join(bazel_bin, 'foo', 'bin3.bat.cmd'
+                        if self.IsWindows() else 'bin3.bat')
+    self.assertTrue(os.path.exists(bin3))
+    self.assertTrue(
+        os.path.isdir(os.path.join(bazel_bin, 'foo/bin3.bat.runfiles')))
+
+    if self.IsWindows():
+      self.assertTrue(os.path.isfile(bin1))
+      self.assertTrue(os.path.isfile(bin2))
+      self.assertTrue(os.path.isfile(bin3))
+    else:
+      self.assertTrue(os.path.islink(bin1))
+      self.assertTrue(os.path.islink(bin2))
+      self.assertTrue(os.path.islink(bin3))
+
+    if self.IsWindows():
+      self.AssertRunfilesManifestContains(
+          os.path.join(bazel_bin, 'foo/bin1.sh.runfiles/MANIFEST'),
+          '__main__/bar/bar.txt')
+      self.AssertRunfilesManifestContains(
+          os.path.join(bazel_bin, 'foo/bin2.cmd.runfiles/MANIFEST'),
+          '__main__/bar/bar.txt')
+      self.AssertRunfilesManifestContains(
+          os.path.join(bazel_bin, 'foo/bin3.bat.runfiles/MANIFEST'),
+          '__main__/bar/bar.txt')
+    else:
+      self.assertTrue(
+          os.path.islink(
+              os.path.join(bazel_bin,
+                           'foo/bin1.sh.runfiles/__main__/bar/bar.txt')))
+      self.assertTrue(
+          os.path.islink(
+              os.path.join(bazel_bin,
+                           'foo/bin2.cmd.runfiles/__main__/bar/bar.txt')))
+      self.assertTrue(
+          os.path.islink(
+              os.path.join(bazel_bin,
+                           'foo/bin3.bat.runfiles/__main__/bar/bar.txt')))
+
+    exit_code, stdout, stderr = self.RunProgram([bin1])
+    self.AssertExitCode(exit_code, 0, stderr)
+    self.assertEqual(stdout[0], 'hello shell')
+
+    if self.IsWindows():
+      exit_code, stdout, stderr = self.RunProgram([bin2])
+      self.AssertExitCode(exit_code, 0, stderr)
+      self.assertEqual(stdout[0], 'hello batch')
+
+      exit_code, stdout, stderr = self.RunProgram([bin3])
+      self.AssertExitCode(exit_code, 0, stderr)
+      self.assertEqual(stdout[0], 'hello batch')
+
   def AssertRunfilesManifestContains(self, manifest, entry):
     with open(manifest, 'r') as f:
       for l in f:
diff --git a/src/test/py/bazel/test_base.py b/src/test/py/bazel/test_base.py
index a5def619a8..135977beda 100644
--- a/src/test/py/bazel/test_base.py
+++ b/src/test/py/bazel/test_base.py
@@ -1,3 +1,4 @@
+# pylint: disable=g-bad-file-header
 # Copyright 2017 The Bazel Authors. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -141,6 +142,8 @@ class TestBase(unittest.TestCase):
         e.g. "foo/bar/BUILD"
       lines: [string]; the contents of the file (newlines are added
         automatically)
+    Returns:
+      The absolute path of the scratch file.
     Raises:
       ArgumentError: if `path` is absolute or contains uplevel references
       IOError: if an I/O error occurs
@@ -156,6 +159,7 @@ class TestBase(unittest.TestCase):
         for l in lines:
           f.write(l)
           f.write('\n')
+    return abspath
 
   def RunBazel(self, args, env_remove=None):
     """Runs "bazel <args>", waits for it to exit.