Skip to content

Commit

Permalink
[wip] Update boostrap tests to support book dependencies
Browse files Browse the repository at this point in the history
Since TRPL now depends on a `trpl` crate, the test needs to be able to
build that crate to run mdbook against it, and also to invoke mdbook
with `--library-path` in that case. Use the support for that flag added
to `rustbook` in the previous change to

Still to-do:

- [ ] Build `trpl` with the `run_cargo` command. Use its result to pass
      the correct locations to `--library-path`.
- [ ] Test this to make sure it works (!) and that it does not generate
      any extra noise during build.
  • Loading branch information
chriskrycho committed Oct 25, 2024
1 parent f927307 commit 206b688
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 9 deletions.
57 changes: 56 additions & 1 deletion src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{env, fs, iter};

use clap_complete::shells;

use crate::core::build_steps::compile::run_cargo;
use crate::core::build_steps::doc::DocumentationFormat;
use crate::core::build_steps::synthetic_targets::MirOptPanicAbortSyntheticTarget;
use crate::core::build_steps::tool::{self, SourceType, Tool};
Expand Down Expand Up @@ -2168,9 +2169,11 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct BookTest {
compiler: Compiler,
target: TargetSelection,
path: PathBuf,
name: &'static str,
is_ext_doc: bool,
dependencies: Vec<&'static str>,
}

impl Step for BookTest {
Expand Down Expand Up @@ -2223,6 +2226,44 @@ impl BookTest {
// Books often have feature-gated example text.
rustbook_cmd.env("RUSTC_BOOTSTRAP", "1");
rustbook_cmd.env("PATH", new_path).arg("test").arg(path);

// Books may also need to build dependencies. For example, `TheBook` has
// code samples which use the `trpl` crate. For the `rustdoc` invocation
// to find them them successfully, they need to be built first and their
// paths used to generate the
let libs = if !self.dependencies.is_empty() {
let mut lib_paths = vec![];
for dep in self.dependencies {
let mode = Mode::ToolBootstrap;
let target = builder.config.build;
// CHECKME: is this correct, or should it be using `builder::Cargo::new`?
let cargo = tool::prepare_tool_cargo(
&builder,
self.compiler,
mode,
builder.config.build,
Kind::Build,
dep,
SourceType::Submodule,
&[],
);
// CHECKME: this is used for the "stamp" for this `run_cargo`; is this reasonable?
let out_dir = builder.cargo_out(self.compiler, mode, target);
let output_paths =
run_cargo(builder, cargo, vec![], &out_dir, vec![], false, false);
lib_paths.extend(output_paths);
}
lib_paths
} else {
vec![]
};

if !libs.is_empty() {
let mut cli_args = vec![String::from("--library-path")];
cli_args.extend(libs.into_iter().map(|path| format!("{}", path.display())));
rustbook_cmd.args(dbg!(cli_args));
}

builder.add_rust_test_threads(&mut rustbook_cmd);
let _guard = builder.msg(
Kind::Test,
Expand Down Expand Up @@ -2281,12 +2322,14 @@ macro_rules! test_book {
$name:ident, $path:expr, $book_name:expr,
default=$default:expr
$(,submodules = $submodules:expr)?
$(,dependencies=$dependencies:expr)?
;
)+) => {
$(
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct $name {
compiler: Compiler,
target: TargetSelection,
}

impl Step for $name {
Expand All @@ -2301,6 +2344,7 @@ macro_rules! test_book {
fn make_run(run: RunConfig<'_>) {
run.builder.ensure($name {
compiler: run.builder.compiler(run.builder.top_stage, run.target),
target: run.target,
});
}

Expand All @@ -2310,11 +2354,22 @@ macro_rules! test_book {
builder.require_submodule(submodule, None);
}
)*

let dependencies = vec![];
$(
let mut dependencies = dependencies;
for dep in $dependencies {
dependencies.push(dep);
}
)?

builder.ensure(BookTest {
compiler: self.compiler,
target: self.target,
path: PathBuf::from($path),
name: $book_name,
is_ext_doc: !$default,
dependencies,
});
}
}
Expand All @@ -2329,7 +2384,7 @@ test_book!(
RustcBook, "src/doc/rustc", "rustc", default=true;
RustByExample, "src/doc/rust-by-example", "rust-by-example", default=false, submodules=["src/doc/rust-by-example"];
EmbeddedBook, "src/doc/embedded-book", "embedded-book", default=false, submodules=["src/doc/embedded-book"];
TheBook, "src/doc/book", "book", default=false, submodules=["src/doc/book"];
TheBook, "src/doc/book", "book", default=false, submodules=["src/doc/book"], dependencies=["src/doc/book/packages/trpl"];
UnstableBook, "src/doc/unstable-book", "unstable-book", default=true;
EditionGuide, "src/doc/edition-guide", "edition-guide", default=false, submodules=["src/doc/edition-guide"];
);
Expand Down
4 changes: 3 additions & 1 deletion src/tools/rustbook/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ env_logger = "0.11"
mdbook-trpl-listing = { path = "../../doc/book/packages/mdbook-trpl-listing" }
mdbook-trpl-note = { path = "../../doc/book/packages/mdbook-trpl-note" }
mdbook-i18n-helpers = "0.3.3"
mdbook-spec = { path = "../../doc/reference/mdbook-spec"}
mdbook-spec = { path = "../../doc/reference/mdbook-spec" }
# Note: `trpl` is unused, but this ensures it and its dependencies get vendored.
trpl = { path = "../../doc/book/packages/trpl" }

[dependencies.mdbook]
version = "0.4.37"
Expand Down
10 changes: 3 additions & 7 deletions src/tools/rustbook/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use std::env;
use std::path::{Path, PathBuf};

use clap::{arg, crate_version, ArgMatches, Command};
use mdbook::errors::Result as Result3;
use clap::{ArgMatches, Command, arg, crate_version};
use mdbook::MDBook;
use mdbook::errors::Result as Result3;
use mdbook_i18n_helpers::preprocessors::Gettext;
use mdbook_spec::Spec;
use mdbook_trpl_listing::TrplListing;
Expand Down Expand Up @@ -132,11 +132,7 @@ fn test(args: &ArgMatches) -> Result3<()> {
fn get_book_dir(args: &ArgMatches) -> PathBuf {
if let Some(p) = args.get_one::<PathBuf>("dir") {
// Check if path is relative from current dir, or absolute...
if p.is_relative() {
env::current_dir().unwrap().join(p)
} else {
p.to_path_buf()
}
if p.is_relative() { env::current_dir().unwrap().join(p) } else { p.to_path_buf() }
} else {
env::current_dir().unwrap()
}
Expand Down

0 comments on commit 206b688

Please sign in to comment.