From 53e9fea30e58ab479e5cd7a6c804634f56fc2ddb Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 7 Dec 2022 08:47:10 -0800 Subject: [PATCH] Use long executable path instead of `argv[0]` in all launchers `argv[0]` can differ from the path of the launcher executable. The latter can contain 8.3 style filenames, which need to be resolved to long paths before path manipulation (e.g. appending ".runfiles") can succeed. The Python launcher handled this correctly, but other launchers didn't use the long executable path consistently. Closes #16916. PiperOrigin-RevId: 493615543 Change-Id: Ic8161890181c0110ecdf6893b9835e6f99d01097 --- site/en/extending/rules.md | 4 +- src/test/py/bazel/launcher_test.py | 145 ++++++++++++++++++++------ src/tools/launcher/bash_launcher.cc | 6 +- src/tools/launcher/bash_launcher.h | 5 +- src/tools/launcher/java_launcher.cc | 9 +- src/tools/launcher/java_launcher.h | 5 +- src/tools/launcher/launcher.cc | 27 +++-- src/tools/launcher/launcher.h | 18 +++- src/tools/launcher/launcher_main.cc | 20 ++-- src/tools/launcher/python_launcher.cc | 8 +- src/tools/launcher/python_launcher.h | 9 +- 11 files changed, 177 insertions(+), 79 deletions(-) diff --git a/site/en/extending/rules.md b/site/en/extending/rules.md index a8d8f3af9305e9..c4f6ef4fd4973e 100644 --- a/site/en/extending/rules.md +++ b/site/en/extending/rules.md @@ -717,8 +717,8 @@ When an executable target is run with `bazel run` (or `test`), the root of the runfiles directory is adjacent to the executable. The paths relate as follows: ```python -# Given executable_file and runfile_file: -runfiles_root = executable_file.path + ".runfiles" +# Given launcher_path and runfile_file: +runfiles_root = launcher_path.path + ".runfiles" workspace_name = ctx.workspace_name runfile_path = runfile_file.short_path execution_root_relative_path = "%s/%s/%s" % ( diff --git a/src/test/py/bazel/launcher_test.py b/src/test/py/bazel/launcher_test.py index 53feee61d6e78b..22022c094c8f49 100644 --- a/src/test/py/bazel/launcher_test.py +++ b/src/test/py/bazel/launcher_test.py @@ -616,21 +616,24 @@ def testWindowsNativeLauncherInLongPath(self): if not self.IsWindows(): return self.CreateWorkspaceWithDefaultRepos('WORKSPACE') - self.ScratchFile('bin/BUILD', [ - 'java_binary(', - ' name = "bin_java",', - ' srcs = ["Main.java"],', - ' main_class = "Main",', - ')', - 'sh_binary(', - ' name = "bin_sh",', - ' srcs = ["main.sh"],', - ')', - 'py_binary(', - ' name = "bin_py",', - ' srcs = ["bin_py.py"],', - ')', - ]) + self.ScratchFile( + 'bin/BUILD', + [ + 'java_binary(', + ' name = "not_short_bin_java",', + ' srcs = ["Main.java"],', + ' main_class = "Main",', + ')', + 'sh_binary(', + ' name = "not_short_bin_sh",', + ' srcs = ["main.sh"],', + ')', + 'py_binary(', + ' name = "not_short_bin_py",', + ' srcs = ["not_short_bin_py.py"],', + ')', + ], + ) self.ScratchFile('bin/Main.java', [ 'public class Main {', ' public static void main(String[] args) {' @@ -641,9 +644,12 @@ def testWindowsNativeLauncherInLongPath(self): self.ScratchFile('bin/main.sh', [ 'echo "helloworld"', ]) - self.ScratchFile('bin/bin_py.py', [ - 'print("helloworld")', - ]) + self.ScratchFile( + 'bin/not_short_bin_py.py', + [ + 'print("helloworld")', + ], + ) exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin']) self.AssertExitCode(exit_code, 0, stderr) @@ -656,52 +662,133 @@ def testWindowsNativeLauncherInLongPath(self): long_dir_path = './' + '/'.join( [(c * 8 + '.' + c * 3) for c in string.ascii_lowercase]) + # The 'not_short_' prefix ensures that the basenames are not already 8.3 + # short paths. Due to the long directory path, the basename will thus be + # replaced with a short path such as "not_sh~1.exe" below. for f in [ - 'bin_java.exe', - 'bin_java.exe.runfiles_manifest', - 'bin_sh.exe', - 'bin_sh', - 'bin_sh.exe.runfiles_manifest', - 'bin_py.exe', - 'bin_py.zip', - 'bin_py.exe.runfiles_manifest', + 'not_short_bin_java.exe', + 'not_short_bin_java.exe.runfiles_manifest', + 'not_short_bin_sh.exe', + 'not_short_bin_sh', + 'not_short_bin_sh.exe.runfiles_manifest', + 'not_short_bin_py.exe', + 'not_short_bin_py.zip', + 'not_short_bin_py.exe.runfiles_manifest', ]: self.CopyFile( os.path.join(bazel_bin, 'bin', f), os.path.join(long_dir_path, f)) - long_binary_path = os.path.abspath(long_dir_path + '/bin_java.exe') + long_binary_path = os.path.abspath( + long_dir_path + '/not_short_bin_java.exe' + ) # subprocess doesn't support long path without shell=True exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) # Make sure we can launch the binary with a shortened Windows 8dot3 path short_binary_path = win32api.GetShortPathName(long_binary_path) + self.assertIn('~', os.path.basename(short_binary_path)) exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) - long_binary_path = os.path.abspath(long_dir_path + '/bin_sh.exe') + long_binary_path = os.path.abspath(long_dir_path + '/not_short_bin_sh.exe') # subprocess doesn't support long path without shell=True exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) # Make sure we can launch the binary with a shortened Windows 8dot3 path short_binary_path = win32api.GetShortPathName(long_binary_path) + self.assertIn('~', os.path.basename(short_binary_path)) exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) - long_binary_path = os.path.abspath(long_dir_path + '/bin_py.exe') + long_binary_path = os.path.abspath(long_dir_path + '/not_short_bin_py.exe') # subprocess doesn't support long path without shell=True exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) # Make sure we can launch the binary with a shortened Windows 8dot3 path short_binary_path = win32api.GetShortPathName(long_binary_path) + self.assertIn('~', os.path.basename(short_binary_path)) exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True) self.AssertExitCode(exit_code, 0, stderr) self.assertEqual('helloworld', ''.join(stdout)) + def testWindowsNativeLauncherInvalidArgv0(self): + if not self.IsWindows(): + return + self.CreateWorkspaceWithDefaultRepos('WORKSPACE') + self.ScratchFile( + 'bin/BUILD', + [ + 'java_binary(', + ' name = "bin_java",', + ' srcs = ["Main.java"],', + ' main_class = "Main",', + ')', + 'sh_binary(', + ' name = "bin_sh",', + ' srcs = ["main.sh"],', + ')', + 'py_binary(', + ' name = "bin_py",', + ' srcs = ["bin_py.py"],', + ')', + ], + ) + self.ScratchFile( + 'bin/Main.java', + [ + 'public class Main {', + ( + ' public static void main(String[] args) {' + ' System.out.println("helloworld");' + ), + ' }', + '}', + ], + ) + self.ScratchFile( + 'bin/main.sh', + [ + 'echo "helloworld"', + ], + ) + self.ScratchFile( + 'bin/bin_py.py', + [ + 'print("helloworld")', + ], + ) + + 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', '//bin/...']) + self.AssertExitCode(exit_code, 0, stderr) + + exit_code, stdout, stderr = self.RunProgram( + ['C:\\Invalid'], + executable=os.path.join(bazel_bin, 'bin', 'bin_java.exe'), + ) + self.AssertExitCode(exit_code, 0, stderr) + self.assertEqual('helloworld', ''.join(stdout)) + + exit_code, stdout, stderr = self.RunProgram( + ['C:\\Invalid'], executable=os.path.join(bazel_bin, 'bin', 'bin_sh.exe') + ) + self.AssertExitCode(exit_code, 0, stderr) + self.assertEqual('helloworld', ''.join(stdout)) + + exit_code, stdout, stderr = self.RunProgram( + ['C:\\Invalid'], executable=os.path.join(bazel_bin, 'bin', 'bin_py.exe') + ) + self.AssertExitCode(exit_code, 0, stderr) + self.assertEqual('helloworld', ''.join(stdout)) + def AssertRunfilesManifestContains(self, manifest, entry): with open(manifest, 'r') as f: for l in f: diff --git a/src/tools/launcher/bash_launcher.cc b/src/tools/launcher/bash_launcher.cc index 56b77275c1f152..67582eb2a5e83e 100644 --- a/src/tools/launcher/bash_launcher.cc +++ b/src/tools/launcher/bash_launcher.cc @@ -52,11 +52,7 @@ ExitCode BashBinaryLauncher::Launch() { vector origin_args = this->GetCommandlineArguments(); wostringstream bash_command; - // In case the given binary path is a shortened Windows 8dot3 path, we need to - // convert it back to its long path form before using it to find the bash main - // file. - wstring full_binary_path = GetWindowsLongPath(origin_args[0]); - wstring bash_main_file = GetBinaryPathWithoutExtension(full_binary_path); + wstring bash_main_file = GetBinaryPathWithoutExtension(GetLauncherPath()); bash_command << BashEscapeArg(bash_main_file); for (int i = 1; i < origin_args.size(); i++) { bash_command << L' '; diff --git a/src/tools/launcher/bash_launcher.h b/src/tools/launcher/bash_launcher.h index b00828109f81b9..27fd353d1a4f50 100644 --- a/src/tools/launcher/bash_launcher.h +++ b/src/tools/launcher/bash_launcher.h @@ -22,9 +22,10 @@ namespace launcher { class BashBinaryLauncher : public BinaryLauncherBase { public: - BashBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, int argc, + BashBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, + const std::wstring& launcher_path, int argc, wchar_t* argv[]) - : BinaryLauncherBase(launch_info, argc, argv) {} + : BinaryLauncherBase(launch_info, launcher_path, argc, argv) {} ~BashBinaryLauncher() override = default; ExitCode Launch() override; }; diff --git a/src/tools/launcher/java_launcher.cc b/src/tools/launcher/java_launcher.cc index 12cb6a82b97c14..61d0fa3d537f0c 100644 --- a/src/tools/launcher/java_launcher.cc +++ b/src/tools/launcher/java_launcher.cc @@ -162,8 +162,7 @@ static void WriteJarClasspath(const wstring& jar_path, } wstring JavaBinaryLauncher::GetJunctionBaseDir() { - wstring binary_base_path = - GetBinaryPathWithExtension(this->GetCommandlineArguments()[0]); + wstring binary_base_path = GetBinaryPathWithExtension(GetLauncherPath()); wstring result; if (!NormalizePath(binary_base_path + L".j", &result)) { die(L"Failed to get normalized junction base directory."); @@ -191,8 +190,7 @@ void JavaBinaryLauncher::DeleteJunctionBaseDir() { } wstring JavaBinaryLauncher::CreateClasspathJar(const wstring& classpath) { - wstring binary_base_path = - GetBinaryPathWithoutExtension(this->GetCommandlineArguments()[0]); + wstring binary_base_path = GetBinaryPathWithoutExtension(GetLauncherPath()); wstring abs_manifest_jar_dir_norm = GetManifestJarDir(binary_base_path); wostringstream manifest_classpath; @@ -312,8 +310,7 @@ ExitCode JavaBinaryLauncher::Launch() { // Run deploy jar if needed, otherwise generate the CLASSPATH by rlocation. if (this->singlejar) { wstring deploy_jar = - GetBinaryPathWithoutExtension(this->GetCommandlineArguments()[0]) + - L"_deploy.jar"; + GetBinaryPathWithoutExtension(GetLauncherPath()) + L"_deploy.jar"; if (!DoesFilePathExist(deploy_jar.c_str())) { die(L"Option --singlejar was passed, but %s does not exist.\n (You may " "need to build it explicitly.)", diff --git a/src/tools/launcher/java_launcher.h b/src/tools/launcher/java_launcher.h index 1881d587d998d8..4ca9958101537d 100644 --- a/src/tools/launcher/java_launcher.h +++ b/src/tools/launcher/java_launcher.h @@ -29,9 +29,10 @@ static const int MAX_ARG_STRLEN = 7000; class JavaBinaryLauncher : public BinaryLauncherBase { public: - JavaBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, int argc, + JavaBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, + const std::wstring& launcher_path, int argc, wchar_t* argv[]) - : BinaryLauncherBase(launch_info, argc, argv), + : BinaryLauncherBase(launch_info, launcher_path, argc, argv), singlejar(false), print_javabin(false), classpath_limit(MAX_ARG_STRLEN) {} diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc index 2d30bb4ed047ec..92cf7d2e85fe3a 100644 --- a/src/tools/launcher/launcher.cc +++ b/src/tools/launcher/launcher.cc @@ -42,14 +42,14 @@ using std::vector; using std::wostringstream; using std::wstring; -static wstring GetRunfilesDir(const wchar_t* argv0) { +static wstring GetRunfilesDir(const wchar_t* launcher_path) { wstring runfiles_dir; // If RUNFILES_DIR is already set (probably we are either in a test or in a // data dependency) then use it. if (!GetEnv(L"RUNFILES_DIR", &runfiles_dir)) { // Otherwise this is probably a top-level non-test binary (e.g. a genrule // tool) and should look for its runfiles beside the executable. - runfiles_dir = GetBinaryPathWithExtension(argv0) + L".runfiles"; + runfiles_dir = GetBinaryPathWithExtension(launcher_path) + L".runfiles"; } // Make sure we return a normalized absolute path. if (!blaze_util::IsAbsolute(runfiles_dir)) { @@ -63,10 +63,12 @@ static wstring GetRunfilesDir(const wchar_t* argv0) { } BinaryLauncherBase::BinaryLauncherBase( - const LaunchDataParser::LaunchInfo& _launch_info, int argc, wchar_t* argv[]) - : launch_info(_launch_info), - manifest_file(FindManifestFile(argv[0])), - runfiles_dir(GetRunfilesDir(argv[0])), + const LaunchDataParser::LaunchInfo& _launch_info, + const std::wstring& launcher_path, int argc, wchar_t* argv[]) + : launcher_path(launcher_path), + launch_info(_launch_info), + manifest_file(FindManifestFile(launcher_path.c_str())), + runfiles_dir(GetRunfilesDir(launcher_path.c_str())), workspace_name(GetLaunchInfoByKey(WORKSPACE_NAME)), symlink_runfiles_enabled(GetLaunchInfoByKey(SYMLINK_RUNFILES_ENABLED) == L"1") { @@ -81,7 +83,8 @@ BinaryLauncherBase::BinaryLauncherBase( } } -static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) { +static bool FindManifestFileImpl(const wchar_t* launcher_path, + wstring* result) { // 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) && @@ -100,7 +103,7 @@ 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"; + directory = GetBinaryPathWithExtension(launcher_path) + L".runfiles"; *result = directory + L"/MANIFEST"; if (DoesFilePathExist(result->c_str())) { return true; @@ -114,9 +117,9 @@ static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) { return false; } -wstring BinaryLauncherBase::FindManifestFile(const wchar_t* argv0) { +wstring BinaryLauncherBase::FindManifestFile(const wchar_t* launcher_path) { wstring manifest_file; - if (!FindManifestFileImpl(argv0, &manifest_file)) { + if (!FindManifestFileImpl(launcher_path, &manifest_file)) { return L""; } // The path will be set as the RUNFILES_MANIFEST_FILE envvar and used by the @@ -125,9 +128,11 @@ wstring BinaryLauncherBase::FindManifestFile(const wchar_t* argv0) { return manifest_file; } +wstring BinaryLauncherBase::GetLauncherPath() const { return launcher_path; } + wstring BinaryLauncherBase::GetRunfilesPath() const { wstring runfiles_path = - GetBinaryPathWithExtension(this->commandline_arguments[0]) + L".runfiles"; + GetBinaryPathWithExtension(launcher_path) + L".runfiles"; std::replace(runfiles_path.begin(), runfiles_path.end(), L'/', L'\\'); return runfiles_path; } diff --git a/src/tools/launcher/launcher.h b/src/tools/launcher/launcher.h index c2e8cf16417986..b1e81a53dd4a8e 100644 --- a/src/tools/launcher/launcher.h +++ b/src/tools/launcher/launcher.h @@ -41,7 +41,8 @@ class BinaryLauncherBase { typedef std::unordered_map ManifestFileMap; public: - BinaryLauncherBase(const LaunchDataParser::LaunchInfo& launch_info, int argc, + BinaryLauncherBase(const LaunchDataParser::LaunchInfo& launch_info, + const std::wstring& launcher_path, int argc, wchar_t* argv[]); virtual ~BinaryLauncherBase() = default; @@ -77,13 +78,22 @@ class BinaryLauncherBase { // A launch function to be implemented for a specific language. virtual ExitCode Launch() = 0; + // Returns the path of the launcher's executable file. + // + // The returned path is always a long path, that is, it never contains 8.3 + // style filenames. + std::wstring GetLauncherPath() const; + // Return the runfiles directory of this binary. // - // The method appends ".exe.runfiles" to the first command line argument, - // converts forward slashes to back slashes, then returns that. + // The method appends ".exe.runfiles" to the path of the launcher executable, + // converts forward slashes to backslashes, then returns that. std::wstring GetRunfilesPath() const; private: + // The path of the launcher binary. + const std::wstring launcher_path; + // A map to store all the launch information. const LaunchDataParser::LaunchInfo& launch_info; @@ -127,7 +137,7 @@ class BinaryLauncherBase { // Expect the manifest file to be at // 1. ///.runfiles/MANIFEST // or 2. ///.runfiles_manifest - static std::wstring FindManifestFile(const wchar_t* argv0); + static std::wstring FindManifestFile(const wchar_t* launcher_path); // Parse manifest file into a map static void ParseManifestFile(ManifestFileMap* manifest_file_map, diff --git a/src/tools/launcher/launcher_main.cc b/src/tools/launcher/launcher_main.cc index e2fc4735e47276..bcf7d89e5dadeb 100644 --- a/src/tools/launcher/launcher_main.cc +++ b/src/tools/launcher/launcher_main.cc @@ -44,11 +44,12 @@ static constexpr const char* BINARY_TYPE = "binary_type"; using bazel::launcher::BashBinaryLauncher; using bazel::launcher::BinaryLauncherBase; +using bazel::launcher::die; using bazel::launcher::GetBinaryPathWithExtension; +using bazel::launcher::GetWindowsLongPath; using bazel::launcher::JavaBinaryLauncher; using bazel::launcher::LaunchDataParser; using bazel::launcher::PythonBinaryLauncher; -using bazel::launcher::die; using std::make_unique; using std::unique_ptr; @@ -64,9 +65,14 @@ static std::wstring GetExecutableFileName() { } int wmain(int argc, wchar_t* argv[]) { - const std::wstring executable_file = GetExecutableFileName(); + // In case the given binary path is a shortened Windows 8dot3 path, we convert + // it back to its long path form so that path manipulations (e.g. appending + // ".runfiles") work as expected. Note that GetExecutableFileName may return a + // path different from argv[0]. + const std::wstring launcher_path = + GetWindowsLongPath(GetExecutableFileName()); LaunchDataParser::LaunchInfo launch_info; - if (!LaunchDataParser::GetLaunchInfo(executable_file, &launch_info)) { + if (!LaunchDataParser::GetLaunchInfo(launcher_path, &launch_info)) { die(L"Failed to parse launch info."); } @@ -79,11 +85,13 @@ int wmain(int argc, wchar_t* argv[]) { if (result->second == L"Python") { binary_launcher = make_unique( - launch_info, executable_file, argc, argv); + launch_info, launcher_path, argc, argv); } else if (result->second == L"Bash") { - binary_launcher = make_unique(launch_info, argc, argv); + binary_launcher = + make_unique(launch_info, launcher_path, argc, argv); } else if (result->second == L"Java") { - binary_launcher = make_unique(launch_info, argc, argv); + binary_launcher = + make_unique(launch_info, launcher_path, argc, argv); } else { die(L"Unknown binary type, cannot launch anything."); } diff --git a/src/tools/launcher/python_launcher.cc b/src/tools/launcher/python_launcher.cc index 6a0b487f1a3623..fdfeb0320ae0e1 100644 --- a/src/tools/launcher/python_launcher.cc +++ b/src/tools/launcher/python_launcher.cc @@ -56,14 +56,10 @@ ExitCode PythonBinaryLauncher::Launch() { vector args = this->GetCommandlineArguments(); wstring use_zip_file = this->GetLaunchInfoByKey(USE_ZIP_FILE); wstring python_file; - // In case the given binary path is a shortened Windows 8dot3 path, we need to - // convert it back to its long path form before using it to find the python - // file. - wstring full_binary_path = GetWindowsLongPath(executable_file_); if (use_zip_file == L"1") { - python_file = GetBinaryPathWithoutExtension(full_binary_path) + L".zip"; + python_file = GetBinaryPathWithoutExtension(GetLauncherPath()) + L".zip"; } else { - python_file = GetBinaryPathWithoutExtension(full_binary_path); + python_file = GetBinaryPathWithoutExtension(GetLauncherPath()); } // Replace the first argument with python file path diff --git a/src/tools/launcher/python_launcher.h b/src/tools/launcher/python_launcher.h index eb1ca6d2a17ce8..99fd9018284a21 100644 --- a/src/tools/launcher/python_launcher.h +++ b/src/tools/launcher/python_launcher.h @@ -26,14 +26,11 @@ namespace launcher { class PythonBinaryLauncher : public BinaryLauncherBase { public: PythonBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, - std::wstring executable_file, int argc, wchar_t* argv[]) - : BinaryLauncherBase(launch_info, argc, argv), - executable_file_(std::move(executable_file)) {} + const std::wstring& launcher_path, int argc, + wchar_t* argv[]) + : BinaryLauncherBase(launch_info, launcher_path, argc, argv) {} ~PythonBinaryLauncher() override = default; ExitCode Launch() override; - - private: - std::wstring executable_file_; }; } // namespace launcher