Skip to content

Commit 59a507e

Browse files
authored
Split rustdoc test compilation and execution (#4034)
This change introduces a new `RustdocTestCompile` action which takes advantage of the nightly rustc flag `-Zunstable-options --persist-doctests` to have rustdoc tests written to disk to be executed in a separate action, similar to a normal `rust_test` which first compiles the test and then executes it in a test action. To use this change a nightly toolchain is required and the use of `--@rules_rust//rust/settings:experimental_compile_rustdoc_tests=True`. closes #1431
1 parent 6910b34 commit 59a507e

9 files changed

Lines changed: 642 additions & 28 deletions

File tree

.bazelci/presubmit.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ min_rust_version_shell_commands: &min_rust_version_shell_commands
1010
- sed -i 's|^rust\.toolchain(|rust.toolchain(versions = ["1.85.0"],\n|' MODULE.bazel
1111
nightly_flags: &nightly_flags
1212
- "--//rust/toolchain/channel=nightly"
13+
- "--//rust/settings:experimental_compile_rustdoc_tests=True"
1314
nightly_aspects_flags: &nightly_aspects_flags
1415
- "--//rust/toolchain/channel=nightly"
1516
- "--config=rustfmt"

rust/private/rustdoc.bzl

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ def rustdoc_compile_action(
5555
lints_info = None,
5656
output = None,
5757
rustdoc_flags = [],
58-
is_test = False):
58+
is_test = False,
59+
force_depend_on_objects = None):
5960
"""Create a struct of information needed for a `rustdoc` compile action based on crate passed to the rustdoc rule.
6061
6162
Args:
@@ -64,24 +65,29 @@ def rustdoc_compile_action(
6465
crate_info (CrateInfo): The provider of the crate passed to a rustdoc rule.
6566
lints_info (LintsInfo, optional): The LintsInfo provider of the crate passed to the rustdoc rule.
6667
output (File, optional): An optional output a `rustdoc` action is intended to produce.
67-
rustdoc_flags (list, optional): A list of `rustdoc` specific flags.
68+
rustdoc_flags (Args, optional): An `Args` object of `rustdoc` specific flags.
6869
is_test (bool, optional): If True, the action will be configured for `rust_doc_test` targets
70+
force_depend_on_objects (bool, optional): If set, overrides is_test for controlling whether
71+
to depend on .rlib files instead of .rmeta. Defaults to is_test.
6972
7073
Returns:
7174
struct: A struct of some `ctx.actions.run` arguments.
7275
"""
76+
if force_depend_on_objects == None:
77+
force_depend_on_objects = is_test
7378

7479
# If an output was provided, ensure it's used in rustdoc arguments
7580
if output:
76-
rustdoc_flags = [
77-
"--output",
78-
output.path,
79-
] + rustdoc_flags
81+
rustdoc_flags.add_all(
82+
[output],
83+
before_each = "--output",
84+
expand_directories = False,
85+
)
8086

8187
# Specify rustc flags for lints, if they were provided.
8288
lint_files = []
8389
if lints_info:
84-
rustdoc_flags = rustdoc_flags + lints_info.rustdoc_lint_flags
90+
rustdoc_flags.add_all(lints_info.rustdoc_lint_flags)
8591
lint_files = lint_files + lints_info.rustdoc_lint_files
8692

8793
# Collect HTML customization files
@@ -115,8 +121,7 @@ def rustdoc_compile_action(
115121
dep_info = dep_info,
116122
build_info = build_info,
117123
lint_files = lint_files,
118-
# If this is a rustdoc test, we need to depend on rlibs rather than .rmeta.
119-
force_depend_on_objects = is_test,
124+
force_depend_on_objects = force_depend_on_objects,
120125
include_link_flags = False,
121126
)
122127

@@ -147,7 +152,7 @@ def rustdoc_compile_action(
147152
remap_path_prefix = None,
148153
add_flags_for_binary = True,
149154
include_link_flags = False,
150-
force_depend_on_objects = is_test,
155+
force_depend_on_objects = force_depend_on_objects,
151156
skip_expanding_rustc_env = True,
152157
)
153158

@@ -168,6 +173,7 @@ def rustdoc_compile_action(
168173
inputs = all_inputs,
169174
env = env,
170175
arguments = args.all,
176+
supports_path_mapping = args.supports_path_mapping,
171177
tools = [toolchain.rust_doc],
172178
)
173179

