diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py index 7682ae4bbd5965..30196366aea9b9 100644 --- a/src/test/py/bazel/runfiles_test.py +++ b/src/test/py/bazel/runfiles_test.py @@ -14,10 +14,7 @@ # limitations under the License. import os -import shutil -import stat import unittest - import six from src.test.py.bazel import test_base @@ -231,7 +228,7 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self): # runfiles tree, Bazel actually creates empty __init__.py files (again # on every platform). However to keep these manifest entries correct, # they need to have a space character. - # We could probably strip these lines completely, but this test doesn't + # We could probably strip thses lines completely, but this test doesn't # aim to exercise what would happen in that case. mock_manifest_data = [ mock_manifest_line @@ -242,18 +239,6 @@ def testRunfilesLibrariesFindRunfilesWithRunfilesManifestEnvvar(self): substitute_manifest = self.ScratchFile( "mock-%s.runfiles/MANIFEST" % lang[0], mock_manifest_data) - # remove the original manifest file and directory so the launcher picks - # the one in the environment variable RUNFILES_MANIFEST_FILE - manifest_dir = bin_path + ".runfiles" - manifest_dir_file = os.path.join(manifest_dir, "MANIFEST") - os.chmod(manifest_dir_file, stat.S_IRWXU) - shutil.rmtree(manifest_dir) - self.assertFalse(os.path.exists(manifest_dir)) - - os.chmod(manifest_path, stat.S_IRWXU) - os.remove(manifest_path) - self.assertFalse(os.path.exists(manifest_path)) - exit_code, stdout, stderr = self.RunProgram( [bin_path], env_remove=set(["RUNFILES_DIR"]), @@ -328,68 +313,6 @@ def testLegacyExternalRunfilesOption(self): "host/bin/bin.runfiles_manifest") self.AssertFileContentNotContains(manifest_path, "__main__/external/A") - def testRunUnderWithRunfiles(self): - for s, t, exe in [ - ("WORKSPACE.mock", "WORKSPACE", False), - ("bar/BUILD.mock", "bar/BUILD", False), - ("bar/bar.py", "bar/bar.py", True), - ("bar/bar-py-data.txt", "bar/bar-py-data.txt", False), - ("bar/Bar.java", "bar/Bar.java", False), - ("bar/bar-java-data.txt", "bar/bar-java-data.txt", False), - ("bar/bar.sh", "bar/bar.sh", True), - ("bar/bar-sh-data.txt", "bar/bar-sh-data.txt", False), - ("bar/bar-run-under.sh", "bar/bar-run-under.sh", True), - ("bar/bar.cc", "bar/bar.cc", False), - ("bar/bar-cc-data.txt", "bar/bar-cc-data.txt", False), - ]: - self.CopyFile( - self.Rlocation("io_bazel/src/test/py/bazel/testdata/runfiles_test/" + - s), t, exe) - - exit_code, stdout, stderr = self.RunBazel(["info", "bazel-bin"]) - self.AssertExitCode(exit_code, 0, stderr) - bazel_bin = stdout[0] - - # build bar-sh-run-under target to run targets under it - exit_code, _, stderr = self.RunBazel( - ["build", "--verbose_failures", "//bar:bar-sh-run-under"]) - self.AssertExitCode(exit_code, 0, stderr) - - if test_base.TestBase.IsWindows(): - run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under.exe") - else: - run_under_bin_path = os.path.join(bazel_bin, "bar/bar-sh-run-under") - - self.assertTrue(os.path.exists(run_under_bin_path)) - - for lang in [("py", "Python", "bar.py"), ("java", "Java", "Bar.java"), - ("sh", "Bash", "bar.sh"), ("cc", "C++", "bar.cc")]: - exit_code, stdout, stderr = self.RunBazel([ - "run", "--verbose_failures", "--run_under=//bar:bar-sh-run-under", - "//bar:bar-" + lang[0] - ]) - self.AssertExitCode(exit_code, 0, stderr) - - if test_base.TestBase.IsWindows(): - bin_path = os.path.join(bazel_bin, "bar/bar-%s.exe" % lang[0]) - else: - bin_path = os.path.join(bazel_bin, "bar/bar-" + lang[0]) - - self.assertTrue(os.path.exists(bin_path)) - - if len(stdout) < 3: - self.fail("stdout(%s): %s" % (lang[0], stdout)) - - self.assertEqual(stdout[0], "Hello Bash Bar Run under!") - self.assertEqual(stdout[1], "Hello %s Bar!" % lang[1]) - six.assertRegex(self, stdout[2], "^rloc=.*/bar/bar-%s-data.txt" % lang[0]) - - with open(stdout[2].split("=", 1)[1], "r") as f: - lines = [l.strip() for l in f.readlines()] - if len(lines) != 1: - self.fail("lines(%s): %s" % (lang[0], lines)) - self.assertEqual(lines[0], "data for " + lang[2]) - if __name__ == "__main__": unittest.main() diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock index 21ce2e0bc9b877..de3a5cf9d086bf 100644 --- a/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock +++ b/src/test/py/bazel/testdata/runfiles_test/bar/BUILD.mock @@ -29,8 +29,3 @@ cc_binary( data = ["bar-cc-data.txt"], deps = ["@bazel_tools//tools/cpp/runfiles"], ) - -sh_binary( - name = "bar-sh-run-under", - srcs = ["bar-run-under.sh"], -) diff --git a/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh b/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh deleted file mode 100644 index 57fbf389e37095..00000000000000 --- a/src/test/py/bazel/testdata/runfiles_test/bar/bar-run-under.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash -# Copyright 2018 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. - -echo "Hello Bash Bar Run under!" -"$@" diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc index 205618b6cf139d..2d30bb4ed047ec 100644 --- a/src/tools/launcher/launcher.cc +++ b/src/tools/launcher/launcher.cc @@ -82,27 +82,14 @@ BinaryLauncherBase::BinaryLauncherBase( } static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) { - // Look for the runfiles manifest of the binary in a runfiles directory next - // to the binary, then look for it (the manifest) next to the binary. - wstring directory = GetBinaryPathWithExtension(argv0) + L".runfiles"; - *result = directory + L"/MANIFEST"; - if (DoesFilePathExist(result->c_str())) { - return true; - } - - *result = directory + L"_manifest"; - if (DoesFilePathExist(result->c_str())) { - return true; - } - - // If the manifest is not found then this binary (X) runs as the - // data-dependency of some other binary (Y) and X has no runfiles - // manifest/directory so it should use Y's. + // If this binary X runs as the data-dependency of some other binary Y, then + // X has no runfiles manifest/directory and should use Y's. if (GetEnv(L"RUNFILES_MANIFEST_FILE", result) && DoesFilePathExist(result->c_str())) { return true; } + wstring directory; if (GetEnv(L"RUNFILES_DIR", &directory)) { *result = directory + L"/MANIFEST"; if (DoesFilePathExist(result->c_str())) { @@ -110,6 +97,20 @@ static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) { } } + // If this binary X runs by itself (not as a data-dependency of another + // binary), then look for the manifest in a runfiles directory next to the + // main binary, then look for it (the manifest) next to the main binary. + directory = GetBinaryPathWithExtension(argv0) + L".runfiles"; + *result = directory + L"/MANIFEST"; + if (DoesFilePathExist(result->c_str())) { + return true; + } + + *result = directory + L"_manifest"; + if (DoesFilePathExist(result->c_str())) { + return true; + } + return false; } diff --git a/tools/cpp/runfiles/runfiles_src.cc b/tools/cpp/runfiles/runfiles_src.cc index bc0e4a8d35f4e3..75d606d7c080ad 100644 --- a/tools/cpp/runfiles/runfiles_src.cc +++ b/tools/cpp/runfiles/runfiles_src.cc @@ -93,22 +93,6 @@ bool IsDirectory(const string& path) { #endif } -// Computes the path of the runfiles manifest and the runfiles directory. -// By searching first next to the binary location `argv0` and then falling -// back on the values passed in `runfiles_manifest_file` and `runfiles_dir` -// -// If the method finds both a valid manifest and valid directory according to -// `is_runfiles_manifest` and `is_runfiles_directory`, then the method sets -// the corresponding values to `out_manifest` and `out_directory` and returns -// true. -// -// If the method only finds a valid manifest or a valid directory, but not -// both, then it sets the corresponding output variable (`out_manifest` or -// `out_directory`) to the value while clearing the other output variable. The -// method still returns true in this case. -// -// If the method cannot find either a valid manifest or valid directory, it -// clears both output variables and returns false. bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file, std::string runfiles_dir, std::string* out_manifest, std::string* out_directory); @@ -119,12 +103,6 @@ bool PathsFrom(const std::string& argv0, std::string runfiles_manifest_file, std::function is_runfiles_directory, std::string* out_manifest, std::string* out_directory); -bool PathsFrom(const std::string& runfiles_manifest_file, - const std::string& runfiles_dir, - std::function is_runfiles_manifest, - std::function is_runfiles_directory, - std::string* out_manifest, std::string* out_directory); - bool ParseManifest(const string& path, map* result, string* error); @@ -287,71 +265,45 @@ bool PathsFrom(const string& argv0, string mf, string dir, out_manifest->clear(); out_directory->clear(); - string existing_mf = mf; - string existing_dir = dir; + bool mfValid = is_runfiles_manifest(mf); + bool dirValid = is_runfiles_directory(dir); - // if argv0 is not empty, try to use it to find the runfiles manifest - // file/directory paths - if (!argv0.empty()) { + if (!argv0.empty() && !mfValid && !dirValid) { mf = argv0 + ".runfiles/MANIFEST"; dir = argv0 + ".runfiles"; - if (!is_runfiles_manifest(mf)) { + mfValid = is_runfiles_manifest(mf); + dirValid = is_runfiles_directory(dir); + if (!mfValid) { mf = argv0 + ".runfiles_manifest"; + mfValid = is_runfiles_manifest(mf); } - PathsFrom(mf, dir, is_runfiles_manifest, is_runfiles_directory, - out_manifest, out_directory); - } - - // if the runfiles manifest file/directory paths are not found, use existing - // mf and dir to find the paths - if (out_manifest->empty() && out_directory->empty()) { - return PathsFrom(existing_mf, existing_dir, is_runfiles_manifest, - is_runfiles_directory, out_manifest, out_directory); } - return true; -} - -bool PathsFrom(const std::string& runfiles_manifest, - const std::string& runfiles_directory, - function is_runfiles_manifest, - function is_runfiles_directory, - std::string* out_manifest, std::string* out_directory) { - out_manifest->clear(); - out_directory->clear(); - - - std::string mf = runfiles_manifest; - std::string dir = runfiles_directory; - - bool mf_valid = is_runfiles_manifest(mf); - bool dir_valid = is_runfiles_directory(dir); - - if (!mf_valid && !dir_valid) { + if (!mfValid && !dirValid) { return false; } - if (!mf_valid) { + if (!mfValid) { mf = dir + "/MANIFEST"; - mf_valid = is_runfiles_manifest(mf); - if (!mf_valid) { + mfValid = is_runfiles_manifest(mf); + if (!mfValid) { mf = dir + "_manifest"; - mf_valid = is_runfiles_manifest(mf); + mfValid = is_runfiles_manifest(mf); } } - if (!dir_valid && + if (!dirValid && (ends_with(mf, ".runfiles_manifest") || ends_with(mf, "/MANIFEST"))) { static const size_t kSubstrLen = 9; // "_manifest" or "/MANIFEST" dir = mf.substr(0, mf.size() - kSubstrLen); - dir_valid = is_runfiles_directory(dir); + dirValid = is_runfiles_directory(dir); } - if (mf_valid) { + if (mfValid) { *out_manifest = mf; } - if (dir_valid) { + if (dirValid) { *out_directory = dir; }