Skip to content

Commit 53e9fea

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
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 bazelbuild#16916. PiperOrigin-RevId: 493615543 Change-Id: Ic8161890181c0110ecdf6893b9835e6f99d01097
1 parent db68419 commit 53e9fea

File tree

11 files changed

+177
-79
lines changed

11 files changed

+177
-79
lines changed

‎site/en/extending/rules.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,8 +717,8 @@ When an executable target is run with `bazel run` (or `test`), the root of the
717717
runfiles directory is adjacent to the executable. The paths relate as follows:
718718

719719
```python
720-
# Given executable_file and runfile_file:
721-
runfiles_root = executable_file.path + ".runfiles"
720+
# Given launcher_path and runfile_file:
721+
runfiles_root = launcher_path.path + ".runfiles"
722722
workspace_name = ctx.workspace_name
723723
runfile_path = runfile_file.short_path
724724
execution_root_relative_path = "%s/%s/%s" % (

‎src/test/py/bazel/launcher_test.py‎

Lines changed: 116 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -616,21 +616,24 @@ def testWindowsNativeLauncherInLongPath(self):
616616
if not self.IsWindows():
617617
return
618618
self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
619-
self.ScratchFile('bin/BUILD', [
620-
'java_binary(',
621-
' name = "bin_java",',
622-
' srcs = ["Main.java"],',
623-
' main_class = "Main",',
624-
')',
625-
'sh_binary(',
626-
' name = "bin_sh",',
627-
' srcs = ["main.sh"],',
628-
')',
629-
'py_binary(',
630-
' name = "bin_py",',
631-
' srcs = ["bin_py.py"],',
632-
')',
633-
])
619+
self.ScratchFile(
620+
'bin/BUILD',
621+
[
622+
'java_binary(',
623+
' name = "not_short_bin_java",',
624+
' srcs = ["Main.java"],',
625+
' main_class = "Main",',
626+
')',
627+
'sh_binary(',
628+
' name = "not_short_bin_sh",',
629+
' srcs = ["main.sh"],',
630+
')',
631+
'py_binary(',
632+
' name = "not_short_bin_py",',
633+
' srcs = ["not_short_bin_py.py"],',
634+
')',
635+
],
636+
)
634637
self.ScratchFile('bin/Main.java', [
635638
'public class Main {',
636639
' public static void main(String[] args) {'
@@ -641,9 +644,12 @@ def testWindowsNativeLauncherInLongPath(self):
641644
self.ScratchFile('bin/main.sh', [
642645
'echo "helloworld"',
643646
])
644-
self.ScratchFile('bin/bin_py.py', [
645-
'print("helloworld")',
646-
])
647+
self.ScratchFile(
648+
'bin/not_short_bin_py.py',
649+
[
650+
'print("helloworld")',
651+
],
652+
)
647653

648654
exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
649655
self.AssertExitCode(exit_code, 0, stderr)
@@ -656,52 +662,133 @@ def testWindowsNativeLauncherInLongPath(self):
656662
long_dir_path = './' + '/'.join(
657663
[(c * 8 + '.' + c * 3) for c in string.ascii_lowercase])
658664

665+
# The 'not_short_' prefix ensures that the basenames are not already 8.3
666+
# short paths. Due to the long directory path, the basename will thus be
667+
# replaced with a short path such as "not_sh~1.exe" below.
659668
for f in [
660-
'bin_java.exe',
661-
'bin_java.exe.runfiles_manifest',
662-
'bin_sh.exe',
663-
'bin_sh',
664-
'bin_sh.exe.runfiles_manifest',
665-
'bin_py.exe',
666-
'bin_py.zip',
667-
'bin_py.exe.runfiles_manifest',
669+
'not_short_bin_java.exe',
670+
'not_short_bin_java.exe.runfiles_manifest',
671+
'not_short_bin_sh.exe',
672+
'not_short_bin_sh',
673+
'not_short_bin_sh.exe.runfiles_manifest',
674+
'not_short_bin_py.exe',
675+
'not_short_bin_py.zip',
676+
'not_short_bin_py.exe.runfiles_manifest',
668677
]:
669678
self.CopyFile(
670679
os.path.join(bazel_bin, 'bin', f), os.path.join(long_dir_path, f))
671680

672-
long_binary_path = os.path.abspath(long_dir_path + '/bin_java.exe')
681+
long_binary_path = os.path.abspath(
682+
long_dir_path + '/not_short_bin_java.exe'
683+
)
673684
# subprocess doesn't support long path without shell=True
674685
exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True)
675686
self.AssertExitCode(exit_code, 0, stderr)
676687
self.assertEqual('helloworld', ''.join(stdout))
677688
# Make sure we can launch the binary with a shortened Windows 8dot3 path
678689
short_binary_path = win32api.GetShortPathName(long_binary_path)
690+
self.assertIn('~', os.path.basename(short_binary_path))
679691
exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True)
680692
self.AssertExitCode(exit_code, 0, stderr)
681693
self.assertEqual('helloworld', ''.join(stdout))
682694

683-
long_binary_path = os.path.abspath(long_dir_path + '/bin_sh.exe')
695+
long_binary_path = os.path.abspath(long_dir_path + '/not_short_bin_sh.exe')
684696
# subprocess doesn't support long path without shell=True
685697
exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True)
686698
self.AssertExitCode(exit_code, 0, stderr)
687699
self.assertEqual('helloworld', ''.join(stdout))
688700
# Make sure we can launch the binary with a shortened Windows 8dot3 path
689701
short_binary_path = win32api.GetShortPathName(long_binary_path)
702+
self.assertIn('~', os.path.basename(short_binary_path))
690703
exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True)
691704
self.AssertExitCode(exit_code, 0, stderr)
692705
self.assertEqual('helloworld', ''.join(stdout))
693706

694-
long_binary_path = os.path.abspath(long_dir_path + '/bin_py.exe')
707+
long_binary_path = os.path.abspath(long_dir_path + '/not_short_bin_py.exe')
695708
# subprocess doesn't support long path without shell=True
696709
exit_code, stdout, stderr = self.RunProgram([long_binary_path], shell=True)
697710
self.AssertExitCode(exit_code, 0, stderr)
698711
self.assertEqual('helloworld', ''.join(stdout))
699712
# Make sure we can launch the binary with a shortened Windows 8dot3 path
700713
short_binary_path = win32api.GetShortPathName(long_binary_path)
714+
self.assertIn('~', os.path.basename(short_binary_path))
701715
exit_code, stdout, stderr = self.RunProgram([short_binary_path], shell=True)
702716
self.AssertExitCode(exit_code, 0, stderr)
703717
self.assertEqual('helloworld', ''.join(stdout))
704718

719+
def testWindowsNativeLauncherInvalidArgv0(self):
720+
if not self.IsWindows():
721+
return
722+
self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
723+
self.ScratchFile(
724+
'bin/BUILD',
725+
[
726+
'java_binary(',
727+
' name = "bin_java",',
728+
' srcs = ["Main.java"],',
729+
' main_class = "Main",',
730+
')',
731+
'sh_binary(',
732+
' name = "bin_sh",',
733+
' srcs = ["main.sh"],',
734+
')',
735+
'py_binary(',
736+
' name = "bin_py",',
737+
' srcs = ["bin_py.py"],',
738+
')',
739+
],
740+
)
741+
self.ScratchFile(
742+
'bin/Main.java',
743+
[
744+
'public class Main {',
745+
(
746+
' public static void main(String[] args) {'
747+
' System.out.println("helloworld");'
748+
),
749+
' }',
750+
'}',
751+
],
752+
)
753+
self.ScratchFile(
754+
'bin/main.sh',
755+
[
756+
'echo "helloworld"',
757+
],
758+
)
759+
self.ScratchFile(
760+
'bin/bin_py.py',
761+
[
762+
'print("helloworld")',
763+
],
764+
)
765+
766+
exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
767+
self.AssertExitCode(exit_code, 0, stderr)
768+
bazel_bin = stdout[0]
769+
770+
exit_code, _, stderr = self.RunBazel(['build', '//bin/...'])
771+
self.AssertExitCode(exit_code, 0, stderr)
772+
773+
exit_code, stdout, stderr = self.RunProgram(
774+
['C:\\Invalid'],
775+
executable=os.path.join(bazel_bin, 'bin', 'bin_java.exe'),
776+
)
777+
self.AssertExitCode(exit_code, 0, stderr)
778+
self.assertEqual('helloworld', ''.join(stdout))
779+
780+
exit_code, stdout, stderr = self.RunProgram(
781+
['C:\\Invalid'], executable=os.path.join(bazel_bin, 'bin', 'bin_sh.exe')
782+
)
783+
self.AssertExitCode(exit_code, 0, stderr)
784+
self.assertEqual('helloworld', ''.join(stdout))
785+
786+
exit_code, stdout, stderr = self.RunProgram(
787+
['C:\\Invalid'], executable=os.path.join(bazel_bin, 'bin', 'bin_py.exe')
788+
)
789+
self.AssertExitCode(exit_code, 0, stderr)
790+
self.assertEqual('helloworld', ''.join(stdout))
791+
705792
def AssertRunfilesManifestContains(self, manifest, entry):
706793
with open(manifest, 'r') as f:
707794
for l in f:

‎src/tools/launcher/bash_launcher.cc‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@ ExitCode BashBinaryLauncher::Launch() {
5252

5353
vector<wstring> origin_args = this->GetCommandlineArguments();
5454
wostringstream bash_command;
55-
// In case the given binary path is a shortened Windows 8dot3 path, we need to
56-
// convert it back to its long path form before using it to find the bash main
57-
// file.
58-
wstring full_binary_path = GetWindowsLongPath(origin_args[0]);
59-
wstring bash_main_file = GetBinaryPathWithoutExtension(full_binary_path);
55+
wstring bash_main_file = GetBinaryPathWithoutExtension(GetLauncherPath());
6056
bash_command << BashEscapeArg(bash_main_file);
6157
for (int i = 1; i < origin_args.size(); i++) {
6258
bash_command << L' ';

‎src/tools/launcher/bash_launcher.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ namespace launcher {
2222

2323
class BashBinaryLauncher : public BinaryLauncherBase {
2424
public:
25-
BashBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, int argc,
25+
BashBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info,
26+
const std::wstring& launcher_path, int argc,
2627
wchar_t* argv[])
27-
: BinaryLauncherBase(launch_info, argc, argv) {}
28+
: BinaryLauncherBase(launch_info, launcher_path, argc, argv) {}
2829
~BashBinaryLauncher() override = default;
2930
ExitCode Launch() override;
3031
};

‎src/tools/launcher/java_launcher.cc‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,7 @@ static void WriteJarClasspath(const wstring& jar_path,
162162
}
163163

164164
wstring JavaBinaryLauncher::GetJunctionBaseDir() {
165-
wstring binary_base_path =
166-
GetBinaryPathWithExtension(this->GetCommandlineArguments()[0]);
165+
wstring binary_base_path = GetBinaryPathWithExtension(GetLauncherPath());
167166
wstring result;
168167
if (!NormalizePath(binary_base_path + L".j", &result)) {
169168
die(L"Failed to get normalized junction base directory.");
@@ -191,8 +190,7 @@ void JavaBinaryLauncher::DeleteJunctionBaseDir() {
191190
}
192191

193192
wstring JavaBinaryLauncher::CreateClasspathJar(const wstring& classpath) {
194-
wstring binary_base_path =
195-
GetBinaryPathWithoutExtension(this->GetCommandlineArguments()[0]);
193+
wstring binary_base_path = GetBinaryPathWithoutExtension(GetLauncherPath());
196194
wstring abs_manifest_jar_dir_norm = GetManifestJarDir(binary_base_path);
197195

198196
wostringstream manifest_classpath;
@@ -312,8 +310,7 @@ ExitCode JavaBinaryLauncher::Launch() {
312310
// Run deploy jar if needed, otherwise generate the CLASSPATH by rlocation.
313311
if (this->singlejar) {
314312
wstring deploy_jar =
315-
GetBinaryPathWithoutExtension(this->GetCommandlineArguments()[0]) +
316-
L"_deploy.jar";
313+
GetBinaryPathWithoutExtension(GetLauncherPath()) + L"_deploy.jar";
317314
if (!DoesFilePathExist(deploy_jar.c_str())) {
318315
die(L"Option --singlejar was passed, but %s does not exist.\n (You may "
319316
"need to build it explicitly.)",

‎src/tools/launcher/java_launcher.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ static const int MAX_ARG_STRLEN = 7000;
2929

3030
class JavaBinaryLauncher : public BinaryLauncherBase {
3131
public:
32-
JavaBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info, int argc,
32+
JavaBinaryLauncher(const LaunchDataParser::LaunchInfo& launch_info,
33+
const std::wstring& launcher_path, int argc,
3334
wchar_t* argv[])
34-
: BinaryLauncherBase(launch_info, argc, argv),
35+
: BinaryLauncherBase(launch_info, launcher_path, argc, argv),
3536
singlejar(false),
3637
print_javabin(false),
3738
classpath_limit(MAX_ARG_STRLEN) {}

‎src/tools/launcher/launcher.cc‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ using std::vector;
4242
using std::wostringstream;
4343
using std::wstring;
4444

45-
static wstring GetRunfilesDir(const wchar_t* argv0) {
45+
static wstring GetRunfilesDir(const wchar_t* launcher_path) {
4646
wstring runfiles_dir;
4747
// If RUNFILES_DIR is already set (probably we are either in a test or in a
4848
// data dependency) then use it.
4949
if (!GetEnv(L"RUNFILES_DIR", &runfiles_dir)) {
5050
// Otherwise this is probably a top-level non-test binary (e.g. a genrule
5151
// tool) and should look for its runfiles beside the executable.
52-
runfiles_dir = GetBinaryPathWithExtension(argv0) + L".runfiles";
52+
runfiles_dir = GetBinaryPathWithExtension(launcher_path) + L".runfiles";
5353
}
5454
// Make sure we return a normalized absolute path.
5555
if (!blaze_util::IsAbsolute(runfiles_dir)) {
@@ -63,10 +63,12 @@ static wstring GetRunfilesDir(const wchar_t* argv0) {
6363
}
6464

6565
BinaryLauncherBase::BinaryLauncherBase(
66-
const LaunchDataParser::LaunchInfo& _launch_info, int argc, wchar_t* argv[])
67-
: launch_info(_launch_info),
68-
manifest_file(FindManifestFile(argv[0])),
69-
runfiles_dir(GetRunfilesDir(argv[0])),
66+
const LaunchDataParser::LaunchInfo& _launch_info,
67+
const std::wstring& launcher_path, int argc, wchar_t* argv[])
68+
: launcher_path(launcher_path),
69+
launch_info(_launch_info),
70+
manifest_file(FindManifestFile(launcher_path.c_str())),
71+
runfiles_dir(GetRunfilesDir(launcher_path.c_str())),
7072
workspace_name(GetLaunchInfoByKey(WORKSPACE_NAME)),
7173
symlink_runfiles_enabled(GetLaunchInfoByKey(SYMLINK_RUNFILES_ENABLED) ==
7274
L"1") {
@@ -81,7 +83,8 @@ BinaryLauncherBase::BinaryLauncherBase(
8183
}
8284
}
8385

84-
static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
86+
static bool FindManifestFileImpl(const wchar_t* launcher_path,
87+
wstring* result) {
8588
// If this binary X runs as the data-dependency of some other binary Y, then
8689
// X has no runfiles manifest/directory and should use Y's.
8790
if (GetEnv(L"RUNFILES_MANIFEST_FILE", result) &&
@@ -100,7 +103,7 @@ static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
100103
// If this binary X runs by itself (not as a data-dependency of another
101104
// binary), then look for the manifest in a runfiles directory next to the
102105
// main binary, then look for it (the manifest) next to the main binary.
103-
directory = GetBinaryPathWithExtension(argv0) + L".runfiles";
106+
directory = GetBinaryPathWithExtension(launcher_path) + L".runfiles";
104107
*result = directory + L"/MANIFEST";
105108
if (DoesFilePathExist(result->c_str())) {
106109
return true;
@@ -114,9 +117,9 @@ static bool FindManifestFileImpl(const wchar_t* argv0, wstring* result) {
114117
return false;
115118
}
116119

117-
wstring BinaryLauncherBase::FindManifestFile(const wchar_t* argv0) {
120+
wstring BinaryLauncherBase::FindManifestFile(const wchar_t* launcher_path) {
118121
wstring manifest_file;
119-
if (!FindManifestFileImpl(argv0, &manifest_file)) {
122+
if (!FindManifestFileImpl(launcher_path, &manifest_file)) {
120123
return L"";
121124
}
122125
// 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) {
125128
return manifest_file;
126129
}
127130

131+
wstring BinaryLauncherBase::GetLauncherPath() const { return launcher_path; }
132+
128133
wstring BinaryLauncherBase::GetRunfilesPath() const {
129134
wstring runfiles_path =
130-
GetBinaryPathWithExtension(this->commandline_arguments[0]) + L".runfiles";
135+
GetBinaryPathWithExtension(launcher_path) + L".runfiles";
131136
std::replace(runfiles_path.begin(), runfiles_path.end(), L'/', L'\\');
132137
return runfiles_path;
133138
}

0 commit comments

Comments
 (0)