Skip to content

Commit 13502b7

Browse files
chandlercjonmeow
andauthored
Replace #4505 with a different set of workarounds (#4527)
This restores the symlinks for the installation, but teaches the busybox info search to look for a relative path to the busybox binary itself before walking through symlinks. This let's it find the tree structure when directly invoking `prefix_root/bin/carbon` or similar, either inside of a Bazel rule or from the command line, and mirrors how we expect the installed tree to look. This works even when Bazel resolves the symlink target fully, and potentially to something nonsensical like a CAS file. In order to make a convenient Bazel target that can be used with `bazel run //toolchain`, this adds an override to explicitly set the desired argv[0] to use when selecting a mode for the busybox and a busybox binary. Currently, the workaround uses an environment variable because that required the least amount of plumbing, and seems a useful override mechanism generally, but I'm open to other approaches. This should allow a few things to work a bit more nicely: - It should handle sibling symlinks like `clang++` to `clang` or `ld.lld` to `lld`, where that symlink in turn points at the busybox. We want to use *initial* `argv[0]` value to select the mode there. - It avoids bouncing through Python (or other subprocesses) when invoking the `carbon` binary in Bazel rules, which will be nice for building the example code and benchmarking. It does come at a cost of removing one feature: the initial symlink can't be some unrelated alias like `my_carbon_symlink` -- we expect the *first* argv[0] name to have the meaningful filename for selecting a busybox mode. It also trades the complexity of the Python script for some complexity in the busybox search in order to look for a relative `carbon-busybox` binary. On the whole, I think that tradeoff is worthwhile, but it isn't free. --------- Co-authored-by: Jon Ross-Perkins <[email protected]>
1 parent 85ea848 commit 13502b7

16 files changed

+292
-216
lines changed

.vscode/gdb_launch.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"type": "by-gdb",
1515
"request": "launch",
1616
"name": "carbon compile (gdb)",
17-
"program": "bazel-bin/toolchain/install/prefix_root/lib/carbon/carbon-busybox",
17+
"program": "bazel-bin/toolchain/carbon",
1818
"programArgs": "compile --phase=lower --dump-sem-ir --stream-errors ${relativeFile}",
1919
"cwd": "${workspaceFolder}"
2020
}

