Windows, sh_bin. launcher: export runfiles envvars

laszlocsomor · dslomov · commit 837e1b3d4859 · 2017-08-03T12:10:09.000+02:00
Fix the exe launcher of sh_binary rules to export the RUNFILES_MANIFEST_ONLY and RUNFILES_MANIFEST_FILE environment variables. Fixes bazelbuild#3492 Change-Id: I8507565f44c8b59f8218570306375cc083a41e03 PiperOrigin-RevId: 164095286
diff --git a/src/test/py/bazel/launcher_test.py b/src/test/py/bazel/launcher_test.py
@@ -112,6 +112,8 @@ def testShBinaryLauncher(self):
     foo_sh = self.ScratchFile('foo/foo.sh', [
         '#!/bin/bash',
         'echo hello shell',
+        'echo runfiles_manifest_only=${RUNFILES_MANIFEST_ONLY:-}',
+        'echo runfiles_manifest_file=${RUNFILES_MANIFEST_FILE:-}',
     ])
     foo_cmd = self.ScratchFile('foo/foo.cmd', ['@echo hello batch'])
     self.ScratchFile('bar/BUILD', ['exports_files(["bar.txt"])'])
@@ -182,7 +184,18 @@ def testShBinaryLauncher(self):
 
     exit_code, stdout, stderr = self.RunProgram([bin1])
     self.AssertExitCode(exit_code, 0, stderr)
+    self.assertEqual(len(stdout), 3)
     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.*MANIFEST$')
+    else:
+      # TODO(laszlocsomor): Find out whether the runfiles-related envvars should
+      # be set on Linux (e.g. $RUNFILES, $RUNFILES_MANIFEST_FILE). Currently
+      # they aren't, and that may be a bug. If it's indeed a bug, fix that bug
+      # and update this test.
+      self.assertEqual(stdout[1], 'runfiles_manifest_only=')
+      self.assertEqual(stdout[2], 'runfiles_manifest_file=')
 
     if self.IsWindows():
       exit_code, stdout, stderr = self.RunProgram([bin2])
