Merge pull request #534 from jyn514/one-default-target

Don't build default target twice

Co-authored-by: QuietMisdreavus <[email protected]>

Author: pietroalbini

src/db/add_package.rs

@@ -26,9 +26,9 @@ pub(crate) fn add_package_into_database(conn: &Connection,
                                  metadata_pkg: &MetadataPackage,
                                  source_dir: &Path,
                                  res: &BuildResult,
+                                 default_target: &str,
                                  files: Option<Json>,
                                  doc_targets: Vec<String>,
-                                 default_target: &Option<String>,
                                  cratesio_data: &CratesIoData,
                                  has_docs: bool,
                                  has_examples: bool)

src/db/migrate.rs

@@ -269,6 +269,19 @@ fn migrate_inner(version: Option<Version>, conn: &Connection, apply_mode: ApplyM
             // downgrade query
             "DROP TABLE blacklisted_crates;"
         ),
+        migration!(
+            context,
+            // version
+            7,
+            // description
+            "Make default_target non-nullable",
+            // upgrade query
+            "UPDATE releases SET default_target = 'x86_64-unknown-linux-gnu' WHERE default_target IS NULL;
+             ALTER TABLE releases ALTER COLUMN default_target SET NOT NULL",
+            // downgrade query
+            "ALTER TABLE releases ALTER COLUMN default_target DROP NOT NULL;
+             ALTER TABLE releases ALTER COLUMN default_target DROP DEFAULT",
+        ),
     ];
 
     for migration in migrations {

src/docbuilder/rustwide_builder.rs

@@ -18,18 +18,19 @@ use std::path::Path;
 use utils::{copy_doc_dir, parse_rustc_version, CargoMetadata};
 use Metadata;
 
-static USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
-static DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide";
+const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
+const DEFAULT_RUSTWIDE_WORKSPACE: &str = ".rustwide";
 
-static TARGETS: &[&str] = &[
+const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu";
+const TARGETS: &[&str] = &[
     "i686-pc-windows-msvc",
     "i686-unknown-linux-gnu",
     "x86_64-apple-darwin",
     "x86_64-pc-windows-msvc",
     "x86_64-unknown-linux-gnu",
 ];
 
-static ESSENTIAL_FILES_VERSIONED: &[&str] = &[
+const ESSENTIAL_FILES_VERSIONED: &[&str] = &[
     "brush.svg",
     "wheel.svg",
     "down-arrow.svg",
@@ -46,7 +47,7 @@ static ESSENTIAL_FILES_VERSIONED: &[&str] = &[
     "noscript.css",
     "rust-logo.png",
 ];
-static ESSENTIAL_FILES_UNVERSIONED: &[&str] = &[
+const ESSENTIAL_FILES_UNVERSIONED: &[&str] = &[
     "FiraSans-Medium.woff",
     "FiraSans-Regular.woff",
     "SourceCodePro-Regular.woff",
@@ -56,8 +57,8 @@ static ESSENTIAL_FILES_UNVERSIONED: &[&str] = &[
     "SourceSerifPro-It.ttf.woff",
 ];
 
-static DUMMY_CRATE_NAME: &str = "acme-client";
-static DUMMY_CRATE_VERSION: &str = "0.0.0";
+const DUMMY_CRATE_NAME: &str = "acme-client";
+const DUMMY_CRATE_VERSION: &str = "0.0.0";
 
 pub struct RustwideBuilder {
     workspace: Workspace,
@@ -189,7 +190,7 @@ impl RustwideBuilder {
                 }
 
                 info!("copying essential files for {}", self.rustc_version);
-                let source = build.host_target_dir().join(&res.target).join("doc");
+                let source = build.host_target_dir().join("doc");
                 let dest = ::tempdir::TempDir::new("essential-files")?;
 
                 let files = ESSENTIAL_FILES_VERSIONED
@@ -303,7 +304,7 @@ impl RustwideBuilder {
             .build(&self.toolchain, &krate, sandbox)
             .run(|build| {
                 let mut files_list = None;
-                let (mut has_docs, mut in_target) = (false, false);
+                let mut has_docs = false;
                 let mut successful_targets = Vec::new();
 
                 // Do an initial build and then copy the sources in the database
@@ -319,20 +320,7 @@ impl RustwideBuilder {
 
                     if let Some(name) = res.cargo_metadata.root().library_name() {
                         let host_target = build.host_target_dir();
-                        if host_target
-                            .join(&res.target)
-                            .join("doc")
-                            .join(&name)
-                            .is_dir()
-                        {
-                            has_docs = true;
-                            in_target = true;
-                        // hack for proc-macro documentation:
-                        // it really should be in target/$target/doc,
-                        // but rustdoc has a bug and puts it in target/doc
-                        } else if host_target.join("doc").join(name).is_dir() {
-                            has_docs = true;
-                        }
+                        has_docs = host_target.join("doc").join(name).is_dir();
                     }
                 }
 
@@ -341,22 +329,24 @@ impl RustwideBuilder {
                     self.copy_docs(
                         &build.host_target_dir(),
                         local_storage.path(),
-                        if in_target { &res.target } else { "" },
+                        "",
                         true,
                     )?;
 
-                    if in_target {
-                        // Then build the documentation for all the targets
-                        for target in TARGETS {
-                            debug!("building package {} {} for {}", name, version, target);
-                            self.build_target(
-                                target,
-                                &build,
-                                &limits,
-                                &local_storage.path(),
-                                &mut successful_targets,
-                            )?;
+                    successful_targets.push(res.target.clone());
+                    // Then build the documentation for all the targets
+                    for target in TARGETS {
+                        if *target == res.target {
+                            continue;
                         }
+                        debug!("building package {} {} for {}", name, version, &target);
+                        self.build_target(
+                            &target,
+                            &build,
+                            &limits,
+                            &local_storage.path(),
+                            &mut successful_targets,
+                        )?;
                     }
                     self.upload_docs(&conn, name, version, local_storage.path())?;
                 }
@@ -374,9 +364,9 @@ impl RustwideBuilder {
                     res.cargo_metadata.root(),
                     &build.host_source_dir(),
                     &res.result,
+                    &res.target,
                     files_list,
                     successful_targets,
-                    &res.default_target,
                     &CratesIoData::get_from_network(res.cargo_metadata.root())?,
                     has_docs,
                     has_examples,
@@ -425,6 +415,7 @@ 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<String> = vec![
@@ -484,6 +475,20 @@ impl RustwideBuilder {
                 .run()
                 .is_ok()
         });
+        // If we're passed a default_target which requires a cross-compile,
+        // cargo will put the output in `target/<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)?;
+            }
+        }
 
         Ok(FullBuildResult {
             result: BuildResult {
@@ -493,8 +498,7 @@ impl RustwideBuilder {
                 successful,
             },
             cargo_metadata,
-            target: target.unwrap_or_default().to_string(),
-            default_target: metadata.default_target.clone(),
+            target: target.unwrap_or(DEFAULT_TARGET).to_string(),
         })
     }
 
@@ -542,7 +546,6 @@ impl RustwideBuilder {
 struct FullBuildResult {
     result: BuildResult,
     target: String,
-    default_target: Option<String>,
     cargo_metadata: CargoMetadata,
 }
 

src/test/fakes.rs

@@ -84,9 +84,9 @@ impl<'db> FakeRelease<'db> {
             &self.package,
             tempdir.path(),
             &self.build_result,
+            self.default_target.as_deref().unwrap_or("x86_64-unknown-linux-gnu"),
             self.files,
             self.doc_targets,
-            &self.default_target,
             &self.cratesio_data,
             self.has_docs,
             self.has_examples,

src/web/crate_details.rs

@@ -43,7 +43,7 @@ pub struct CrateDetails {
     github_stars: Option<i32>,
     github_forks: Option<i32>,
     github_issues: Option<i32>,
-    metadata: MetaData,
+    pub metadata: MetaData,
     is_library: bool,
     doc_targets: Option<Json>,
     license: Option<String>,
@@ -120,7 +120,8 @@ impl CrateDetails {
                             releases.is_library,
                             releases.doc_targets,
                             releases.license,
-                            releases.documentation_url
+                            releases.documentation_url,
+                            releases.default_target
                      FROM releases
                      INNER JOIN crates ON releases.crate_id = crates.id
                      WHERE crates.name = $1 AND releases.version = $2;";
@@ -160,6 +161,7 @@ impl CrateDetails {
             description: rows.get(0).get(4),
             rustdoc_status: rows.get(0).get(11),
             target_name: rows.get(0).get(16),
+            default_target: rows.get(0).get(25),
         };
 
         let mut crate_details = CrateDetails {

src/web/mod.rs

@@ -451,6 +451,7 @@ pub struct MetaData {
     pub description: Option<String>,
     pub target_name: Option<String>,
     pub rustdoc_status: bool,
+    pub default_target: String,
 }
 
 
@@ -460,7 +461,8 @@ impl MetaData {
                                        releases.version,
                                        releases.description,
                                        releases.target_name,
-                                       releases.rustdoc_status
+                                       releases.rustdoc_status,
+                                       releases.default_target
                                 FROM releases
                                 INNER JOIN crates ON crates.id = releases.crate_id
                                 WHERE crates.name = $1 AND releases.version = $2",
@@ -473,6 +475,7 @@ impl MetaData {
                 description: row.get(2),
                 target_name: row.get(3),
                 rustdoc_status: row.get(4),
+                default_target: row.get(5),
             });
         }
 
@@ -489,6 +492,7 @@ impl ToJson for MetaData {
         m.insert("description".to_owned(), self.description.to_json());
         m.insert("target_name".to_owned(), self.target_name.to_json());
         m.insert("rustdoc_status".to_owned(), self.rustdoc_status.to_json());
+        m.insert("default_target".to_owned(), self.default_target.to_json());
         m.to_json()
     }
 }

src/web/rustdoc.rs

@@ -193,6 +193,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
     let url_version = router.find("version");
     let version; // pre-declaring it to enforce drop order relative to `req_path`
     let conn = extension!(req, Pool);
+    let base = redirect_base(req);
 
     let mut req_path = req.url.path();
 
@@ -208,7 +209,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
             // versions, redirect the browser to the returned version instead of loading it
             // immediately
             let url = ctry!(Url::parse(&format!("{}/{}/{}/{}",
-                                                redirect_base(req),
+                                                base,
                                                 name,
                                                 v,
                                                 req_path.join("/"))[..]));
@@ -224,6 +225,15 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
     req_path.insert(1, &name);
     req_path.insert(2, &version);
 
+    // if visiting the full path to the default target, remove the target from the path
+    // expects a req_path that looks like `/rustdoc/:crate/:version[/:target]/.*`
+    let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version));
+    if req_path[3] == crate_details.metadata.default_target {
+        let path = [base, req_path[1..3].join("/"), req_path[4..].join("/")].join("/");
+        let canonical = Url::parse(&path).expect("got an invalid URL to start");
+        return Ok(super::redirect(canonical));
+    }
+
     let path = {
         let mut path = req_path.join("/");
         if path.ends_with('/') {
@@ -261,7 +271,6 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult<Response> {
     content.body_class = body_class;
 
     content.full = file_content;
-    let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version));
     let (path, version) = if let Some(version) = latest_version(&crate_details.versions, &version) {
         req_path[2] = &version;
         (path_for_version(&req_path, &crate_details.target_name, &conn), version)

src/web/source.rs

@@ -93,7 +93,8 @@ impl FileList {
                                       releases.description,
                                       releases.target_name,
                                       releases.rustdoc_status,
-                                      releases.files
+                                      releases.files,
+                                      releases.default_target
                                FROM releases
                                LEFT OUTER JOIN crates ON crates.id = releases.crate_id
                                WHERE crates.name = $1 AND releases.version = $2",
@@ -173,6 +174,7 @@ impl FileList {
                 description: rows.get(0).get(2),
                 target_name: rows.get(0).get(3),
                 rustdoc_status: rows.get(0).get(4),
+                default_target: rows.get(0).get(5),
             },
             files: file_list,
         })

templates/about.hbs

@@ -127,21 +127,41 @@
     </tbody>
   </table>
 
-  <h4>Metadata for custom builds</h4>
+  <h4 id="metadata"><a href="#metadata">Metadata for custom builds</a></h4>
 
   <p>You can customize docs.rs builds by defining <code>[package.metadata.docs.rs]</code> table in your crates' `Cargo.toml`.</p>
 
-  <p>An example metadata:</p>
+  <p>The available configuration flags you can customize are:</p>
 
   <pre><code>[package]
 name = "test"
 
 [package.metadata.docs.rs]
+
+# Features to pass to Cargo (default: [])
 features = [ "feature1", "feature2" ]
+
+# Whether to pass `--all-features` to Cargo (default: false)
 all-features = true
+
+# Whether to pass `--no-default-features` to Cargo (default: false)
 no-default-features = true
+
+# Target to test build on, used as the default landing page (default: "x86_64-unknown-linux-gnu")
+#
+# Available targets:
+# - x86_64-unknown-linux-gnu
+# - x86_64-apple-darwin
+# - x86_64-pc-windows-msvc
+# - i686-unknown-linux-gnu
+# - i686-apple-darwin
+# - i686-pc-windows-msvc
 default-target = "x86_64-unknown-linux-gnu"
+
+# Additional `RUSTFLAGS` to set (default: [])
 rustc-args = [ "--example-rustc-arg" ]
+
+# Additional `RUSTDOCFLAGS` to set (default: [])
 rustdoc-args = [ "--example-rustdoc-arg" ]</pre></code>
 
   <h4>Version</h4>