.vscode/lldb_launch.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"type": "lldb-dap",
1919
"request": "launch",
2020
"name": "carbon compile (lldb)",
21-
"program": "bazel-bin/toolchain/install/prefix_root/lib/carbon/carbon-busybox",
21+
"program": "bazel-bin/toolchain/carbon",
2222
"args": [
2323
"compile",
2424
"--phase=lower",

docs/project/contribution_tools.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ bazel build -c dbg //toolchain
345345
Then debugging works with LLDB:
346346

347347
```shell
348-
lldb bazel-bin/toolchain/install/prefix_root/lib/carbon/carbon-busybox
348+
lldb bazel-bin/toolchain/carbon
349349
```
350350

351351
Any installed version of LLDB at least as recent as the installed Clang used for

toolchain/BUILD

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,21 @@
22
# Exceptions. See /LICENSE for license information.
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

5+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
6+
load("run_tool.bzl", "run_tool")
7+
8+
# Support `bazel run` and create a convenience symlink for the tool in the
9+
# toolchain's install.
10+
run_tool(
11+
name = "carbon",
12+
data = ["//toolchain/install:install_data"],
13+
env = cc_env(),
14+
tool = "//toolchain/install:prefix_root/bin/carbon",
15+
)
16+
517
# A convenience target for running the toolchain with the full prelude
618
# available.
719
alias(
820
name = "toolchain",
9-
actual = "//toolchain/install:run_carbon",
21+
actual = ":carbon",
1022
)

toolchain/docs/adding_features.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,10 @@ example, with `toolchain/parse/testdata/basics/empty.carbon`:
467467
- Executes an individual test.
468468
- `bazel run //toolchain -- compile --phase=parse --dump-parse-tree toolchain/parse/testdata/basics/empty.carbon`
469469
- Explicitly runs `carbon` with the provided arguments.
470-
- `bazel-bin/toolchain/install/run_carbon compile --phase=parse --dump-parse-tree toolchain/parse/testdata/basics/empty.carbon`
471-
- Similar to the previous command, but without using `bazel`.
470+
- `bazel-bin/toolchain/carbon compile --phase=parse --dump-parse-tree toolchain/parse/testdata/basics/empty.carbon`
471+
- Similar to the previous command, but without using `bazel run`. This can
472+
be useful with a debugger or other tool that needs to directly run the
473+
binary.
472474
- `bazel run //toolchain -- -v compile --phase=check toolchain/check/testdata/basics/run.carbon`
473475
- Runs using `-v` for verbose log output, and running through the `check`
474476
phase.

toolchain/install/BUILD

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
66
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
77
load("//bazel/manifest:defs.bzl", "manifest")
8-
load("install_filegroups.bzl", "install_busybox_wrapper", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups")
8+
load("install_filegroups.bzl", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups")
99
load("pkg_helpers.bzl", "pkg_naming_variables", "pkg_tar_and_test")
10-
load("run_tool.bzl", "run_tool")
1110

1211
package(default_visibility = ["//visibility:public"])
1312

@@ -89,6 +88,7 @@ cc_test(
8988
"//common:check",
9089
"//testing/base:gtest_main",
9190
"@googletest//:gtest",
91+
"@llvm-project//llvm:Support",
9292
],
9393
)
9494

@@ -130,9 +130,10 @@ lld_aliases = [
130130
# based on the FHS (Filesystem Hierarchy Standard).
131131
install_dirs = {
132132
"bin": [
133-
install_busybox_wrapper(
133+
install_symlink(
134134
"carbon",
135135
"../lib/carbon/carbon-busybox",
136+
is_driver = True,
136137
),
137138
],
138139
"lib/carbon": [
@@ -151,13 +152,10 @@ install_dirs = {
151152
"@llvm-project//lld:lld",
152153
executable = True,
153154
),
154-
install_busybox_wrapper(
155+
install_symlink(
155156
"clang",
156157
"../../carbon-busybox",
157-
[
158-
"clang",
159-
"--",
160-
],
158+
is_driver = True,
161159
),
162160
] + [install_symlink(name, "lld") for name in lld_aliases],
163161
}
@@ -194,11 +192,3 @@ pkg_tar_and_test(
194192
package_variables = ":packaging_variables",
195193
stamp = -1, # Allow `--stamp` builds to produce file timestamps.
196194
)
197-
198-
# Support `bazel run` on specific binaries.
199-
run_tool(
200-
name = "run_carbon",
201-
data = [":install_data"],
202-
env = cc_env(),
203-
tool = "prefix_root/bin/carbon",
204-
)

toolchain/install/busybox_info.h

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#define CARBON_TOOLCHAIN_INSTALL_BUSYBOX_INFO_H_
77

88
#include <filesystem>
9+
#include <iterator>
910
#include <optional>
1011
#include <string>
1112

@@ -14,31 +15,85 @@
1415

1516
namespace Carbon {
1617

18+
constexpr const char* Argv0OverrideEnv = "CARBON_ARGV0_OVERRIDE";
19+
1720
struct BusyboxInfo {
1821
// The path to `carbon-busybox`.
1922
std::filesystem::path bin_path;
2023
// The mode, such as `carbon` or `clang`.
2124
std::optional<std::string> mode;
2225
};
2326

24-
// Returns the busybox information, given argv[0]. This primarily handles
25-
// resolving symlinks that point at the busybox.
27+
// Returns the busybox information, given argv[0].
28+
//
29+
// Extracts the desired mode for the busybox from the initial command name.
30+
//
31+
// Checks if the path in argv0 is an executable in a valid Carbon install, or a
32+
// symlink to such an executable, and sets `bin_path` to the path of
33+
// `lib/carbon/carbon-busybox` within that install.
34+
//
35+
// If unable to locate a plausible busybox binary, returns an error instead.
2636
inline auto GetBusyboxInfo(llvm::StringRef argv0) -> ErrorOr<BusyboxInfo> {
27-
BusyboxInfo info = BusyboxInfo{argv0.str(), std::nullopt};
37+
// Check for an override of `argv[0]` from the environment and apply it.
38+
if (const char* argv0_override = getenv(Argv0OverrideEnv)) {
39+
argv0 = argv0_override;
40+
}
41+
42+
BusyboxInfo info = {.bin_path = argv0.str(), .mode = std::nullopt};
43+
std::filesystem::path filename = info.bin_path.filename();
44+
// The mode is set to the initial filename used for `argv[0]`.
45+
if (filename != "carbon" && filename != "carbon-busybox") {
46+
info.mode = filename;
47+
}
48+
49+
// Now search through any symlinks to locate the installed busybox binary.
2850
while (true) {
29-
std::string filename = info.bin_path.filename();
51+
filename = info.bin_path.filename();
3052
if (filename == "carbon-busybox") {
3153
return info;
3254
}
55+
56+
// If we've not already reached the busybox, look for it relative to the
57+
// current binary path. This can help more immediately locate an
58+
// installation tree, and avoids walking through a final layer of symlinks
59+
// which may point to content-addressed storage or other parts of a build
60+
// output tree.
61+
//
62+
// We break this into two cases we need to handle:
63+
// - Carbon's CLI will be: `<prefix>/bin/carbon`
64+
// - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
65+
//
66+
// We also check that the current path is within a `bin` directory to
67+
// provide best-effort checking for accidentally walking up from symlinks
68+
// that aren't within an installation-shaped tree.
69+
auto parent_path = info.bin_path.parent_path();
70+
// Strip any `.` path components at the end to simplify processing.
71+
while (parent_path.filename() == ".") {
72+
parent_path = parent_path.parent_path();
73+
}
74+
if (parent_path.filename() == "bin") {
75+
auto lib_path = filename == "carbon"
76+
? parent_path / ".." / "lib" / "carbon"
77+
: parent_path / ".." / "..";
78+
auto busybox_path = lib_path / "carbon-busybox";
79+
std::error_code ec;
80+
if (std::filesystem::exists(busybox_path, ec)) {
81+
info.bin_path = busybox_path;
82+
return info;
83+
}
84+
}
85+
86+
// Try to walk through another layer of symlinks and see if we can find the
87+
// installation there or are linked directly to the busybox.
3388
std::error_code ec;
3489
auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec);
3590
if (ec) {
3691
return ErrorBuilder()
3792
<< "expected carbon-busybox symlink at `" << info.bin_path << "`";
3893
}
39-
info.mode = filename;
94+
4095
// Do a path join, to handle relative symlinks.
41-
info.bin_path = info.bin_path.parent_path() / symlink_target;
96+
info.bin_path = parent_path / symlink_target;
4297
}
4398
}
4499

0 commit comments

Comments
 (0)