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.

Sign up for GitHub

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

Jump to bottom

rustdoc --test: Prevent reaching the maximum size of command-line by using files for arguments if there are too many #122840

Merged
merged 6 commits into from
Mar 24, 2024
Merged

rustdoc --test: Prevent reaching the maximum size of command-line by using files for arguments if there are too many #122840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0d3ef80
`rustdoc --test`: Prevent reaching the maximum size of command-line b…
GuillaumeGomez Mar 21, 2024
33735d5
Prevent cloning more than necessary `RustdocOptions` fields
GuillaumeGomez Mar 22, 2024
86fe400
Put temporary directory into one common function
GuillaumeGomez Mar 23, 2024
773084f
Rename `RustdocTestOptions` into `IndividualTestOptions`
GuillaumeGomez Mar 23, 2024
5fab016
Add `Rustdoc` into `run-make-support`
GuillaumeGomez Mar 23, 2024
bc4f169
Add regression test for #122722
GuillaumeGomez Mar 23, 2024
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
213 changes: 141 additions & 72 deletions src/librustdoc/doctest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ use rustc_span::source_map::SourceMap;
use rustc_span::symbol::sym;
use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP};
use rustc_target::spec::{Target, TargetTriple};
use tempfile::Builder as TempFileBuilder;

use std::env;
use std::fs::File;
use std::io::{self, Write};
use std::panic;
use std::path::{Path, PathBuf};
Expand All @@ -31,6 +31,8 @@ use std::str;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex};

use tempfile::{Builder as TempFileBuilder, TempDir};

use crate::clean::{types::AttributesExt, Attributes};
use crate::config::Options as RustdocOptions;
use crate::html::markdown::{self, ErrorCodes, Ignore, LangString};
Expand All @@ -48,7 +50,55 @@ pub(crate) struct GlobalTestOptions {
pub(crate) attrs: Vec<String>,
}

pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> Result<(), String> {
let mut file = File::create(file_path)
.map_err(|error| format!("failed to create args file: {error:?}"))?;

// We now put the common arguments into the file we created.
let mut content = vec!["--crate-type=bin".to_string()];

for cfg in &options.cfgs {
content.push(format!("--cfg={cfg}"));
}
if !options.check_cfgs.is_empty() {
content.push("-Zunstable-options".to_string());
for check_cfg in &options.check_cfgs {
content.push(format!("--check-cfg={check_cfg}"));
}
}

if let Some(sysroot) = &options.maybe_sysroot {
content.push(format!("--sysroot={}", sysroot.display()));
}
for lib_str in &options.lib_strs {
content.push(format!("-L{lib_str}"));
}
for extern_str in &options.extern_strs {
content.push(format!("--extern={extern_str}"));
}
content.push("-Ccodegen-units=1".to_string());
for codegen_options_str in &options.codegen_options_strs {
content.push(format!("-C{codegen_options_str}"));
}
for unstable_option_str in &options.unstable_opts_strs {
content.push(format!("-Z{unstable_option_str}"));
}

let content = content.join("\n");

file.write(content.as_bytes())
.map_err(|error| format!("failed to write arguments to temporary file: {error:?}"))?;
Ok(())
}

fn get_doctest_dir() -> io::Result<TempDir> {
TempFileBuilder::new().prefix("rustdoctest").tempdir()
}

