-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add support for rustdoc mergeable cross-crate info parts #16167
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
weihanglo marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| //! [`BuildRunner`] is the mutable state used during the build process. | ||
|
|
||
| use std::collections::{HashMap, HashSet}; | ||
| use std::ffi::OsString; | ||
| use std::path::{Path, PathBuf}; | ||
| use std::sync::{Arc, Mutex}; | ||
|
|
||
|
|
@@ -302,6 +303,49 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { | |
| .insert(dir.clone().into_path_buf()); | ||
| } | ||
| } | ||
|
|
||
| if self.bcx.build_config.intent.is_doc() | ||
| && self.bcx.gctx.cli_unstable().rustdoc_mergeable_info | ||
| && let Some(unit) = self | ||
| .bcx | ||
| .roots | ||
| .iter() | ||
| .filter(|unit| unit.mode.is_doc()) | ||
| .next() | ||
|
Comment on lines
+309
to
+314
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the finial merge step required to be run always? I was wondering the fingerprint/caching story from here #16291. Perhaps a better model is making this a (Happy to help continue Cargo integration if you need more time on rustdoc side 🙂)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The final merge step has to run if any of the docs have been changed. It can be skipped only if absolutely none of the crates needed documented.
I would love to. Unfortunately, the merge step is not associated with any crate in particular, and
Thank you! |
||
| { | ||
| let mut rustdoc = self.compilation.rustdoc_process(unit, None)?; | ||
| let doc_dir = self.files().out_dir(unit); | ||
| let mut include_arg = OsString::from("--include-parts-dir="); | ||
| include_arg.push(self.files().doc_parts_dir(&unit)); | ||
| rustdoc | ||
| .arg("-o") | ||
| .arg(&doc_dir) | ||
| .arg("--emit=toolchain-shared-resources") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That does seem weird. Probably a cut-and-paste error. |
||
| .arg("-Zunstable-options") | ||
| .arg("--merge=finalize") | ||
| .arg(include_arg); | ||
| exec.exec( | ||
| &rustdoc, | ||
| unit.pkg.package_id(), | ||
| &unit.target, | ||
| CompileMode::Doc, | ||
| // This is always single-threaded, and always gets run, | ||
| // so thread delinterleaving isn't needed and neither is | ||
| // the output cache. | ||
| &mut |line| { | ||
| let mut shell = self.bcx.gctx.shell(); | ||
| shell.print_ansi_stdout(line.as_bytes())?; | ||
| shell.err().write_all(b"\n")?; | ||
| Ok(()) | ||
| }, | ||
| &mut |line| { | ||
| let mut shell = self.bcx.gctx.shell(); | ||
| shell.print_ansi_stderr(line.as_bytes())?; | ||
| shell.err().write_all(b"\n")?; | ||
| Ok(()) | ||
| }, | ||
| )?; | ||
| } | ||
| Ok(self.compilation) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -353,6 +353,10 @@ impl BuildDirLayout { | |
| self.build().join(pkg_dir) | ||
| } | ||
| } | ||
| /// Fetch the doc parts path. | ||
| pub fn doc_parts(&self) -> PathBuf { | ||
| self.build().join("doc.parts") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess we can probably put them under something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the crate-info file is per-unit. The crate-info file name can't really be changed. It is named |
||
| } | ||
| /// Fetch the build script execution path. | ||
| pub fn build_script_execution(&self, pkg_dir: &str) -> PathBuf { | ||
| if self.is_new_layout { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -830,8 +830,13 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu | |
| if build_runner.bcx.gctx.cli_unstable().rustdoc_depinfo { | ||
| // toolchain-shared-resources is required for keeping the shared styling resources | ||
| // invocation-specific is required for keeping the original rustdoc emission | ||
| let mut arg = | ||
| OsString::from("--emit=toolchain-shared-resources,invocation-specific,dep-info="); | ||
| let mut arg = if build_runner.bcx.gctx.cli_unstable().rustdoc_mergeable_info { | ||
| // toolchain resources are written at the end, at the same time as merging | ||
| OsString::from("--emit=invocation-specific,dep-info=") | ||
| } else { | ||
| // if not using mergeable CCI, everything is written every time | ||
| OsString::from("--emit=toolchain-shared-resources,invocation-specific,dep-info=") | ||
|
Comment on lines
+833
to
+838
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another reason we need to stabilize rustdoc's
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I know. I've been working on that, and will try to get it stabilized before trying to get this stabilized. |
||
| }; | ||
| arg.push(rustdoc_dep_info_loc(build_runner, unit)); | ||
| rustdoc.arg(arg); | ||
|
|
||
|
|
@@ -840,6 +845,18 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu | |
| } | ||
|
|
||
| rustdoc.arg("-Zunstable-options"); | ||
| } else if build_runner.bcx.gctx.cli_unstable().rustdoc_mergeable_info { | ||
| // toolchain resources are written at the end, at the same time as merging | ||
| rustdoc.arg("--emit=invocation-specific"); | ||
| rustdoc.arg("-Zunstable-options"); | ||
| } | ||
|
|
||
| if build_runner.bcx.gctx.cli_unstable().rustdoc_mergeable_info { | ||
| // write out mergeable data to be imported | ||
| rustdoc.arg("--merge=none"); | ||
| let mut arg = OsString::from("--parts-out-dir="); | ||
| arg.push(build_runner.files().doc_parts_dir(&unit)); | ||
| rustdoc.arg(arg); | ||
| } | ||
|
|
||
| if let Some(trim_paths) = unit.profile.trim_paths.as_ref() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this draft PR! I didn't know about RFC 3662 until now, so may need more time to catch up.
From my quick skimming through that, it seem prettty aligned with the WIP new build-dir layout in #15010 (one unit per independent directory), as well as the fine-grained locking in #4282.
Not sure if we want to pair them together. It may depend on the timeline of the stabilization of each unstable feature.