Skip to content

Commit

Permalink
Auto merge of #8287 - ehuss:rustdoc-map, r=alexcrichton
Browse files Browse the repository at this point in the history
Add support for rustdoc root URL mappings.

This adds an experimental configuration setting to allow Cargo to pass the `--extern-html-root-url` flag to rustdoc. This flag allows rustdoc to link to other locations when a dependency is not locally documented. See the documentation in `unstable.md` for more details.

There are some known issues with this implementation:

* Rustdoc doesn't seem to know much about renamed dependencies. The links it generates are to the package name, not the renamed name. The code is written to pass in package names, but if there are multiple dependencies to the same package, it won't work properly.

* Similarly, if there are multiple versions of the same package within the dep graph, rustdoc will only link to one of them. To fix this, Cargo would need to pass metadata info into rustdoc (such as the package version).

* If a dependency is built with different features than what is on docs.rs, some links may break.

* This explodes the command-line length significantly. Before stabilizing, we may want to consider addressing that. I'm not sure if it would make sense to change rustdoc's interface, or to use response files?

* This does not pass mappings for transitive dependencies. This normally isn't an issue, but can arise for re-exports (see the `alt_registry` test for an example). I'm not sure if this is a bug in rustdoc or not (there is a large number of issues regarding reexports and rustdoc). Cargo could include these, but this would make the command-line length even longer. Not sure what to do here.

* The config value does not support environment variables. This would be very difficult to support, because Cargo doesn't retain the registry name in `SourceId`. I looked into fixing that, but it is very difficult, and hard to make it reliable.

I have tried to consider future changes in this design, to ensure it doesn't make them more difficult:

* Single-tab browsing. This would be a mode where the std docs are merged with the local crate's docs so that the std docs are shown in the same place (and included in the index). This could be expressed with something like `doc.extern-map.std = "include"` or something like that.  (Or maybe just use build-std?)

* Direct-dependencies only. Often transitive dependencies aren't that interesting, and take up a lot of space in the output, and clog the search index. Some users want the ability to (locally) document their package + direct dependencies only. I think this could be implemented with some kind of command-line flag, perhaps with a config setting in the `[doc]` table. `--extern-html-root-url` flag will automatically handle second-level dependencies.

* Manual-exclusions. Sometimes there are specific dependencies that are very expensive to document locally, but you still want everything else. I think this could be implemented with a command-line flag (`--exclude winapi`?), and the rustdoc-map feature would automatically link those excluded crates' items to docs.rs.  This could also be added to the `[doc]` table.

We can also consider at any time to change the defaults (such as making `crates-io = "https://docs.rs"` the default). It could also potentially auto-detect `std = "local"`, although rustdoc could do the same internally.

Closes #6279
  • Loading branch information
