Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use long executable path instead of argv[0] in all launchers #16916

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions site/en/extending/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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" % (
Expand Down
94 changes: 78 additions & 16 deletions src/test/py/bazel/launcher_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,17 +618,17 @@ def testWindowsNativeLauncherInLongPath(self):
self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
self.ScratchFile('bin/BUILD', [
'java_binary(',
' name = "bin_java",',
' name = "not_short_bin_java",',
' srcs = ["Main.java"],',
' main_class = "Main",',
')',
'sh_binary(',
' name = "bin_sh",',
' name = "not_short_bin_sh",',
' srcs = ["main.sh"],',
')',
'py_binary(',
' name = "bin_py",',
' srcs = ["bin_py.py"],',
' name = "not_short_bin_py",',
' srcs = ["not_short_bin_py.py"],',
')',
])
self.ScratchFile('bin/Main.java', [
Expand All @@ -641,7 +641,7 @@ def testWindowsNativeLauncherInLongPath(self):
self.ScratchFile('bin/main.sh', [
'echo "helloworld"',
])
self.ScratchFile('bin/bin_py.py', [
self.ScratchFile('bin/not_short_bin_py.py', [
'print("helloworld")',
])

Expand All @@ -656,52 +656,114 @@ def testWindowsNativeLauncherInLongPath(self):
long_dir_path = './' + '/'.join(
[(c * 8 + '.' + c * 3) for c in string.ascii_lowercase])

fmeum marked this conversation as resolved.
Show resolved Hide resolved
# 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
6 changes: 4 additions & 2 deletions src/tools/launcher/bash_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ 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
8 changes: 3 additions & 5 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,7 +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]) +
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 "
Expand Down
6 changes: 4 additions & 2 deletions src/tools/launcher/java_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ 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
33 changes: 21 additions & 12 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,13 +63,17 @@ 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") {
L"1") {
for (int i = 0; i < argc; i++) {
commandline_arguments.push_back(argv[i]);
}
Expand All @@ -81,7 +85,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 +105,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 +119,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 +130,13 @@ 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
19 changes: 15 additions & 4 deletions src/tools/launcher/launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ class BinaryLauncherBase {
typedef std::unordered_map<std::wstring, std::wstring> 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;
Expand Down Expand Up @@ -77,13 +79,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;

Expand Down Expand Up @@ -127,7 +138,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
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,
Expand Down
Loading