diff --git a/src/tools/launcher/bash_launcher.cc b/src/tools/launcher/bash_launcher.cc
@@ -30,7 +30,7 @@ ExitCode BashBinaryLauncher::Launch() {
   string bash_binary = this->GetLaunchInfoByKey(BASH_BIN_PATH);
   // If specified bash binary path doesn't exist, then fall back to
   // bash.exe and hope it's in PATH.
-  if (!DoesFilePathExist(bash_binary)) {
+  if (!DoesFilePathExist(bash_binary.c_str())) {
     bash_binary = "bash.exe";
   }
 
diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc
@@ -34,27 +34,28 @@ using std::vector;
 
 BinaryLauncherBase::BinaryLauncherBase(
     const LaunchDataParser::LaunchInfo& _launch_info, int argc, char* argv[])
-    : launch_info(_launch_info) {
-  this->workspace_name = GetLaunchInfoByKey(WORKSPACE_NAME);
+    : launch_info(_launch_info),
+      manifest_file(FindManifestFile(argv[0])),
+      workspace_name(GetLaunchInfoByKey(WORKSPACE_NAME)) {
   for (int i = 0; i < argc; i++) {
     this->commandline_arguments.push_back(argv[i]);
   }
-  ParseManifestFile(&this->manifest_file_map, FindManifestFile());
+  ParseManifestFile(&this->manifest_file_map, this->manifest_file);
 }
 
-string BinaryLauncherBase::FindManifestFile() const {
+string BinaryLauncherBase::FindManifestFile(const char* argv0) {
   // Get the name of the binary
-  string binary = GetBinaryPathWithoutExtension(this->commandline_arguments[0]);
+  string binary = GetBinaryPathWithoutExtension(argv0);
 
   // Try to find <path to binary>.runfiles/MANIFEST
   string manifest_file = binary + ".runfiles\\MANIFEST";
-  if (DoesFilePathExist(manifest_file)) {
+  if (DoesFilePathExist(manifest_file.c_str())) {
     return manifest_file;
   }
 
   // Also try to check if <path to binary>.runfiles_manifest exists
   manifest_file = binary + ".runfiles_manifest";
-  if (DoesFilePathExist(manifest_file)) {
+  if (DoesFilePathExist(manifest_file.c_str())) {
     return manifest_file;
   }
 
@@ -125,6 +126,8 @@ void BinaryLauncherBase::CreateCommandLine(
 
 ExitCode BinaryLauncherBase::LaunchProcess(
     const string& executable, const vector<string>& arguments) const {
+  SetEnvironmentVariableA("RUNFILES_MANIFEST_ONLY", "1");
+  SetEnvironmentVariableA("RUNFILES_MANIFEST_FILE", manifest_file.c_str());
   CmdLine cmdline;
   CreateCommandLine(&cmdline, executable, arguments);
   PROCESS_INFORMATION processInfo = {0};
diff --git a/src/tools/launcher/launcher.h b/src/tools/launcher/launcher.h
@@ -66,12 +66,15 @@ class BinaryLauncherBase {
   // A map to store all the launch information.
   const LaunchDataParser::LaunchInfo& launch_info;
 
+  // Absolute path to the runfiles manifest file.
+  const std::string manifest_file;
+
   // The commandline arguments recieved.
   // The first argument is the path of this launcher itself.
   std::vector<std::string> commandline_arguments;
 
   // The workspace name of the repository this target belongs to.
-  std::string workspace_name;
+  const std::string workspace_name;
 
   // A map to store all entries of the manifest file.
   std::unordered_map<std::string, std::string> manifest_file_map;
@@ -89,7 +92,7 @@ class BinaryLauncherBase {
   // Expect the manifest file to be at
   //    1. <path>/<to>/<binary>/<target_name>.runfiles/MANIFEST
   // or 2. <path>/<to>/<binary>/<target_name>.runfiles_manifest
-  std::string FindManifestFile() const;
+  static std::string FindManifestFile(const char* argv0);
 
   // Parse manifest file into a map
   static void ParseManifestFile(ManifestFileMap* manifest_file_map,
diff --git a/src/tools/launcher/util/launcher_util.cc b/src/tools/launcher/util/launcher_util.cc
@@ -67,8 +67,8 @@ void PrintError(const char* format, ...) {
   fputc('\n', stderr);
 }
 
-bool DoesFilePathExist(const string& path) {
-  DWORD dwAttrib = GetFileAttributes(path.c_str());
+bool DoesFilePathExist(const char* path) {
+  DWORD dwAttrib = GetFileAttributes(path);
 
   return (dwAttrib != INVALID_FILE_ATTRIBUTES &&
           !(dwAttrib & FILE_ATTRIBUTE_DIRECTORY));
diff --git a/src/tools/launcher/util/launcher_util.h b/src/tools/launcher/util/launcher_util.h
@@ -48,7 +48,7 @@ std::string GetBinaryPathWithExtension(const std::string& binary);
 std::string GetEscapedArgument(const std::string& argument);
 
 // Check if a file exists at a given path.
-bool DoesFilePathExist(const std::string& path);
+bool DoesFilePathExist(const char* path);
 
 }  // namespace launcher
 }  // namespace bazel
diff --git a/src/tools/launcher/util/launcher_util_test.cc b/src/tools/launcher/util/launcher_util_test.cc
@@ -86,8 +86,8 @@ TEST_F(LaunchUtilTest, DoesFilePathExistTest) {
   string file1 = GetTmpDir() + "/foo";
   string file2 = GetTmpDir() + "/bar";
   CreateEmptyFile(file1);
-  ASSERT_TRUE(DoesFilePathExist(file1));
-  ASSERT_FALSE(DoesFilePathExist(file2));
+  ASSERT_TRUE(DoesFilePathExist(file1.c_str()));
+  ASSERT_FALSE(DoesFilePathExist(file2.c_str()));
 }
 
 }  // namespace launcher

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ ExitCode BashBinaryLauncher::Launch() {`
`30`	`30`	`string bash_binary = this->GetLaunchInfoByKey(BASH_BIN_PATH);`
`31`	`31`	`// If specified bash binary path doesn't exist, then fall back to`
`32`	`32`	`// bash.exe and hope it's in PATH.`
`33`		`- if (!DoesFilePathExist(bash_binary)) {`
	`33`	`+ if (!DoesFilePathExist(bash_binary.c_str())) {`
`34`	`34`	`bash_binary = "bash.exe";`
`35`	`35`	`}`
`36`	`36`
Original file line number	Diff line number	Diff line change
`@@ -67,8 +67,8 @@ void PrintError(const char* format, ...) {`
`67`	`67`	`fputc('\n', stderr);`
`68`	`68`	`}`
`69`	`69`
`70`		`-bool DoesFilePathExist(const string& path) {`
`71`		`- DWORD dwAttrib = GetFileAttributes(path.c_str());`
	`70`	`+bool DoesFilePathExist(const char* path) {`
	`71`	`+ DWORD dwAttrib = GetFileAttributes(path);`
`72`	`72`
`73`	`73`	`return (dwAttrib != INVALID_FILE_ATTRIBUTES &&`
`74`	`74`	`!(dwAttrib & FILE_ATTRIBUTE_DIRECTORY));`