bors committed May 30, 2020
2 parents 6030f57 + f2d32ac commit 00fe8a5
Show file tree
Hide file tree
Showing 10 changed files with 650 additions and 11 deletions.
2 changes: 1 addition & 1 deletion ci/azure-install-rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ steps:
rustup component remove --toolchain=$TOOLCHAIN rust-docs || echo "already removed"
rustup update --no-self-update $TOOLCHAIN
if [[ "$TOOLCHAIN" == "nightly"* ]]; then
rustup component add --toolchain=$TOOLCHAIN rustc-dev llvm-tools-preview
rustup component add --toolchain=$TOOLCHAIN rustc-dev llvm-tools-preview rust-docs
fi
rustup default $TOOLCHAIN
displayName: Install rust
Expand Down
22 changes: 21 additions & 1 deletion src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
//! mtime of sources | ✓[^3] |
//! RUSTFLAGS/RUSTDOCFLAGS | ✓ |
//! LTO flags | ✓ |
//! config settings[^5] | ✓ |
//! is_std | | ✓
//!
//! [^1]: Build script and bin dependencies are not included.
Expand All @@ -82,6 +83,9 @@
//! [^4]: `__CARGO_DEFAULT_LIB_METADATA` is set by rustbuild to embed the
//! release channel (bootstrap/stable/beta/nightly) in libstd.
//!
//! [^5]: Config settings that are not otherwise captured anywhere else.
//! Currently, this is only `doc.extern-map`.
//!
//! When deciding what should go in the Metadata vs the Fingerprint, consider
//! that some files (like dylibs) do not have a hash in their filename. Thus,
//! if a value changes, only the fingerprint will detect the change (consider,
Expand Down Expand Up @@ -533,6 +537,8 @@ pub struct Fingerprint {
/// "description", which are exposed as environment variables during
/// compilation.
metadata: u64,
/// Hash of various config settings that change how things are compiled.
config: u64,
/// Description of whether the filesystem status for this unit is up to date
/// or should be considered stale.
#[serde(skip)]
Expand Down Expand Up @@ -746,6 +752,7 @@ impl Fingerprint {
memoized_hash: Mutex::new(None),
rustflags: Vec::new(),
metadata: 0,
config: 0,
fs_status: FsStatus::Stale,
outputs: Vec::new(),
}
Expand Down Expand Up @@ -806,6 +813,9 @@ impl Fingerprint {
if self.metadata != old.metadata {
bail!("metadata changed")
}
if self.config != old.config {
bail!("configuration settings have changed")
}
let my_local = self.local.lock().unwrap();
let old_local = old.local.lock().unwrap();
if my_local.len() != old_local.len() {
Expand Down Expand Up @@ -1040,12 +1050,13 @@ impl hash::Hash for Fingerprint {
ref deps,
ref local,
metadata,
config,
ref rustflags,
..
} = *self;
let local = local.lock().unwrap();
(
rustc, features, target, path, profile, &*local, metadata, rustflags,
rustc, features, target, path, profile, &*local, metadata, config, rustflags,
)
.hash(h);

Expand Down Expand Up @@ -1252,6 +1263,14 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
// Include metadata since it is exposed as environment variables.
let m = unit.pkg.manifest().metadata();
let metadata = util::hash_u64((&m.authors, &m.description, &m.homepage, &m.repository));
let config = if unit.mode.is_doc() && cx.bcx.config.cli_unstable().rustdoc_map {
cx.bcx
.config
.doc_extern_map()
.map_or(0, |map| util::hash_u64(map))
} else {
0
};
Ok(Fingerprint {
rustc: util::hash_u64(&cx.bcx.rustc().verbose_version),
target: util::hash_u64(&unit.target),
Expand All @@ -1264,6 +1283,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Finger
local: Mutex::new(local),
memoized_hash: Mutex::new(None),
metadata,
config,
rustflags: extra_flags,
fs_status: FsStatus::Stale,
outputs,
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ mod layout;
mod links;
mod lto;
mod output_depinfo;
pub mod rustdoc;
pub mod standard_lib;
mod timings;
mod unit;
Expand Down Expand Up @@ -570,6 +571,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
}

build_deps_args(&mut rustdoc, cx, unit)?;
rustdoc::add_root_urls(cx, unit, &mut rustdoc)?;

rustdoc.args(bcx.rustdocflags_args(unit));

Expand Down
172 changes: 172 additions & 0 deletions src/cargo/core/compiler/rustdoc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
//! Utilities for building with rustdoc.
use crate::core::compiler::context::Context;
use crate::core::compiler::unit::Unit;
use crate::core::compiler::CompileKind;
use crate::sources::CRATES_IO_REGISTRY;
use crate::util::errors::{internal, CargoResult};
use crate::util::ProcessBuilder;
use std::collections::HashMap;
use std::fmt;
use std::hash;
use url::Url;

/// Mode used for `std`.
#[derive(Debug, Hash)]
pub enum RustdocExternMode {
/// Use a local `file://` URL.
Local,
/// Use a remote URL to https://doc.rust-lang.org/ (default).
Remote,
/// An arbitrary URL.
Url(String),
}

impl From<String> for RustdocExternMode {
fn from(s: String) -> RustdocExternMode {
match s.as_ref() {
"local" => RustdocExternMode::Local,
"remote" => RustdocExternMode::Remote,
_ => RustdocExternMode::Url(s),
}
}
}

impl fmt::Display for RustdocExternMode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
RustdocExternMode::Local => "local".fmt(f),
RustdocExternMode::Remote => "remote".fmt(f),
RustdocExternMode::Url(s) => s.fmt(f),
}
}
}

impl<'de> serde::de::Deserialize<'de> for RustdocExternMode {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::de::Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
Ok(s.into())
}
}

