Skip to content

Commit

Permalink
Use long executable path instead of argv[0] in all launchers (#17272)
Browse files Browse the repository at this point in the history
`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

Co-authored-by: Fabian Meumertzheim <[email protected]>
  • Loading branch information
ShreeM01 and fmeum authored Jan 20, 2023
1 parent ea49dd4 commit 795779e
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 79 deletions.
4 changes: 2 additions & 2 deletions site/en/extending/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -715,8 +715,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" % (
Expand Down
145 changes: 116 additions & 29 deletions src/test/py/bazel/launcher_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) {'
Expand All @@ -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)
Expand All @@ -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:
Expand Down
6 changes: 1 addition & 5 deletions src/tools/launcher/bash_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,7 @@ ExitCode BashBinaryLauncher::Launch() {

vector<wstring> 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' ';
Expand Down
5 changes: 3 additions & 2 deletions src/tools/launcher/bash_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
9 changes: 3 additions & 6 deletions src/tools/launcher/java_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.)",
Expand Down
5 changes: 3 additions & 2 deletions src/tools/launcher/java_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
27 changes: 16 additions & 11 deletions src/tools/launcher/launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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") {
Expand All @@ -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) &&
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
Loading

0 comments on commit 795779e

Please sign in to comment.