@@ -214,27 +220,28 @@ def _rust_doc_impl(ctx):
214220

215221
output_dir = ctx.actions.declare_directory("{}.rustdoc".format(ctx.label.name))
216222

217-
# Add the current crate as an extern for the compile action
218-
rustdoc_flags = [
219-
"--extern",
220-
"{}={}".format(crate_info.name, crate_info.output.path),
221-
]
223+
rustdoc_flags = ctx.actions.args()
224+
rustdoc_flags.add_all(
225+
[crate_info.output],
226+
format_each = "--extern={}=%s".format(crate_info.name),
227+
expand_directories = False,
228+
)
222229

223230
# Add HTML customization flags if attributes are provided
224231
if ctx.attr.html_in_header:
225-
rustdoc_flags.extend(["--html-in-header", ctx.file.html_in_header.path])
232+
rustdoc_flags.add("--html-in-header", ctx.file.html_in_header)
226233

227234
if ctx.attr.html_before_content:
228-
rustdoc_flags.extend(["--html-before-content", ctx.file.html_before_content.path])
235+
rustdoc_flags.add("--html-before-content", ctx.file.html_before_content)
229236

230237
if ctx.attr.html_after_content:
231-
rustdoc_flags.extend(["--html-after-content", ctx.file.html_after_content.path])
238+
rustdoc_flags.add("--html-after-content", ctx.file.html_after_content)
232239

233240
# Add markdown CSS files if provided
234241
for css_file in ctx.files.markdown_css:
235-
rustdoc_flags.extend(["--markdown-css", css_file.path])
242+
rustdoc_flags.add(["--markdown-css", css_file])
236243

237-
rustdoc_flags.extend(ctx.attr.rustdoc_flags)
244+
rustdoc_flags.add_all(ctx.attr.rustdoc_flags)
238245

