From 095e673a332c8f1a152c308ab3fc93931e7df52f Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Fri, 3 May 2019 10:14:21 -0500 Subject: [PATCH 01/21] Add extra-targets to package metadata --- src/docbuilder/metadata.rs | 16 ++++++++++++++++ templates/about.hbs | 3 +++ 2 files changed, 19 insertions(+) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 97f0ea963..14128a129 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -20,6 +20,7 @@ use failure::err_msg; /// all-features = true /// no-default-features = true /// default-target = "x86_64-unknown-linux-gnu" +/// extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] /// rustc-args = [ "--example-rustc-arg" ] /// rustdoc-args = [ "--example-rustdoc-arg" ] /// ``` @@ -43,6 +44,11 @@ pub struct Metadata { /// is always built on this target. You can change default target by setting this. pub default_target: Option, + /// Docs.rs doesn't automatically build extra targets for crates. If you want a crate to build + /// for multiple targets, set `extra-targets` to the list of targets to build, in addition to + /// `default-target`. + pub extra_targets: Vec, + /// List of command line arguments for `rustc`. pub rustc_args: Option>, @@ -87,6 +93,7 @@ impl Metadata { default_target: None, rustc_args: None, rustdoc_args: None, + extra_targets: Vec::new(), } } @@ -111,6 +118,9 @@ impl Metadata { .and_then(|v| v.as_bool()).unwrap_or(metadata.all_features); metadata.default_target = table.get("default-target") .and_then(|v| v.as_str()).map(|v| v.to_owned()); + metadata.extra_targets = table.get("extra-targets").and_then(|f| f.as_array()) + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()) + .unwrap_or_default(); metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); metadata.rustdoc_args = table.get("rustdoc-args").and_then(|f| f.as_array()) @@ -140,6 +150,7 @@ mod test { all-features = true no-default-features = true default-target = "x86_64-unknown-linux-gnu" + extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] rustc-args = [ "--example-rustc-arg" ] rustdoc-args = [ "--example-rustdoc-arg" ] "#; @@ -159,6 +170,11 @@ mod test { assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); + let extra_targets = metadata.extra_targets; + assert_eq!(extra_targets.len(), 2); + assert_eq!(extra_targets[0], "x86_64-apple-darwin"); + assert_eq!(extra_targets[1], "x86_64-pc-windows-msvc"); + let rustc_args = metadata.rustc_args.unwrap(); assert_eq!(rustc_args.len(), 1); assert_eq!(rustc_args[0], "--example-rustc-arg".to_owned()); diff --git a/templates/about.hbs b/templates/about.hbs index 2002a52d4..621202152 100644 --- a/templates/about.hbs +++ b/templates/about.hbs @@ -162,6 +162,9 @@ no-default-features = true # - i686-pc-windows-msvc default-target = "x86_64-unknown-linux-gnu" +# Extra targets to build, same available targets as `default-target` (default: none) +extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] + # Additional `RUSTFLAGS` to set (default: none) rustc-args = [ "--example-rustc-arg" ] From 0e9246c6ebcbfb76b49ec6adc2cc6e995c569b78 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 21 Jan 2020 14:14:14 -0500 Subject: [PATCH 02/21] give build_package access to metadata as a side effect, only calculate metadata once instead of for each target --- src/docbuilder/rustwide_builder.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 64ab688b2..d4eb13955 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -189,7 +189,8 @@ impl RustwideBuilder { build_dir .build(&self.toolchain, &krate, sandbox) .run(|build| { - let res = self.execute_build(None, build, &limits)?; + let metadata = Metadata::from_source_dir(&build.host_source_dir())?; + let res = self.execute_build(None, build, &limits, &metadata)?; if !res.result.successful { bail!("failed to build dummy crate for {}", self.rustc_version); } @@ -312,9 +313,10 @@ impl RustwideBuilder { let mut files_list = None; let mut has_docs = false; let mut successful_targets = Vec::new(); + let metadata = Metadata::from_source_dir(&build.host_source_dir())?; // Do an initial build and then copy the sources in the database - let res = self.execute_build(None, &build, &limits)?; + let res = self.execute_build(None, &build, &limits, &metadata)?; if res.result.successful { debug!("adding sources into database"); let prefix = format!("sources/{}/{}", name, version); @@ -352,6 +354,7 @@ impl RustwideBuilder { &limits, &local_storage.path(), &mut successful_targets, + &metadata, )?; } self.upload_docs(&conn, name, version, local_storage.path())?; @@ -396,8 +399,9 @@ impl RustwideBuilder { limits: &Limits, local_storage: &Path, successful_targets: &mut Vec, + metadata: &Metadata, ) -> Result<()> { - let target_res = self.execute_build(Some(target), build, limits)?; + let target_res = self.execute_build(Some(target), build, limits, metadata)?; if target_res.result.successful { // Cargo is not giving any error and not generating documentation of some crates // when we use a target compile options. Check documentation exists before @@ -416,8 +420,8 @@ impl RustwideBuilder { target: Option<&str>, build: &Build, limits: &Limits, + metadata: &Metadata, ) -> Result { - let metadata = Metadata::from_source_dir(&build.host_source_dir())?; let cargo_metadata = CargoMetadata::load(&self.workspace, &self.toolchain, &build.host_source_dir())?; From edba858ff3f16c7321caf347b02b325730cc1ec2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 21 Jan 2020 14:09:30 -0500 Subject: [PATCH 03/21] If env-var is set, don't build all targets --- src/docbuilder/rustwide_builder.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index d4eb13955..94a19ced1 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -342,8 +342,18 @@ impl RustwideBuilder { )?; successful_targets.push(res.target.clone()); + + // this is a breaking change, don't enable it by default + let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") + .map(|s| s == "true").unwrap_or(false); + let strs: Vec<_>; + let targets: &[&str] = if build_specific { + strs = metadata.extra_targets.iter().map(|s| s.as_str()).collect(); + &strs + } else { TARGETS }; + // Then build the documentation for all the targets - for target in TARGETS { + for target in targets { if *target == res.target { continue; } From da02d0e53e69ac523ce1b028853606183a2ddf58 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 02:15:56 -0500 Subject: [PATCH 04/21] Add opt-in way to only build for specific targets --- src/docbuilder/metadata.rs | 15 ++++++++------- src/docbuilder/rustwide_builder.rs | 16 ++++++++++++++-- templates/about.hbs | 5 ++++- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 14128a129..cbcc94e3d 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -44,10 +44,12 @@ pub struct Metadata { /// is always built on this target. You can change default target by setting this. pub default_target: Option, - /// Docs.rs doesn't automatically build extra targets for crates. If you want a crate to build - /// for multiple targets, set `extra-targets` to the list of targets to build, in addition to - /// `default-target`. - pub extra_targets: Vec, + /// If you want a crate to build only for specific targets, + /// set `extra-targets` to the list of targets to build, in addition to `default-target`. + /// + /// If you do not set `extra_targets`, all of the tier 1 supported targets will be built. + /// If you set `extra_targets` to an empty array, only the default target will be built. + pub extra_targets: Option>, /// List of command line arguments for `rustc`. pub rustc_args: Option>, @@ -93,7 +95,7 @@ impl Metadata { default_target: None, rustc_args: None, rustdoc_args: None, - extra_targets: Vec::new(), + extra_targets: None, } } @@ -119,8 +121,7 @@ impl Metadata { metadata.default_target = table.get("default-target") .and_then(|v| v.as_str()).map(|v| v.to_owned()); metadata.extra_targets = table.get("extra-targets").and_then(|f| f.as_array()) - .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()) - .unwrap_or_default(); + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); metadata.rustdoc_args = table.get("rustdoc-args").and_then(|f| f.as_array()) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 94a19ced1..0e2f698d3 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -347,10 +347,22 @@ impl RustwideBuilder { let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") .map(|s| s == "true").unwrap_or(false); let strs: Vec<_>; + // If the env variable is set, _only_ build the specified targets + // If no targets are specified, only build the default target. let targets: &[&str] = if build_specific { - strs = metadata.extra_targets.iter().map(|s| s.as_str()).collect(); + strs = metadata.extra_targets + .as_ref() + .map(|v| v.iter().map(|s| s.as_str()).collect()) + .unwrap_or_default(); &strs - } else { TARGETS }; + // Otherwise, let people opt-in to only having specific targets + } else if let Some(explicit_targets) = &metadata.extra_targets { + strs = explicit_targets.iter().map(|s| s.as_str()).collect(); + &strs + // Otherwise, keep the existing behavior + } else { + TARGETS + }; // Then build the documentation for all the targets for target in targets { diff --git a/templates/about.hbs b/templates/about.hbs index 621202152..c6a762539 100644 --- a/templates/about.hbs +++ b/templates/about.hbs @@ -162,7 +162,10 @@ no-default-features = true # - i686-pc-windows-msvc default-target = "x86_64-unknown-linux-gnu" -# Extra targets to build, same available targets as `default-target` (default: none) +# Targets to build in addition to `default-target` (default: all tier 1 targets) +# Same available targets as `default-target`. +# If not specified, builds all tier 1 targets. +# Set this to `[]` to only build the default target. extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] # Additional `RUSTFLAGS` to set (default: none) From b09eb7aba251822a486bed4e01efbe0f3a6b9161 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 02:18:51 -0500 Subject: [PATCH 05/21] clean up code style --- src/docbuilder/rustwide_builder.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 0e2f698d3..de7a865c5 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -365,13 +365,13 @@ impl RustwideBuilder { }; // Then build the documentation for all the targets - for target in targets { - if *target == res.target { + for &target in targets { + if target == res.target { continue; } - debug!("building package {} {} for {}", name, version, &target); + debug!("building package {} {} for {}", name, version, target); self.build_target( - &target, + target, &build, &limits, &local_storage.path(), From 6bc3f923b260864195c03a903c702c29a7317734 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 02:21:37 -0500 Subject: [PATCH 06/21] Remove duplicate docs --- templates/about.hbs | 1 - 1 file changed, 1 deletion(-) diff --git a/templates/about.hbs b/templates/about.hbs index c6a762539..07ccb4adf 100644 --- a/templates/about.hbs +++ b/templates/about.hbs @@ -164,7 +164,6 @@ default-target = "x86_64-unknown-linux-gnu" # Targets to build in addition to `default-target` (default: all tier 1 targets) # Same available targets as `default-target`. -# If not specified, builds all tier 1 targets. # Set this to `[]` to only build the default target. extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] From 82d0f8448a871038cdbb1ed8033bf8658f0e4b70 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 03:08:10 -0500 Subject: [PATCH 07/21] Fix tests (and new test) --- src/docbuilder/metadata.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index cbcc94e3d..6047749bb 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -171,7 +171,7 @@ mod test { assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); - let extra_targets = metadata.extra_targets; + let extra_targets = metadata.extra_targets.expect("should have explicit extra target"); assert_eq!(extra_targets.len(), 2); assert_eq!(extra_targets[0], "x86_64-apple-darwin"); assert_eq!(extra_targets[1], "x86_64-pc-windows-msvc"); @@ -184,4 +184,32 @@ mod test { assert_eq!(rustdoc_args.len(), 1); assert_eq!(rustdoc_args[0], "--example-rustdoc-arg".to_owned()); } + + #[test] + fn test_no_extra_targets() { + // metadata section but no extra_targets + let manifest = r#" + [package] + name = "test" + + [package.metadata.docs.rs] + features = [ "feature1", "feature2" ] + "#; + let metadata = Metadata::from_str(manifest); + assert!(metadata.extra_targets.is_none()); + + // no package.metadata.docs.rs section + let metadata = Metadata::from_str(r#" + [package] + name = "test" + "#); + assert!(metadata.extra_targets.is_none()); + + // extra targets explicitly set to empty array + let metadata = Metadata::from_str(r#" + [package.metadata.docs.rs] + extra-targets = [] + "#); + assert!(metadata.extra_targets.unwrap().is_empty()); + } } From 263a05f9105d37648303cee14d6f54ff03737bfd Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 08:02:01 -0500 Subject: [PATCH 08/21] Separate target selection into a separate function --- src/docbuilder/rustwide_builder.rs | 49 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index de7a865c5..1758aebbd 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -342,33 +342,10 @@ impl RustwideBuilder { )?; successful_targets.push(res.target.clone()); - - // this is a breaking change, don't enable it by default - let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") - .map(|s| s == "true").unwrap_or(false); - let strs: Vec<_>; - // If the env variable is set, _only_ build the specified targets - // If no targets are specified, only build the default target. - let targets: &[&str] = if build_specific { - strs = metadata.extra_targets - .as_ref() - .map(|v| v.iter().map(|s| s.as_str()).collect()) - .unwrap_or_default(); - &strs - // Otherwise, let people opt-in to only having specific targets - } else if let Some(explicit_targets) = &metadata.extra_targets { - strs = explicit_targets.iter().map(|s| s.as_str()).collect(); - &strs - // Otherwise, keep the existing behavior - } else { - TARGETS - }; + let targets = select_extra_targets(&res.target, &metadata); // Then build the documentation for all the targets - for &target in targets { - if target == res.target { - continue; - } + for target in targets { debug!("building package {} {} for {}", name, version, target); self.build_target( target, @@ -574,6 +551,28 @@ impl RustwideBuilder { } } +fn select_extra_targets<'a>(default_target: &str, metadata: &'a Metadata) -> HashSet<&'a str> { + // this is a breaking change, don't enable it by default + let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") + .map(|s| s == "true").unwrap_or(false); + // If the env variable is set, _only_ build the specified targets + // If no targets are specified, only build the default target. + let mut extra_targets: HashSet<_> = if build_specific { + metadata.extra_targets + .as_ref() + .map(|v| v.iter().map(|s| s.as_str()).collect()) + .unwrap_or_default() + // Otherwise, let people opt-in to only having specific targets + } else if let Some(explicit_targets) = &metadata.extra_targets { + explicit_targets.iter().map(|s| s.as_str()).collect() + // Otherwise, keep the existing behavior + } else { + TARGETS.iter().copied().collect() + }; + extra_targets.remove(default_target); + extra_targets +} + struct FullBuildResult { result: BuildResult, target: String, From bc88ac63a56db4a0b092484eaa24f5e8fb930fed Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 08:09:57 -0500 Subject: [PATCH 09/21] select_extra_targets -> Metadata::select_extra_targets --- src/docbuilder/metadata.rs | 24 +++++++++++++++++++++++- src/docbuilder/rustwide_builder.rs | 26 ++------------------------ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 6047749bb..5a25c39f0 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -1,4 +1,4 @@ - +use std::collections::HashSet; use std::path::Path; use toml::Value; use error::Result; @@ -130,6 +130,28 @@ impl Metadata { metadata } + pub(super) fn select_extra_targets<'a>(&'a self, default_target: &str) -> HashSet<&'a str> { + use super::rustwide_builder::TARGETS; + // this is a breaking change, don't enable it by default + let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") + .map(|s| s == "true").unwrap_or(false); + // If the env variable is set, _only_ build the specified targets + // If no targets are specified, only build the default target. + let mut extra_targets: HashSet<_> = if build_specific { + self.extra_targets + .as_ref() + .map(|v| v.iter().map(|s| s.as_str()).collect()) + .unwrap_or_default() + // Otherwise, let people opt-in to only having specific targets + } else if let Some(explicit_targets) = &self.extra_targets { + explicit_targets.iter().map(|s| s.as_str()).collect() + // Otherwise, keep the existing behavior + } else { + TARGETS.iter().copied().collect() + }; + extra_targets.remove(default_target); + extra_targets + } } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 1758aebbd..f2cf2e4cc 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -22,7 +22,7 @@ const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs) const DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide"; const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu"; -const TARGETS: &[&str] = &[ +pub(super) const TARGETS: &[&str] = &[ "i686-pc-windows-msvc", "i686-unknown-linux-gnu", "x86_64-apple-darwin", @@ -342,7 +342,7 @@ impl RustwideBuilder { )?; successful_targets.push(res.target.clone()); - let targets = select_extra_targets(&res.target, &metadata); + let targets = metadata.select_extra_targets(&res.target); // Then build the documentation for all the targets for target in targets { @@ -551,28 +551,6 @@ impl RustwideBuilder { } } -fn select_extra_targets<'a>(default_target: &str, metadata: &'a Metadata) -> HashSet<&'a str> { - // this is a breaking change, don't enable it by default - let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") - .map(|s| s == "true").unwrap_or(false); - // If the env variable is set, _only_ build the specified targets - // If no targets are specified, only build the default target. - let mut extra_targets: HashSet<_> = if build_specific { - metadata.extra_targets - .as_ref() - .map(|v| v.iter().map(|s| s.as_str()).collect()) - .unwrap_or_default() - // Otherwise, let people opt-in to only having specific targets - } else if let Some(explicit_targets) = &metadata.extra_targets { - explicit_targets.iter().map(|s| s.as_str()).collect() - // Otherwise, keep the existing behavior - } else { - TARGETS.iter().copied().collect() - }; - extra_targets.remove(default_target); - extra_targets -} - struct FullBuildResult { result: BuildResult, target: String, From 9eae8968f621f6d506ca498c0c1d6c88f3f771e7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Mar 2020 08:25:39 -0500 Subject: [PATCH 10/21] Add test for select_extra_targets --- src/docbuilder/metadata.rs | 47 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 5a25c39f0..c5fc06291 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -234,4 +234,51 @@ mod test { "#); assert!(metadata.extra_targets.unwrap().is_empty()); } + #[test] + fn test_select_extra_targets() { + use crate::docbuilder::rustwide_builder::TARGETS; + + let mut metadata = Metadata { + extra_targets: None, + ..Metadata::default() + }; + const DEFAULT_TARGET: &str = "x86-unknown-linux-gnu"; + + // unchanged default_target, extra targets not specified + let tier_one = metadata.select_extra_targets(DEFAULT_TARGET); + // should be equal to TARGETS except for DEFAULT_TARGET + for actual in &tier_one { + assert!(TARGETS.contains(actual)); + } + for expected in TARGETS { + if *expected == DEFAULT_TARGET { + assert!(!tier_one.contains(DEFAULT_TARGET)); + } else { + assert!(tier_one.contains(expected)); + } + } + + // unchanged default_target, extra targets specified to be empty + metadata.extra_targets = Some(Vec::new()); + assert!(metadata.select_extra_targets(DEFAULT_TARGET).is_empty()); + + // unchanged default_target, extra targets includes targets besides default_target + metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); + let extras = metadata.select_extra_targets(DEFAULT_TARGET); + assert_eq!(extras.len(), 2); + assert!(extras.contains("i686-pc-windows-msvc")); + assert!(extras.contains("i686-apple-darwin")); + + // make sure that default_target is not built twice + metadata.extra_targets = Some(vec![DEFAULT_TARGET.into()]); + assert!(metadata.select_extra_targets(DEFAULT_TARGET).is_empty()); + + // make sure that duplicates are removed + metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); + assert_eq!(metadata.select_extra_targets(DEFAULT_TARGET).len(), 1); + + // try it with a different default target just for sanity + assert!(metadata.select_extra_targets("i686-pc-windows-msvc").is_empty()); + + } } From 8a3ed3ca88dc9fe4877ab8d9fcac99aff220f40b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 21:49:39 -0400 Subject: [PATCH 11/21] Use first target in `extra-targets` if `default-target` is unset This is almost entirely untested. --- src/docbuilder/metadata.rs | 47 +++++++++++++++++------------- src/docbuilder/rustwide_builder.rs | 41 ++++++++++++-------------- 2 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index c5fc06291..f8efa762d 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -1,4 +1,3 @@ -use std::collections::HashSet; use std::path::Path; use toml::Value; use error::Result; @@ -130,27 +129,34 @@ impl Metadata { metadata } - pub(super) fn select_extra_targets<'a>(&'a self, default_target: &str) -> HashSet<&'a str> { - use super::rustwide_builder::TARGETS; - // this is a breaking change, don't enable it by default - let build_specific = std::env::var("DOCS_RS_BUILD_ONLY_SPECIFIED_TARGETS") - .map(|s| s == "true").unwrap_or(false); - // If the env variable is set, _only_ build the specified targets - // If no targets are specified, only build the default target. - let mut extra_targets: HashSet<_> = if build_specific { - self.extra_targets - .as_ref() - .map(|v| v.iter().map(|s| s.as_str()).collect()) - .unwrap_or_default() - // Otherwise, let people opt-in to only having specific targets - } else if let Some(explicit_targets) = &self.extra_targets { - explicit_targets.iter().map(|s| s.as_str()).collect() - // Otherwise, keep the existing behavior + pub(super) fn select_extra_targets<'a: 'ret, 'b: 'ret, 'ret>(&'a self) -> (&'ret str, Vec<&'ret str>) { + use super::rustwide_builder::{DEFAULT_TARGET, TARGETS}; + // Let people opt-in to only having specific targets + // Ideally this would use Iterator instead of Vec so I could collect to a `HashSet`, + // but I had trouble with `chain()` ¯\_(ツ)_/¯ + let mut all_targets: Vec<_> = self.default_target.as_deref().into_iter().collect(); + match &self.extra_targets { + Some(targets) => all_targets.extend(targets.iter().map(|s| s.as_str())), + None => all_targets.extend(TARGETS.iter().copied()), + }; + + // default_target unset and extra_targets set to `[]` + let landing_page = if all_targets.is_empty() { + DEFAULT_TARGET } else { - TARGETS.iter().copied().collect() + // This `swap_remove` has to come before the `sort()` to keep the ordering + // `swap_remove` is ok because ordering doesn't matter except for first element + all_targets.swap_remove(0) }; - extra_targets.remove(default_target); - extra_targets + // Remove duplicates + all_targets.sort(); + all_targets.dedup(); + // Remove landing_page so we don't build it twice. + // It wasn't removed during dedup because we called `swap_remove()` first. + if let Ok(index) = all_targets.binary_search(&landing_page) { + all_targets.swap_remove(index); + } + (landing_page, all_targets) } } @@ -279,6 +285,5 @@ mod test { // try it with a different default target just for sanity assert!(metadata.select_extra_targets("i686-pc-windows-msvc").is_empty()); - } } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index f2cf2e4cc..4944a9657 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -21,7 +21,7 @@ use super::Metadata; const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)"; const DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide"; -const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu"; +pub(super) const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu"; pub(super) const TARGETS: &[&str] = &[ "i686-pc-windows-msvc", "i686-unknown-linux-gnu", @@ -190,7 +190,8 @@ impl RustwideBuilder { .build(&self.toolchain, &krate, sandbox) .run(|build| { let metadata = Metadata::from_source_dir(&build.host_source_dir())?; - let res = self.execute_build(None, build, &limits, &metadata)?; + + let res = self.execute_build(DEFAULT_TARGET, true, build, &limits, &metadata)?; if !res.result.successful { bail!("failed to build dummy crate for {}", self.rustc_version); } @@ -314,9 +315,10 @@ impl RustwideBuilder { let mut has_docs = false; let mut successful_targets = Vec::new(); let metadata = Metadata::from_source_dir(&build.host_source_dir())?; + let (default_target, other_targets) = metadata.select_extra_targets(); // Do an initial build and then copy the sources in the database - let res = self.execute_build(None, &build, &limits, &metadata)?; + let res = self.execute_build(default_target, true, &build, &limits, &metadata)?; if res.result.successful { debug!("adding sources into database"); let prefix = format!("sources/{}/{}", name, version); @@ -342,10 +344,9 @@ impl RustwideBuilder { )?; successful_targets.push(res.target.clone()); - let targets = metadata.select_extra_targets(&res.target); // Then build the documentation for all the targets - for target in targets { + for target in other_targets { debug!("building package {} {} for {}", name, version, target); self.build_target( target, @@ -400,7 +401,7 @@ impl RustwideBuilder { successful_targets: &mut Vec, metadata: &Metadata, ) -> Result<()> { - let target_res = self.execute_build(Some(target), build, limits, metadata)?; + let target_res = self.execute_build(target, false, build, limits, metadata)?; if target_res.result.successful { // Cargo is not giving any error and not generating documentation of some crates // when we use a target compile options. Check documentation exists before @@ -416,7 +417,8 @@ impl RustwideBuilder { fn execute_build( &self, - target: Option<&str>, + target: &str, + is_default_target: bool, build: &Build, limits: &Limits, metadata: &Metadata, @@ -424,9 +426,6 @@ impl RustwideBuilder { let cargo_metadata = CargoMetadata::load(&self.workspace, &self.toolchain, &build.host_source_dir())?; - let is_default_target = target.is_none(); - let target = target.or_else(|| metadata.default_target.as_ref().map(|s| s.as_str())); - let mut rustdoc_flags: Vec = vec![ "-Z".to_string(), "unstable-options".to_string(), @@ -448,9 +447,9 @@ impl RustwideBuilder { rustdoc_flags.append(&mut package_rustdoc_args.iter().map(|s| s.to_owned()).collect()); } let mut cargo_args = vec!["doc".to_owned(), "--lib".to_owned(), "--no-deps".to_owned()]; - if let Some(explicit_target) = target { + if target != DEFAULT_TARGET { cargo_args.push("--target".to_owned()); - cargo_args.push(explicit_target.to_owned()); + cargo_args.push(target.to_owned()); }; if let Some(features) = &metadata.features { cargo_args.push("--features".to_owned()); @@ -488,15 +487,13 @@ impl RustwideBuilder { // cargo will put the output in `target//doc`. // However, if this is the default build, we don't want it there, // we want it in `target/doc`. - if let Some(explicit_target) = target { - if is_default_target { - // mv target/$explicit_target/doc target/doc - let target_dir = build.host_target_dir(); - let old_dir = target_dir.join(explicit_target).join("doc"); - let new_dir = target_dir.join("doc"); - debug!("rename {} to {}", old_dir.display(), new_dir.display()); - std::fs::rename(old_dir, new_dir)?; - } + if target != DEFAULT_TARGET && is_default_target { + // mv target/target/doc target/doc + let target_dir = build.host_target_dir(); + let old_dir = target_dir.join(target).join("doc"); + let new_dir = target_dir.join("doc"); + debug!("rename {} to {}", old_dir.display(), new_dir.display()); + std::fs::rename(old_dir, new_dir)?; } Ok(FullBuildResult { @@ -507,7 +504,7 @@ impl RustwideBuilder { successful, }, cargo_metadata, - target: target.unwrap_or(DEFAULT_TARGET).to_string(), + target: target.to_string(), }) } From 01f159e8b5ec7e62747572c0d954688a165aded5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 21:56:42 -0400 Subject: [PATCH 12/21] DEFAULT_TARGET -> HOST_TARGET --- src/docbuilder/metadata.rs | 4 ++-- src/docbuilder/rustwide_builder.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index f8efa762d..7e87cfd6c 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -130,7 +130,7 @@ impl Metadata { metadata } pub(super) fn select_extra_targets<'a: 'ret, 'b: 'ret, 'ret>(&'a self) -> (&'ret str, Vec<&'ret str>) { - use super::rustwide_builder::{DEFAULT_TARGET, TARGETS}; + use super::rustwide_builder::{HOST_TARGET, TARGETS}; // Let people opt-in to only having specific targets // Ideally this would use Iterator instead of Vec so I could collect to a `HashSet`, // but I had trouble with `chain()` ¯\_(ツ)_/¯ @@ -142,7 +142,7 @@ impl Metadata { // default_target unset and extra_targets set to `[]` let landing_page = if all_targets.is_empty() { - DEFAULT_TARGET + HOST_TARGET } else { // This `swap_remove` has to come before the `sort()` to keep the ordering // `swap_remove` is ok because ordering doesn't matter except for first element diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 4944a9657..d1b7d5877 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -21,7 +21,10 @@ use super::Metadata; const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)"; const DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide"; -pub(super) const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu"; +// It is crucial that this be the same as the host that `docs.rs` is being run on. +// Other values may cause strange and hard-to-debug errors. +// TODO: use `TARGET` instead? I think `TARGET` is only set for build scripts, though. +pub(super) const HOST_TARGET: &str = "x86_64-unknown-linux-gnu"; pub(super) const TARGETS: &[&str] = &[ "i686-pc-windows-msvc", "i686-unknown-linux-gnu", @@ -191,7 +194,7 @@ impl RustwideBuilder { .run(|build| { let metadata = Metadata::from_source_dir(&build.host_source_dir())?; - let res = self.execute_build(DEFAULT_TARGET, true, build, &limits, &metadata)?; + let res = self.execute_build(HOST_TARGET, true, build, &limits, &metadata)?; if !res.result.successful { bail!("failed to build dummy crate for {}", self.rustc_version); } @@ -447,7 +450,7 @@ impl RustwideBuilder { rustdoc_flags.append(&mut package_rustdoc_args.iter().map(|s| s.to_owned()).collect()); } let mut cargo_args = vec!["doc".to_owned(), "--lib".to_owned(), "--no-deps".to_owned()]; - if target != DEFAULT_TARGET { + if target != HOST_TARGET { cargo_args.push("--target".to_owned()); cargo_args.push(target.to_owned()); }; @@ -487,7 +490,7 @@ impl RustwideBuilder { // cargo will put the output in `target//doc`. // However, if this is the default build, we don't want it there, // we want it in `target/doc`. - if target != DEFAULT_TARGET && is_default_target { + if target != HOST_TARGET && is_default_target { // mv target/target/doc target/doc let target_dir = build.host_target_dir(); let old_dir = target_dir.join(target).join("doc"); From cb8cac47ff42172008c75f76ed5bcd2e17b0eb8a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:13:56 -0400 Subject: [PATCH 13/21] Fix up tests --- src/docbuilder/metadata.rs | 50 ++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 7e87cfd6c..1f28fbab7 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -137,6 +137,11 @@ impl Metadata { let mut all_targets: Vec<_> = self.default_target.as_deref().into_iter().collect(); match &self.extra_targets { Some(targets) => all_targets.extend(targets.iter().map(|s| s.as_str())), + None if all_targets.is_empty() => { + // Make sure HOST_TARGET is first + all_targets.push(HOST_TARGET); + all_targets.extend(TARGETS.iter().copied().filter(|&t| t != HOST_TARGET)); + } None => all_targets.extend(TARGETS.iter().copied()), }; @@ -242,23 +247,19 @@ mod test { } #[test] fn test_select_extra_targets() { - use crate::docbuilder::rustwide_builder::TARGETS; - - let mut metadata = Metadata { - extra_targets: None, - ..Metadata::default() - }; - const DEFAULT_TARGET: &str = "x86-unknown-linux-gnu"; + use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; + let mut metadata = Metadata::default(); // unchanged default_target, extra targets not specified - let tier_one = metadata.select_extra_targets(DEFAULT_TARGET); - // should be equal to TARGETS except for DEFAULT_TARGET + let (default, tier_one) = metadata.select_extra_targets(); + assert_eq!(default, HOST_TARGET); + // should be equal to TARGETS \ {HOST_TARGET} for actual in &tier_one { assert!(TARGETS.contains(actual)); } for expected in TARGETS { - if *expected == DEFAULT_TARGET { - assert!(!tier_one.contains(DEFAULT_TARGET)); + if *expected == HOST_TARGET { + assert!(!tier_one.contains(&HOST_TARGET)); } else { assert!(tier_one.contains(expected)); } @@ -266,24 +267,27 @@ mod test { // unchanged default_target, extra targets specified to be empty metadata.extra_targets = Some(Vec::new()); - assert!(metadata.select_extra_targets(DEFAULT_TARGET).is_empty()); + let (default, others) = metadata.select_extra_targets(); + assert_eq!(default, HOST_TARGET); + assert!(others.is_empty()); - // unchanged default_target, extra targets includes targets besides default_target + // unchanged default_target, extra targets non-empty metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); - let extras = metadata.select_extra_targets(DEFAULT_TARGET); - assert_eq!(extras.len(), 2); - assert!(extras.contains("i686-pc-windows-msvc")); - assert!(extras.contains("i686-apple-darwin")); + let (default, others) = metadata.select_extra_targets(); + assert_eq!(default, "i686-pc-windows-msvc"); + assert_eq!(others.len(), 1); + assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice - metadata.extra_targets = Some(vec![DEFAULT_TARGET.into()]); - assert!(metadata.select_extra_targets(DEFAULT_TARGET).is_empty()); + metadata.extra_targets = Some(vec![HOST_TARGET.into()]); + let (default, others) = metadata.select_extra_targets(); + assert_eq!(default, HOST_TARGET); + assert!(others.is_empty()); // make sure that duplicates are removed metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); - assert_eq!(metadata.select_extra_targets(DEFAULT_TARGET).len(), 1); - - // try it with a different default target just for sanity - assert!(metadata.select_extra_targets("i686-pc-windows-msvc").is_empty()); + let (default, others) = metadata.select_extra_targets(); + assert_eq!(default, "i686-pc-windows-msvc"); + assert!(others.is_empty()); } } From 18e104600bc51bc397f0152b71b7cf606d9bf942 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:15:49 -0400 Subject: [PATCH 14/21] cleanup --- src/docbuilder/metadata.rs | 13 +++++++------ src/docbuilder/rustwide_builder.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 1f28fbab7..d99f84e5e 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -129,7 +129,8 @@ impl Metadata { metadata } - pub(super) fn select_extra_targets<'a: 'ret, 'b: 'ret, 'ret>(&'a self) -> (&'ret str, Vec<&'ret str>) { + // Return (default_target, all other targets that should be built with duplicates removed) + pub(super) fn targets(&self) -> (&str, Vec<&str>) { use super::rustwide_builder::{HOST_TARGET, TARGETS}; // Let people opt-in to only having specific targets // Ideally this would use Iterator instead of Vec so I could collect to a `HashSet`, @@ -251,7 +252,7 @@ mod test { let mut metadata = Metadata::default(); // unchanged default_target, extra targets not specified - let (default, tier_one) = metadata.select_extra_targets(); + let (default, tier_one) = metadata.targets(); assert_eq!(default, HOST_TARGET); // should be equal to TARGETS \ {HOST_TARGET} for actual in &tier_one { @@ -267,26 +268,26 @@ mod test { // unchanged default_target, extra targets specified to be empty metadata.extra_targets = Some(Vec::new()); - let (default, others) = metadata.select_extra_targets(); + let (default, others) = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // unchanged default_target, extra targets non-empty metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); - let (default, others) = metadata.select_extra_targets(); + let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice metadata.extra_targets = Some(vec![HOST_TARGET.into()]); - let (default, others) = metadata.select_extra_targets(); + let (default, others) = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // make sure that duplicates are removed metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); - let (default, others) = metadata.select_extra_targets(); + let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index d1b7d5877..91d46b6b7 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -318,7 +318,7 @@ impl RustwideBuilder { let mut has_docs = false; let mut successful_targets = Vec::new(); let metadata = Metadata::from_source_dir(&build.host_source_dir())?; - let (default_target, other_targets) = metadata.select_extra_targets(); + let (default_target, other_targets) = metadata.targets(); // Do an initial build and then copy the sources in the database let res = self.execute_build(default_target, true, &build, &limits, &metadata)?; From c9bffa826284658dd4ec38bec2b4a9671c8ae576 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:23:10 -0400 Subject: [PATCH 15/21] Add more tests --- src/docbuilder/metadata.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index d99f84e5e..c4e3ee5fa 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -290,5 +290,26 @@ mod test { let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); + + // make sure that `default_target` always takes priority over `targets` + metadata.default_target = Some("i686-apple-darwin".into()); + let (default, others) = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); + assert_eq!(others.len(), 1); + assert!(others.contains(&"i686-pc-windows-msvc")); + + // make sure that `default_target` takes priority over `HOST_TARGET` + metadata.extra_targets = Some(vec![]); + let (default, others) = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); + assert!(others.is_empty()); + + // and if `extra_targets` is unset, it should still be set to `TARGETS` + metadata.extra_targets = None; + let (default, others) = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); + let tier_one_targets_no_default: Vec<_> = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); + assert_eq!(others, tier_one_targets_no_default); + } } From e06990bc578dbc547f04e8f363894d622409960d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:34:42 -0400 Subject: [PATCH 16/21] extra-targets -> targets and add documentation --- src/docbuilder/metadata.rs | 62 +++++++++++++++++++++----------------- templates/about.hbs | 10 ++++-- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index c4e3ee5fa..580411567 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -19,7 +19,7 @@ use failure::err_msg; /// all-features = true /// no-default-features = true /// default-target = "x86_64-unknown-linux-gnu" -/// extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] +/// targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] /// rustc-args = [ "--example-rustc-arg" ] /// rustdoc-args = [ "--example-rustdoc-arg" ] /// ``` @@ -39,16 +39,22 @@ pub struct Metadata { /// Set `no-default-fatures` to `false` if you want to build only certain features. pub no_default_features: bool, - /// Docs.rs is running on `x86_64-unknown-linux-gnu` target system and default documentation - /// is always built on this target. You can change default target by setting this. + /// docs.rs runs on `x86_64-unknown-linux-gnu`, which is the default target for documentation by default. + /// + /// You can change the default target by setting this. + /// + /// If `default_target` is unset and `targets` is non-empty, + /// the first element of `targets` will be used as the `default_target`. pub default_target: Option, /// If you want a crate to build only for specific targets, - /// set `extra-targets` to the list of targets to build, in addition to `default-target`. + /// set `targets` to the list of targets to build, in addition to `default-target`. /// - /// If you do not set `extra_targets`, all of the tier 1 supported targets will be built. - /// If you set `extra_targets` to an empty array, only the default target will be built. - pub extra_targets: Option>, + /// If you do not set `targets`, all of the tier 1 supported targets will be built. + /// If you set `targets` to an empty array, only the default target will be built. + /// If you set `targets` to a non-empty array but do not set `default_target`, + /// the first element will be treated as the default. + pub targets: Option>, /// List of command line arguments for `rustc`. pub rustc_args: Option>, @@ -94,7 +100,7 @@ impl Metadata { default_target: None, rustc_args: None, rustdoc_args: None, - extra_targets: None, + targets: None, } } @@ -119,7 +125,7 @@ impl Metadata { .and_then(|v| v.as_bool()).unwrap_or(metadata.all_features); metadata.default_target = table.get("default-target") .and_then(|v| v.as_str()).map(|v| v.to_owned()); - metadata.extra_targets = table.get("extra-targets").and_then(|f| f.as_array()) + metadata.targets = table.get("extra-targets").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); @@ -136,7 +142,7 @@ impl Metadata { // Ideally this would use Iterator instead of Vec so I could collect to a `HashSet`, // but I had trouble with `chain()` ¯\_(ツ)_/¯ let mut all_targets: Vec<_> = self.default_target.as_deref().into_iter().collect(); - match &self.extra_targets { + match &self.targets { Some(targets) => all_targets.extend(targets.iter().map(|s| s.as_str())), None if all_targets.is_empty() => { // Make sure HOST_TARGET is first @@ -146,7 +152,7 @@ impl Metadata { None => all_targets.extend(TARGETS.iter().copied()), }; - // default_target unset and extra_targets set to `[]` + // default_target unset and targets set to `[]` let landing_page = if all_targets.is_empty() { HOST_TARGET } else { @@ -205,10 +211,10 @@ mod test { assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); - let extra_targets = metadata.extra_targets.expect("should have explicit extra target"); - assert_eq!(extra_targets.len(), 2); - assert_eq!(extra_targets[0], "x86_64-apple-darwin"); - assert_eq!(extra_targets[1], "x86_64-pc-windows-msvc"); + let targets = metadata.targets.expect("should have explicit extra target"); + assert_eq!(targets.len(), 2); + assert_eq!(targets[0], "x86_64-apple-darwin"); + assert_eq!(targets[1], "x86_64-pc-windows-msvc"); let rustc_args = metadata.rustc_args.unwrap(); assert_eq!(rustc_args.len(), 1); @@ -220,8 +226,8 @@ mod test { } #[test] - fn test_no_extra_targets() { - // metadata section but no extra_targets + fn test_no_targets() { + // metadata section but no targets let manifest = r#" [package] name = "test" @@ -230,24 +236,24 @@ mod test { features = [ "feature1", "feature2" ] "#; let metadata = Metadata::from_str(manifest); - assert!(metadata.extra_targets.is_none()); + assert!(metadata.targets.is_none()); // no package.metadata.docs.rs section let metadata = Metadata::from_str(r#" [package] name = "test" "#); - assert!(metadata.extra_targets.is_none()); + assert!(metadata.targets.is_none()); // extra targets explicitly set to empty array let metadata = Metadata::from_str(r#" [package.metadata.docs.rs] extra-targets = [] "#); - assert!(metadata.extra_targets.unwrap().is_empty()); + assert!(metadata.targets.unwrap().is_empty()); } #[test] - fn test_select_extra_targets() { + fn test_select_targets() { use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; let mut metadata = Metadata::default(); @@ -267,26 +273,26 @@ mod test { } // unchanged default_target, extra targets specified to be empty - metadata.extra_targets = Some(Vec::new()); + metadata.targets = Some(Vec::new()); let (default, others) = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // unchanged default_target, extra targets non-empty - metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); + metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice - metadata.extra_targets = Some(vec![HOST_TARGET.into()]); + metadata.targets = Some(vec![HOST_TARGET.into()]); let (default, others) = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // make sure that duplicates are removed - metadata.extra_targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); + metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); @@ -299,13 +305,13 @@ mod test { assert!(others.contains(&"i686-pc-windows-msvc")); // make sure that `default_target` takes priority over `HOST_TARGET` - metadata.extra_targets = Some(vec![]); + metadata.targets = Some(vec![]); let (default, others) = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); assert!(others.is_empty()); - // and if `extra_targets` is unset, it should still be set to `TARGETS` - metadata.extra_targets = None; + // and if `targets` is unset, it should still be set to `TARGETS` + metadata.targets = None; let (default, others) = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); let tier_one_targets_no_default: Vec<_> = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); diff --git a/templates/about.hbs b/templates/about.hbs index 07ccb4adf..a6404c0ee 100644 --- a/templates/about.hbs +++ b/templates/about.hbs @@ -162,10 +162,16 @@ no-default-features = true # - i686-pc-windows-msvc default-target = "x86_64-unknown-linux-gnu" -# Targets to build in addition to `default-target` (default: all tier 1 targets) +# Targets to build (default: all tier 1 targets) +# # Same available targets as `default-target`. # Set this to `[]` to only build the default target. -extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] +# +# If `default-target` is unset, the first element of `targets` is treated as the default target. +# Otherwise, these `targets` are built in addition to the default target. +# If both `default-target` and `targets` are unset, +# all tier-one targets will be built and `x86_64-unknown-linux-gnu` will be used as the default target. +targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] # Additional `RUSTFLAGS` to set (default: none) rustc-args = [ "--example-rustc-arg" ] From 2761acb88d313258677bc284e4b2206ea1875c16 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:37:58 -0400 Subject: [PATCH 17/21] Fix reading wrong key in manifest --- src/docbuilder/metadata.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 580411567..997d08779 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -125,7 +125,7 @@ impl Metadata { .and_then(|v| v.as_bool()).unwrap_or(metadata.all_features); metadata.default_target = table.get("default-target") .and_then(|v| v.as_str()).map(|v| v.to_owned()); - metadata.targets = table.get("extra-targets").and_then(|f| f.as_array()) + metadata.targets = table.get("targets").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); @@ -191,7 +191,7 @@ mod test { all-features = true no-default-features = true default-target = "x86_64-unknown-linux-gnu" - extra-targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] + targets = [ "x86_64-apple-darwin", "x86_64-pc-windows-msvc" ] rustc-args = [ "--example-rustc-arg" ] rustdoc-args = [ "--example-rustdoc-arg" ] "#; @@ -248,7 +248,7 @@ mod test { // extra targets explicitly set to empty array let metadata = Metadata::from_str(r#" [package.metadata.docs.rs] - extra-targets = [] + targets = [] "#); assert!(metadata.targets.unwrap().is_empty()); } @@ -257,7 +257,7 @@ mod test { use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; let mut metadata = Metadata::default(); - // unchanged default_target, extra targets not specified + // unchanged default_target, targets not specified let (default, tier_one) = metadata.targets(); assert_eq!(default, HOST_TARGET); // should be equal to TARGETS \ {HOST_TARGET} From ff96849fc4341304b2005817e0863e230b7f6a72 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 8 Mar 2020 22:38:59 -0400 Subject: [PATCH 18/21] formatting --- src/docbuilder/metadata.rs | 1 - src/docbuilder/rustwide_builder.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 997d08779..cc411df6e 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -316,6 +316,5 @@ mod test { assert_eq!(default, "i686-apple-darwin"); let tier_one_targets_no_default: Vec<_> = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); assert_eq!(others, tier_one_targets_no_default); - } } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 91d46b6b7..b6fe547a0 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -491,7 +491,7 @@ impl RustwideBuilder { // However, if this is the default build, we don't want it there, // we want it in `target/doc`. if target != HOST_TARGET && is_default_target { - // mv target/target/doc target/doc + // mv target/$target/doc target/doc let target_dir = build.host_target_dir(); let old_dir = target_dir.join(target).join("doc"); let new_dir = target_dir.join("doc"); From 43c24c764fb353c96cac2acfb3c3302e26aa9bfa Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 10 Mar 2020 19:24:07 -0400 Subject: [PATCH 19/21] extra targets -> targets in comments --- src/docbuilder/metadata.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index cc411df6e..a4aecb7ed 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -211,7 +211,7 @@ mod test { assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); - let targets = metadata.targets.expect("should have explicit extra target"); + let targets = metadata.targets.expect("should have explicit target"); assert_eq!(targets.len(), 2); assert_eq!(targets[0], "x86_64-apple-darwin"); assert_eq!(targets[1], "x86_64-pc-windows-msvc"); @@ -245,7 +245,7 @@ mod test { "#); assert!(metadata.targets.is_none()); - // extra targets explicitly set to empty array + // targets explicitly set to empty array let metadata = Metadata::from_str(r#" [package.metadata.docs.rs] targets = [] @@ -272,13 +272,13 @@ mod test { } } - // unchanged default_target, extra targets specified to be empty + // unchanged default_target, targets specified to be empty metadata.targets = Some(Vec::new()); let (default, others) = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); - // unchanged default_target, extra targets non-empty + // unchanged default_target, targets non-empty metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); let (default, others) = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); From f4ff03039b6c72e25e290769f454ef1c37b7d1a3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 11 Mar 2020 14:27:41 -0400 Subject: [PATCH 20/21] Use BuildTargets and simplify implementation --- src/docbuilder/metadata.rs | 73 ++++++++++++++---------------- src/docbuilder/rustwide_builder.rs | 4 +- 2 files changed, 36 insertions(+), 41 deletions(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index a4aecb7ed..8e43eabd3 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::path::Path; use toml::Value; use error::Result; @@ -63,7 +64,16 @@ pub struct Metadata { pub rustdoc_args: Option>, } - +/// The targets that should be built for a crate. +/// +/// The `default_target` is the target to be used as the home page for that crate. +/// +/// # See also +/// - [`Metadata::targets`](struct.Metadata.html#method.targets) +pub(super) struct BuildTargets<'a> { + pub(super) default_target: &'a str, + pub(super) other_targets: HashSet<&'a str>, +} impl Metadata { pub(crate) fn from_source_dir(source_dir: &Path) -> Result { @@ -136,39 +146,21 @@ impl Metadata { metadata } // Return (default_target, all other targets that should be built with duplicates removed) - pub(super) fn targets(&self) -> (&str, Vec<&str>) { + pub(super) fn targets(&self) -> BuildTargets<'_> { use super::rustwide_builder::{HOST_TARGET, TARGETS}; + + let default_target = self.default_target.as_deref() + // Use the first element of `targets` if `default_target` is unset and `targets` is non-empty + .or_else(|| self.targets.as_ref().and_then(|targets| targets.iter().next().map(String::as_str))) + .unwrap_or(HOST_TARGET); + // Let people opt-in to only having specific targets - // Ideally this would use Iterator instead of Vec so I could collect to a `HashSet`, - // but I had trouble with `chain()` ¯\_(ツ)_/¯ - let mut all_targets: Vec<_> = self.default_target.as_deref().into_iter().collect(); - match &self.targets { - Some(targets) => all_targets.extend(targets.iter().map(|s| s.as_str())), - None if all_targets.is_empty() => { - // Make sure HOST_TARGET is first - all_targets.push(HOST_TARGET); - all_targets.extend(TARGETS.iter().copied().filter(|&t| t != HOST_TARGET)); - } - None => all_targets.extend(TARGETS.iter().copied()), - }; + let mut targets: HashSet<_> = self.targets.as_ref() + .map(|targets| targets.iter().map(String::as_str).collect()) + .unwrap_or_else(|| TARGETS.iter().copied().collect()); - // default_target unset and targets set to `[]` - let landing_page = if all_targets.is_empty() { - HOST_TARGET - } else { - // This `swap_remove` has to come before the `sort()` to keep the ordering - // `swap_remove` is ok because ordering doesn't matter except for first element - all_targets.swap_remove(0) - }; - // Remove duplicates - all_targets.sort(); - all_targets.dedup(); - // Remove landing_page so we don't build it twice. - // It wasn't removed during dedup because we called `swap_remove()` first. - if let Ok(index) = all_targets.binary_search(&landing_page) { - all_targets.swap_remove(index); - } - (landing_page, all_targets) + targets.remove(&default_target); + BuildTargets { default_target, other_targets: targets } } } @@ -255,10 +247,11 @@ mod test { #[test] fn test_select_targets() { use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; + use super::BuildTargets; let mut metadata = Metadata::default(); // unchanged default_target, targets not specified - let (default, tier_one) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: tier_one } = metadata.targets(); assert_eq!(default, HOST_TARGET); // should be equal to TARGETS \ {HOST_TARGET} for actual in &tier_one { @@ -274,47 +267,47 @@ mod test { // unchanged default_target, targets specified to be empty metadata.targets = Some(Vec::new()); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // unchanged default_target, targets non-empty metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice metadata.targets = Some(vec![HOST_TARGET.into()]); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // make sure that duplicates are removed metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); // make sure that `default_target` always takes priority over `targets` metadata.default_target = Some("i686-apple-darwin".into()); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-pc-windows-msvc")); // make sure that `default_target` takes priority over `HOST_TARGET` metadata.targets = Some(vec![]); - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); assert!(others.is_empty()); // and if `targets` is unset, it should still be set to `TARGETS` metadata.targets = None; - let (default, others) = metadata.targets(); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); - let tier_one_targets_no_default: Vec<_> = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); + let tier_one_targets_no_default = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); assert_eq!(others, tier_one_targets_no_default); } } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index b6fe547a0..81298edcc 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -314,11 +314,13 @@ impl RustwideBuilder { let res = build_dir .build(&self.toolchain, &krate, sandbox) .run(|build| { + use docbuilder::metadata::BuildTargets; + let mut files_list = None; let mut has_docs = false; let mut successful_targets = Vec::new(); let metadata = Metadata::from_source_dir(&build.host_source_dir())?; - let (default_target, other_targets) = metadata.targets(); + let BuildTargets { default_target, other_targets } = metadata.targets(); // Do an initial build and then copy the sources in the database let res = self.execute_build(default_target, true, &build, &limits, &metadata)?; From 3a41224faa31c0c0c5cd6f22d4fba6eef82b7e85 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 11 Mar 2020 14:29:42 -0400 Subject: [PATCH 21/21] Remove out-of-date comment --- src/docbuilder/metadata.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 8e43eabd3..3fd1bd768 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -145,7 +145,6 @@ impl Metadata { metadata } - // Return (default_target, all other targets that should be built with duplicates removed) pub(super) fn targets(&self) -> BuildTargets<'_> { use super::rustwide_builder::{HOST_TARGET, TARGETS};