#[derive(serde::Deserialize, Debug)]
pub struct RustdocExternMap {
registries: HashMap<String, String>,
std: Option<RustdocExternMode>,
}

impl hash::Hash for RustdocExternMap {
fn hash<H: hash::Hasher>(&self, into: &mut H) {
self.std.hash(into);
for (key, value) in &self.registries {
key.hash(into);
value.hash(into);
}
}
}

pub fn add_root_urls(
cx: &Context<'_, '_>,
unit: &Unit,
rustdoc: &mut ProcessBuilder,
) -> CargoResult<()> {
let config = cx.bcx.config;
if !config.cli_unstable().rustdoc_map {
log::debug!("`doc.extern-map` ignored, requires -Zrustdoc-map flag");
return Ok(());
}
let map = config.doc_extern_map()?;
if map.registries.len() == 0 && map.std.is_none() {
// Skip doing unnecessary work.
return Ok(());
}
let mut unstable_opts = false;
// Collect mapping of registry name -> index url.
let name2url: HashMap<&String, Url> = map
.registries
.keys()
.filter_map(|name| {
if let Ok(index_url) = config.get_registry_index(name) {
return Some((name, index_url));
} else {
log::warn!(
"`doc.extern-map.{}` specifies a registry that is not defined",
name
);
return None;
}
})
.collect();
for dep in cx.unit_deps(unit) {
if dep.unit.target.is_linkable() && !dep.unit.mode.is_doc() {
for (registry, location) in &map.registries {
let sid = dep.unit.pkg.package_id().source_id();
let matches_registry = || -> bool {
if !sid.is_registry() {
return false;
}
if sid.is_default_registry() {
return registry == CRATES_IO_REGISTRY;
}
if let Some(index_url) = name2url.get(registry) {
return index_url == sid.url();
}
false
};
if matches_registry() {
let mut url = location.clone();
if !url.contains("{pkg_name}") && !url.contains("{version}") {
if !url.ends_with('/') {
url.push('/');
}
url.push_str("{pkg_name}/{version}/");
}
let url = url
.replace("{pkg_name}", &dep.unit.pkg.name())
.replace("{version}", &dep.unit.pkg.version().to_string());
rustdoc.arg("--extern-html-root-url");
rustdoc.arg(format!("{}={}", dep.unit.target.crate_name(), url));
unstable_opts = true;
}
}
}
}
let std_url = match &map.std {
None | Some(RustdocExternMode::Remote) => None,
Some(RustdocExternMode::Local) => {
let sysroot = &cx.bcx.target_data.info(CompileKind::Host).sysroot;
let html_root = sysroot.join("share").join("doc").join("rust").join("html");
if html_root.exists() {
let url = Url::from_file_path(&html_root).map_err(|()| {
internal(format!(
"`{}` failed to convert to URL",
html_root.display()
))
})?;
Some(url.to_string())
} else {
log::warn!(
"`doc.extern-map.std` is \"local\", but local docs don't appear to exist at {}",
html_root.display()
);
None
}
}
Some(RustdocExternMode::Url(s)) => Some(s.to_string()),
};
if let Some(url) = std_url {
for name in &["std", "core", "alloc", "proc_macro"] {
rustdoc.arg("--extern-html-root-url");
rustdoc.arg(format!("{}={}", name, url));
unstable_opts = true;
}
}

if unstable_opts {
rustdoc.arg("-Zunstable-options");
}
Ok(())
}
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ pub struct CliUnstable {
pub crate_versions: bool,
pub separate_nightlies: bool,
pub multitarget: bool,
pub rustdoc_map: bool,
}