pub(crate) fn run(
dcx: &rustc_errors::DiagCtxt,
options: RustdocOptions,
) -> Result<(), ErrorGuaranteed> {
let input = config::Input::File(options.input.clone());

let invalid_codeblock_attributes_name = crate::lint::INVALID_CODEBLOCK_ATTRIBUTES.name;
Expand Down Expand Up @@ -118,6 +168,15 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
let externs = options.externs.clone();
let json_unused_externs = options.json_unused_externs;

let temp_dir = match get_doctest_dir()
.map_err(|error| format!("failed to create temporary directory: {error:?}"))
{
Ok(temp_dir) => temp_dir,
Err(error) => return crate::wrap_return(dcx, Err(error)),
};
let file_path = temp_dir.path().join("rustdoc-cfgs");
crate::wrap_return(dcx, generate_args_file(&file_path, &options))?;

let (tests, unused_extern_reports, compiling_test_count) =
interface::run_compiler(config, |compiler| {
compiler.enter(|queries| {
Expand All @@ -134,6 +193,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
Some(compiler.sess.psess.clone_source_map()),
None,
enable_per_target_ignores,
file_path,
);

let mut hir_collector = HirCollector {
Expand Down Expand Up @@ -322,74 +382,53 @@ fn run_test(
test: &str,
crate_name: &str,
line: usize,
rustdoc_options: RustdocOptions,
rustdoc_options: IndividualTestOptions,
mut lang_string: LangString,
no_run: bool,
runtool: Option<String>,
runtool_args: Vec<String>,
target: TargetTriple,
opts: &GlobalTestOptions,
edition: Edition,
outdir: DirState,
path: PathBuf,
test_id: &str,
report_unused_externs: impl Fn(UnusedExterns),
) -> Result<(), TestFailure> {
let (test, line_offset, supports_color) =
make_test(test, Some(crate_name), lang_string.test_harness, opts, edition, Some(test_id));
let (test, line_offset, supports_color) = make_test(
test,
Some(crate_name),
lang_string.test_harness,
opts,
edition,
Some(&rustdoc_options.test_id),
);

// Make sure we emit well-formed executable names for our target.
let rust_out = add_exe_suffix("rust_out".to_owned(), &target);
let output_file = outdir.path().join(rust_out);
let rust_out = add_exe_suffix("rust_out".to_owned(), &rustdoc_options.target);
let output_file = rustdoc_options.outdir.path().join(rust_out);

let rustc_binary = rustdoc_options
.test_builder
.as_deref()
.unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc"));
let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary);
compiler.arg("--crate-type").arg("bin");
for cfg in &rustdoc_options.cfgs {
compiler.arg("--cfg").arg(&cfg);
}
if !rustdoc_options.check_cfgs.is_empty() {
compiler.arg("-Z").arg("unstable-options");
for check_cfg in &rustdoc_options.check_cfgs {
compiler.arg("--check-cfg").arg(&check_cfg);
}
}
if let Some(sysroot) = rustdoc_options.maybe_sysroot {
compiler.arg("--sysroot").arg(sysroot);
}

compiler.arg(&format!("@{}", rustdoc_options.arg_file.display()));

compiler.arg("--edition").arg(&edition.to_string());
compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", path);
compiler.env("UNSTABLE_RUSTDOC_TEST_LINE", format!("{}", line as isize - line_offset as isize));
compiler.arg("-o").arg(&output_file);
if lang_string.test_harness {
compiler.arg("--test");
}
if rustdoc_options.json_unused_externs.is_enabled() && !lang_string.compile_fail {
if rustdoc_options.is_json_unused_externs_enabled && !lang_string.compile_fail {
compiler.arg("--error-format=json");
compiler.arg("--json").arg("unused-externs");
compiler.arg("-Z").arg("unstable-options");
compiler.arg("-W").arg("unused_crate_dependencies");
compiler.arg("-Z").arg("unstable-options");
}
for lib_str in &rustdoc_options.lib_strs {
compiler.arg("-L").arg(&lib_str);
}
for extern_str in &rustdoc_options.extern_strs {
compiler.arg("--extern").arg(&extern_str);
}
compiler.arg("-Ccodegen-units=1");
for codegen_options_str in &rustdoc_options.codegen_options_strs {
compiler.arg("-C").arg(&codegen_options_str);
}
for unstable_option_str in &rustdoc_options.unstable_opts_strs {
compiler.arg("-Z").arg(&unstable_option_str);
}
if no_run && !lang_string.compile_fail && rustdoc_options.persist_doctests.is_none() {

if no_run && !lang_string.compile_fail && rustdoc_options.should_persist_doctests {
compiler.arg("--emit=metadata");
}
compiler.arg("--target").arg(match target {
compiler.arg("--target").arg(match rustdoc_options.target {
TargetTriple::TargetTriple(s) => s,
TargetTriple::TargetJson { path_for_rustdoc, .. } => {
path_for_rustdoc.to_str().expect("target path must be valid unicode").to_string()
Expand Down Expand Up @@ -485,10 +524,10 @@ fn run_test(
let mut cmd;

let output_file = make_maybe_absolute_path(output_file);
if let Some(tool) = runtool {
if let Some(tool) = rustdoc_options.runtool {
let tool = make_maybe_absolute_path(tool.into());
cmd = Command::new(tool);
cmd.args(runtool_args);
cmd.args(rustdoc_options.runtool_args);
cmd.arg(output_file);
} else {
cmd = Command::new(output_file);
Expand Down Expand Up @@ -897,6 +936,56 @@ fn partition_source(s: &str, edition: Edition) -> (String, String, String) {
(before, after, crates)
}

pub(crate) struct IndividualTestOptions {
test_builder: Option<PathBuf>,
test_builder_wrappers: Vec<PathBuf>,
is_json_unused_externs_enabled: bool,
should_persist_doctests: bool,
error_format: ErrorOutputType,
test_run_directory: Option<PathBuf>,
nocapture: bool,
arg_file: PathBuf,
outdir: DirState,
runtool: Option<String>,
runtool_args: Vec<String>,
target: TargetTriple,
test_id: String,
}

impl IndividualTestOptions {
fn new(options: &RustdocOptions, arg_file: &Path, test_id: String) -> Self {
let outdir = if let Some(ref path) = options.persist_doctests {
let mut path = path.clone();
path.push(&test_id);

if let Err(err) = std::fs::create_dir_all(&path) {
eprintln!("Couldn't create directory for doctest executables: {err}");
panic::resume_unwind(Box::new(()));
}

DirState::Perm(path)
} else {
DirState::Temp(get_doctest_dir().expect("rustdoc needs a tempdir"))
};

Self {
test_builder: options.test_builder.clone(),
test_builder_wrappers: options.test_builder_wrappers.clone(),
is_json_unused_externs_enabled: options.json_unused_externs.is_enabled(),
should_persist_doctests: options.persist_doctests.is_none(),
error_format: options.error_format,
test_run_directory: options.test_run_directory.clone(),
nocapture: options.nocapture,
arg_file: arg_file.into(),
outdir,
runtool: options.runtool.clone(),
runtool_args: options.runtool_args.clone(),
target: options.target.clone(),
test_id,
}
}
}

pub(crate) trait Tester {
fn add_test(&mut self, test: String, config: LangString, line: usize);
fn get_line(&self) -> usize {
Expand Down Expand Up @@ -941,6 +1030,7 @@ pub(crate) struct Collector {
visited_tests: FxHashMap<(String, usize), usize>,
unused_extern_reports: Arc<Mutex<Vec<UnusedExterns>>>,
compiling_test_count: AtomicUsize,
arg_file: PathBuf,
}

impl Collector {
Expand All @@ -952,6 +1042,7 @@ impl Collector {
source_map: Option<Lrc<SourceMap>>,
filename: Option<PathBuf>,
enable_per_target_ignores: bool,
arg_file: PathBuf,
) -> Collector {
Collector {
tests: Vec::new(),
Expand All @@ -967,6 +1058,7 @@ impl Collector {
visited_tests: FxHashMap::default(),
unused_extern_reports: Default::default(),
compiling_test_count: AtomicUsize::new(0),
arg_file,
}
}

Expand Down Expand Up @@ -1009,13 +1101,9 @@ impl Tester for Collector {
let crate_name = self.crate_name.clone();
let opts = self.opts.clone();
let edition = config.edition.unwrap_or(self.rustdoc_options.edition);
let rustdoc_options = self.rustdoc_options.clone();
let runtool = self.rustdoc_options.runtool.clone();
let runtool_args = self.rustdoc_options.runtool_args.clone();
let target = self.rustdoc_options.target.clone();
let target_str = target.to_string();
let target_str = self.rustdoc_options.target.to_string();
let unused_externs = self.unused_extern_reports.clone();
let no_run = config.no_run || rustdoc_options.no_run;
let no_run = config.no_run || self.rustdoc_options.no_run;
if !config.compile_fail {
self.compiling_test_count.fetch_add(1, Ordering::SeqCst);
}
Expand Down Expand Up @@ -1049,23 +1137,9 @@ impl Tester for Collector {
self.visited_tests.entry((file.clone(), line)).and_modify(|v| *v += 1).or_insert(0)
},
);
let outdir = if let Some(mut path) = rustdoc_options.persist_doctests.clone() {
path.push(&test_id);

if let Err(err) = std::fs::create_dir_all(&path) {
eprintln!("Couldn't create directory for doctest executables: {err}");
panic::resume_unwind(Box::new(()));
}

DirState::Perm(path)
} else {
DirState::Temp(
TempFileBuilder::new()
.prefix("rustdoctest")
.tempdir()
.expect("rustdoc needs a tempdir"),
)
};
let rustdoc_test_options =
IndividualTestOptions::new(&self.rustdoc_options, &self.arg_file, test_id);

debug!("creating test {name}: {test}");
self.tests.push(test::TestDescAndFn {
Expand Down Expand Up @@ -1096,17 +1170,12 @@ impl Tester for Collector {
&test,
&crate_name,
line,
rustdoc_options,
rustdoc_test_options,
config,
no_run,
runtool,
runtool_args,
target,
&opts,
edition,
outdir,
path,
&test_id,
report_unused_externs,
);

Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ fn usage(argv0: &str) {
/// A result type used by several functions under `main()`.
type MainResult = Result<(), ErrorGuaranteed>;

fn wrap_return(dcx: &rustc_errors::DiagCtxt, res: Result<(), String>) -> MainResult {
pub(crate) fn wrap_return(dcx: &rustc_errors::DiagCtxt, res: Result<(), String>) -> MainResult {
match res {
Ok(()) => dcx.has_errors().map_or(Ok(()), Err),
Err(err) => Err(dcx.err(err)),
Expand Down Expand Up @@ -731,7 +731,7 @@ fn main_args(

match (options.should_test, options.markdown_input()) {
(true, true) => return wrap_return(&diag, markdown::test(options)),
(true, false) => return doctest::run(options),
(true, false) => return doctest::run(&diag, options),
(false, true) => {
let input = options.input.clone();
let edition = options.edition;
Expand Down
Loading
Loading