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

Remove duplicate default target (redux)

Author: pietroalbini

src/db/add_package.rs

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

src/db/migrate.rs

@@ -280,6 +280,19 @@ fn migrate_inner(version: Option<Version>, conn: &Connection, apply_mode: ApplyM
             // downgrade query
             "ALTER TABLE sandbox_overrides ALTER COLUMN max_memory_bytes TYPE INTEGER;"
         ),
+        migration!(
+            context,
+            // version
+            8,
+            // 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

@@ -14,7 +14,7 @@ pub(crate) struct FakeRelease<'a> {
     /// name, content
     rustdoc_files: Vec<(&'a str, &'a [u8])>,
     doc_targets: Vec<String>,
-    default_target: Option<String>,
+    default_target: Option<&'a str>,
     cratesio_data: CratesIoData,
     has_docs: bool,
     has_examples: bool,
@@ -67,6 +67,7 @@ impl<'a> FakeRelease<'a> {
     pub(crate) fn name(mut self, new: &str) -> Self {
         self.package.name = new.into();
         self.package.id = format!("{}-id", new);
+        self.package.targets[0].name = new.into();
         self
     }
 
@@ -90,16 +91,28 @@ impl<'a> FakeRelease<'a> {
         self
     }
 
+    pub(crate) fn default_target(mut self, target: &'a str) -> Self {
+        self.default_target = Some(target);
+        self
+    }
+
     pub(crate) fn create(self) -> Result<i32, Error> {
+        use std::fs;
+        use std::path::Path;
+
         let tempdir = tempdir::TempDir::new("docs.rs-fake")?;
 
         let upload_files = |prefix: &str, files: &[(&str, &[u8])]| {
             let path_prefix = tempdir.path().join(prefix);
-            std::fs::create_dir(&path_prefix)?;
+            fs::create_dir(&path_prefix)?;
 
             for (path, data) in files {
+                // allow `src/main.rs`
+                if let Some(parent) = Path::new(path).parent() {
+                    fs::create_dir_all(path_prefix.join(parent))?;
+                }
                 let file = path_prefix.join(&path);
-                std::fs::write(file, data)?;
+                fs::write(file, data)?;
             }
 
             let prefix = format!("{}/{}/{}", prefix, self.package.name, self.package.version);
@@ -116,9 +129,9 @@ impl<'a> FakeRelease<'a> {
             &self.package,
             tempdir.path(),
             &self.build_result,
+            self.default_target.unwrap_or("x86_64-unknown-linux-gnu"),
             Some(source_meta),
             self.doc_targets,
-            &self.default_target,
             &self.cratesio_data,
             self.has_docs,
             self.has_examples,

src/test/mod.rs

@@ -37,6 +37,20 @@ pub(crate) fn assert_success(path: &str, web: &TestFrontend) -> Result<(), Error
     Ok(())
 }
 
+/// Make sure that a URL redirects to a specific page
+pub(crate) fn assert_redirect(path: &str, expected_target: &str, web: &TestFrontend) -> Result<(), Error> {
+    let response = web.get(path).send()?;
+    let status = response.status();
+    // Reqwest follows redirects
+    assert!(status.is_success(), "failed to GET {}: {}", path, status);
+
+    let redirect_target = response.url().path();
+    assert!(redirect_target == expected_target,
+            "{}: expected redirect to {}, got redirect to {}",
+            path, expected_target, redirect_target);
+    Ok(())
+}
+
 pub(crate) struct TestEnvironment {
     db: OnceCell<TestDatabase>,
     frontend: OnceCell<TestFrontend>,

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>,
@@ -138,7 +138,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;";
@@ -178,6 +179,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

@@ -458,6 +458,7 @@ pub struct MetaData {
     pub description: Option<String>,
     pub target_name: Option<String>,
     pub rustdoc_status: bool,
+    pub default_target: String,
 }
 
 
@@ -467,7 +468,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",
@@ -480,6 +482,7 @@ impl MetaData {
                 description: row.get(2),
                 target_name: row.get(3),
                 rustdoc_status: row.get(4),
+                default_target: row.get(5),
             });
         }
 
@@ -496,6 +499,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()
     }
 }