impl CliUnstable {
Expand Down Expand Up @@ -435,6 +436,7 @@ impl CliUnstable {
"crate-versions" => self.crate_versions = parse_empty(k, v)?,
"separate-nightlies" => self.separate_nightlies = parse_empty(k, v)?,
"multitarget" => self.multitarget = parse_empty(k, v)?,
"rustdoc-map" => self.rustdoc_map = parse_empty(k, v)?,
_ => bail!("unknown `-Z` flag specified: {}", k),
}

Expand Down
32 changes: 24 additions & 8 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ use serde::Deserialize;
use url::Url;

use self::ConfigValue as CV;
use crate::core::compiler::rustdoc::RustdocExternMap;
use crate::core::shell::Verbosity;
use crate::core::{nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace};
use crate::ops;
Expand Down Expand Up @@ -172,6 +173,7 @@ pub struct Config {
net_config: LazyCell<CargoNetConfig>,
build_config: LazyCell<CargoBuildConfig>,
target_cfgs: LazyCell<Vec<(String, TargetCfgConfig)>>,
doc_extern_map: LazyCell<RustdocExternMap>,
}

impl Config {
Expand Down Expand Up @@ -241,6 +243,7 @@ impl Config {
net_config: LazyCell::new(),
build_config: LazyCell::new(),
target_cfgs: LazyCell::new(),
doc_extern_map: LazyCell::new(),
}
}

Expand Down Expand Up @@ -1008,12 +1011,16 @@ impl Config {
/// Gets the index for a registry.
pub fn get_registry_index(&self, registry: &str) -> CargoResult<Url> {
validate_package_name(registry, "registry name", "")?;
Ok(
match self.get_string(&format!("registries.{}.index", registry))? {
Some(index) => self.resolve_registry_index(index)?,
None => bail!("No index found for registry: `{}`", registry),
},
)
if let Some(index) = self.get_string(&format!("registries.{}.index", registry))? {
self.resolve_registry_index(&index).chain_err(|| {
format!(
"invalid index URL for registry `{}` defined in {}",
registry, index.definition
)
})
} else {
bail!("no index found for registry: `{}`", registry);
}
}

/// Returns an error if `registry.index` is set.
Expand All @@ -1027,7 +1034,8 @@ impl Config {
Ok(())
}

fn resolve_registry_index(&self, index: Value<String>) -> CargoResult<Url> {
fn resolve_registry_index(&self, index: &Value<String>) -> CargoResult<Url> {
// This handles relative file: URLs, relative to the config definition.
let base = index
.definition
.root(self)
Expand All @@ -1036,7 +1044,7 @@ impl Config {
let _parsed = index.val.into_url()?;
let url = index.val.into_url_with_base(Some(&*base))?;
if url.password().is_some() {
bail!("Registry URLs may not contain passwords");
bail!("registry URLs may not contain passwords");
}
Ok(url)
}
Expand Down Expand Up @@ -1154,6 +1162,14 @@ impl Config {
.try_borrow_with(|| target::load_target_cfgs(self))
}

pub fn doc_extern_map(&self) -> CargoResult<&RustdocExternMap> {
// Note: This does not support environment variables. The `Unit`
// fundamentally does not have access to the registry name, so there is
// nothing to query. Plumbing the name into SourceId is quite challenging.
self.doc_extern_map
.try_borrow_with(|| self.get::<RustdocExternMap>("doc.extern-map"))
}

/// Returns the `[target]` table definition for the given target triple.
pub fn target_cfg_triple(&self, target: &str) -> CargoResult<TargetConfig> {
target::load_target_triple(self, target)
Expand Down
Loading

0 comments on commit 00fe8a5

Please sign in to comment.