239246
action = rustdoc_compile_action(
240247
ctx = ctx,

rust/private/rustdoc/BUILD.bazel

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ load("//rust/private:rust.bzl", "rust_binary")
22

33
package(default_visibility = ["//visibility:public"])
44

5+
rust_binary(
6+
name = "rustdoc_test_runner",
7+
srcs = ["rustdoc_test_runner.rs"],
8+
edition = "2018",
9+
deps = [
10+
"//rust/runfiles",
11+
],
12+
)
13+
514
rust_binary(
615
name = "rustdoc_test_writer",
716
srcs = ["rustdoc_test_writer.rs"],
@@ -10,3 +19,9 @@ rust_binary(
1019
"//rust/runfiles",
1120
],
1221
)
22+
23+
rust_binary(
24+
name = "rustdoc_compile_wrapper",
25+
srcs = ["rustdoc_compile_wrapper.rs"],
26+
edition = "2018",
27+
)
Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
//! Wrapper around the `rustdoc --test --no-run --persist-doctests` invocation
2+
//! used by `rust_doc_test` when the `experimental_compile_rustdoc_tests` flag
3+
//! is enabled.
4+
//!
5+
//! Doc test compilation is split into a build-time compile action (this
6+
//! wrapper) and a test-time execute action (`rustdoc_test_runner`). At
7+
//! compile time rustdoc both (a) writes one `rust_out` binary per doc test
8+
//! into a `--persist-doctests` directory and (b) prints a `test ... ` line
9+
//! per test to stdout. The runner only sees the persisted directory tree, so
10+
//! it cannot recover the human-readable test names on its own.
11+
//!
12+
//! This wrapper bridges that gap: it spawns rustdoc, captures stdout, and
13+
//! writes a `<mangled-dir-name>=<human-name>` metadata file that the runner
14+
//! consults to label each persisted binary. It also suppresses rustdoc's
15+
//! stdout on clean runs (to keep `bazel test` output tidy) while still
16+
//! surfacing it whenever the compile fails or emits warnings.
17+
18+
use std::collections::{BTreeSet, HashMap};
19+
use std::env;
20+
use std::fs;
21+
use std::io::{self, BufRead, Read, Write};
22+
use std::process::{exit, Command, Stdio};
23+
use std::thread;
24+
25+
/// Parsed command-line for this wrapper.
26+
struct WrapperArgs {
27+
/// Where to write the `<mangled>=<human>` test-name metadata, or `None`
28+
/// if `--test-metadata` was not passed (in which case stdout is not
29+
/// parsed for test names).
30+
test_metadata_path: Option<String>,
31+
/// The rustdoc command and its arguments, everything that appeared
32+
/// after the `--` separator.
33+
child_args: Vec<String>,
34+
}
35+
36+
impl WrapperArgs {
37+
/// Parse command line arguments.
38+
fn parse() -> Self {
39+
let mut test_metadata_path: Option<String> = None;
40+
let mut child_args: Vec<String> = Vec::new();
41+
let mut past_separator = false;
42+
43+
let mut args_iter = env::args().skip(1);
44+
while let Some(arg) = args_iter.next() {
45+
if past_separator {
46+
child_args.push(arg);
47+
} else if arg == "--" {
48+
past_separator = true;
49+
} else if arg == "--test-metadata" {
50+
test_metadata_path = args_iter.next();
51+
} else {
52+
eprintln!("Unknown wrapper flag: {}", arg);
53+
exit(1);
54+
}
55+
}
56+
57+
Self {
58+
test_metadata_path,
59+
child_args,
60+
}
61+
}
62+
}
63+
64+
/// Extracts the human-readable test names from rustdoc's stdout.
65+
///
66+
/// rustdoc prints one `test <name> ... <status>` line per doc test when
67+
/// invoked with `--test`. We don't care about `<status>` here (the binaries
68+
/// haven't been run yet — `--no-run` is in effect); we only need `<name>`,
69+
/// which has the form `<file> - <item> (line <n>)`.
70+
///
71+
/// Returns names in the order rustdoc emitted them, which is the order
72+
/// `mangle_test_name` relies on for assigning per-(file, line) suffixes.
73+
fn parse_test_names(stdout: &str) -> Vec<String> {
74+
stdout
75+
.lines()
76+
.filter_map(|line| {
77+
let rest = line.strip_prefix("test ")?;
78+
let name = rest.rsplit_once(" ... ")?.0;
79+
Some(name.to_string())
80+
})
81+
.collect()
82+
}
83+
84+
/// Reproduces the directory name rustdoc uses under `--persist-doctests`
85+
/// for a given human-readable test name.
86+
///
87+
/// rustdoc's persisted directories are named `<sanitized-file>_<line>_<n>`,
88+
/// where non-alphanumeric characters in the file path are replaced with `_`
89+
/// and `<n>` is a per-(file, line) sequence number that increments when
90+
/// macro expansion produces multiple tests at the same source location.
91+
///
92+
/// `counts` must be threaded across every call within the same rustdoc
93+
/// invocation so the indices match the order rustdoc assigned them. If the
94+
/// input doesn't match the expected `<file> - <item> (line <n>)` shape we
95+
/// fall back to a plain alphanumeric sanitization of the whole name.
96+
fn mangle_test_name(human_name: &str, counts: &mut HashMap<(String, String), usize>) -> String {
97+
if let Some((file_and_item, line_part)) = human_name.rsplit_once(" (line ") {
98+
if let Some(line_num) = line_part.strip_suffix(')') {
99+
if let Some((file_path, _)) = file_and_item.split_once(" - ") {
100+
let mangled: String = file_path
101+
.chars()
102+
.map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
103+
.collect();
104+
// Macro expansion can produce multiple doc tests that share
105+
// the same source file and line. rustdoc assigns each one a
106+
// sequential suffix in the order it emits them, so we mirror
107+
// that here to keep the mangled name in sync with the
108+
// persisted directory name.
109+
let key = (mangled.clone(), line_num.to_string());
110+
let index = counts.entry(key).or_insert(0);
111+
let result = format!("{}_{}_{}", mangled, line_num, *index);
112+
*index += 1;
113+
return result;
114+
}
115+
}
116+
}
117+
human_name
118+
.chars()
119+
.map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
120+
.collect()
121+
}
122+
123+
/// Writes the `<mangled-dir-name>=<human-name>` mapping consumed by
124+
/// `rustdoc_test_runner`.
125+
///
126+
/// Lines are sorted by mangled name (via `BTreeSet`) to keep the output
127+
/// stable across builds, even though the per-(file, line) suffixes
128+
/// themselves are still assigned in rustdoc-emit order. Write failures are
129+
/// intentionally swallowed — the runner falls back to using the directory
130+
/// name as the test name if the metadata file is missing or malformed.
131+
fn write_test_metadata(path: &str, stdout: &str) {
132+
let names = parse_test_names(stdout);
133+
let mut counts: HashMap<(String, String), usize> = HashMap::new();
134+
let entries: BTreeSet<(String, &str)> = names
135+
.iter()
136+
.map(|name| (mangle_test_name(name, &mut counts), name.as_str()))
137+
.collect();
138+
139+
let mut content = String::new();
140+
for (mangled, human) in &entries {
141+
content.push_str(mangled);
142+
content.push('=');
143+
content.push_str(human);
144+
content.push('\n');
145+
}
146+
let _ = fs::write(path, content);
147+
}
148+
149+
/// Returns true if a stderr line looks like a rustdoc warning.
150+
///
151+
/// We treat any line containing `warning:` as a warning; this is the
152+
/// trigger that makes us replay buffered stdout even on a successful
153+
/// build, so warnings printed to stdout don't get silently dropped.
154+
fn line_has_warning(line: &[u8]) -> bool {
155+
contains_subslice(line, b"warning:")
156+
}
157+
158+
/// Naive byte-level substring search, used by `line_has_warning` to avoid
159+
/// allocating a `String` for every stderr line.
160+
fn contains_subslice(haystack: &[u8], needle: &[u8]) -> bool {
161+
haystack
162+
.windows(needle.len())
163+
.any(|window| window == needle)
164+
}
165+
166+
fn main() {
167+
let debug = env::var_os("RULES_RUST_RUSTDOC_DEBUG").is_some();
168+
let args = WrapperArgs::parse();
169+
170+
if args.child_args.is_empty() {
171+
eprintln!("Usage: rustdoc_compile_wrapper [--test-metadata FILE] -- <command> [args...]");
172+
exit(1);
173+
}
174+
175+
let mut child = Command::new(&args.child_args[0])
176+
.args(&args.child_args[1..])
177+
.stdout(if debug {
178+
Stdio::inherit()
179+
} else {
180+
Stdio::piped()
181+
})
182+
.stderr(Stdio::piped())
183+
.spawn()
184+
.unwrap_or_else(|e| {
185+
eprintln!("Failed to spawn {}: {}", args.child_args[0], e);
186+
exit(1);
187+
});
188+
189+
let child_stdout = child.stdout.take();
190+
let child_stderr = child.stderr.take().unwrap();
191+
192+
let stdout_handle = thread::spawn(move || {
193+
let mut buf = Vec::new();
194+
if let Some(mut reader) = child_stdout {
195+
let _ = reader.read_to_end(&mut buf);
196+
}
197+
buf
198+
});
199+
200+
let stderr_handle = thread::spawn(move || {
201+
let reader = io::BufReader::new(child_stderr);
202+
let mut stderr = io::stderr().lock();
203+
let mut has_warning = false;
204+
for line in reader.split(b'\n') {
205+
let line = match line {
206+
Ok(l) => l,
207+
Err(_) => break,
208+
};
209+
if !has_warning && line_has_warning(&line) {
210+
has_warning = true;
211+
}
212+
let _ = stderr.write_all(&line);
213+
let _ = stderr.write_all(b"\n");
214+
}
215+
has_warning
216+
});
217+
218+
let stdout_buf = stdout_handle.join().unwrap_or_default();
219+
let has_warning = stderr_handle.join().unwrap_or(false);
220+
221+
let status = child.wait().unwrap_or_else(|e| {
222+
eprintln!("Failed to wait for child process: {}", e);
223+
exit(1);
224+
});
225+
226+
if let Some(ref path) = args.test_metadata_path {
227+
let stdout_str = String::from_utf8_lossy(&stdout_buf);
228+
write_test_metadata(path, &stdout_str);
229+
}
230+
231+
let code = status.code().unwrap_or(1);
232+
if !debug && (code != 0 || has_warning) && !stdout_buf.is_empty() {
233+
let _ = io::stderr().write_all(&stdout_buf);
234+
}
235+
236+
exit(code);
237+
}

0 commit comments

Comments
 (0)