Merge pull request #1340 from jyn514/shared-files

Upload exactly the shared files needed and give precedence to global files over local ones

Author: jyn514

src/docbuilder/rustwide_builder.rs

@@ -7,20 +7,20 @@ use crate::docbuilder::{crates::crates_from_path, Limits};
 use crate::error::Result;
 use crate::index::api::ReleaseData;
 use crate::storage::CompressionAlgorithms;
-use crate::utils::{copy_doc_dir, parse_rustc_version, CargoMetadata, GithubUpdater};
+use crate::utils::{copy_dir_all, parse_rustc_version, CargoMetadata, GithubUpdater};
 use crate::{db::blacklist::is_blacklisted, utils::MetadataPackage};
 use crate::{Config, Context, Index, Metrics, Storage};
 use docsrs_metadata::{Metadata, DEFAULT_TARGETS, HOST_TARGET};
 use failure::ResultExt;
 use log::{debug, info, warn, LevelFilter};
 use postgres::Client;
-use rustwide::cmd::{Binary, Command, SandboxBuilder, SandboxImage};
+use rustwide::cmd::{Command, SandboxBuilder, SandboxImage};
 use rustwide::logging::{self, LogStorage};
 use rustwide::toolchain::ToolchainError;
 use rustwide::{Build, Crate, Toolchain, Workspace, WorkspaceBuilder};
 use serde_json::Value;
 use std::collections::{HashMap, HashSet};
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::sync::Arc;
 
 const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
@@ -182,26 +182,12 @@ impl RustwideBuilder {
         let krate = Crate::crates_io(DUMMY_CRATE_NAME, DUMMY_CRATE_VERSION);
         krate.fetch(&self.workspace)?;
 
-        // TODO: remove this when https://github.com/rust-lang/rustwide/pull/53 lands.
-        struct Rustdoc<'a> {
-            toolchain_version: &'a str,
-        }
-        impl rustwide::cmd::Runnable for Rustdoc<'_> {
-            fn name(&self) -> Binary {
-                Binary::ManagedByRustwide(PathBuf::from("rustdoc"))
-            }
-
-            fn prepare_command<'w, 'pl>(&self, cmd: Command<'w, 'pl>) -> Command<'w, 'pl> {
-                cmd.args(&[format!("+{}", self.toolchain_version)])
-            }
-        }
-
         build_dir
             .build(&self.toolchain, &krate, self.prepare_sandbox(&limits))
             .run(|build| {
                 let metadata = Metadata::from_crate_root(&build.host_source_dir())?;
 
-                let res = self.execute_build(HOST_TARGET, true, build, &limits, &metadata)?;
+                let res = self.execute_build(HOST_TARGET, true, build, &limits, &metadata, true)?;
                 if !res.result.successful {
                     failure::bail!("failed to build dummy crate for {}", self.rustc_version);
                 }
@@ -211,39 +197,7 @@ impl RustwideBuilder {
                 let dest = tempfile::Builder::new()
                     .prefix("essential-files")
                     .tempdir()?;
-
-                let toolchain_version = self.toolchain.as_dist().unwrap().name();
-                let output = build.cmd(Rustdoc { toolchain_version })
-                    .args(&["-Zunstable-options", "--print=unversioned-files"])
-                    .run_capture()
-                    .context("failed to learn about unversioned files - make sure you have nightly-2021-03-07 or later")?;
-                let essential_files_unversioned = output
-                    .stdout_lines()
-                    .iter()
-                    .map(PathBuf::from);
-                let resource_suffix = format!("-{}", parse_rustc_version(&self.rustc_version)?);
-                let essential_files_versioned: Vec<_> = source.read_dir()?
-                    .collect::<std::result::Result<Vec<_>, _>>()?
-                    .into_iter()
-                    .filter_map(|entry| {
-                        entry.file_name().to_str().and_then(|name| if name.contains(&resource_suffix) {
-                            Some(entry.file_name().into())
-                        } else { None })
-                    })
-                    .collect();
-                for file_name in essential_files_unversioned.chain(essential_files_versioned) {
-                    let source_path = source.join(&file_name);
-                    let dest_path = dest.path().join(&file_name);
-                    debug!("copying {} to {}", source_path.display(), dest_path.display());
-                    ::std::fs::copy(&source_path, &dest_path).with_context(|_| {
-                        format!(
-                            "couldn't copy '{}' to '{}'",
-                            source_path.display(),
-                            dest_path.display()
-                        )
-                    })?;
-                }
-
+                copy_dir_all(source, &dest)?;
                 add_path_into_database(&self.storage, "", &dest)?;
                 conn.query(
                     "INSERT INTO config (name, value) VALUES ('rustc_version', $1) \
@@ -352,7 +306,8 @@ impl RustwideBuilder {
                 } = metadata.targets(self.config.include_default_targets);
 
                 // Perform an initial build
-                let res = self.execute_build(default_target, true, &build, &limits, &metadata)?;
+                let res =
+                    self.execute_build(default_target, true, &build, &limits, &metadata, false)?;
                 if res.result.successful {
                     if let Some(name) = res.cargo_metadata.root().library_name() {
                         let host_target = build.host_target_dir();
@@ -458,7 +413,7 @@ impl RustwideBuilder {
         successful_targets: &mut Vec<String>,
         metadata: &Metadata,
     ) -> Result<()> {
-        let target_res = self.execute_build(target, false, build, limits, metadata)?;
+        let target_res = self.execute_build(target, false, build, limits, metadata, false)?;
         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
@@ -534,12 +489,17 @@ impl RustwideBuilder {
         build: &Build,
         limits: &Limits,
         metadata: &Metadata,
+        create_essential_files: bool,
     ) -> Result<FullBuildResult> {
         let cargo_metadata =
             CargoMetadata::load(&self.workspace, &self.toolchain, &build.host_source_dir())?;
 
-        let mut rustdoc_flags = Vec::new();
-
+        let mut rustdoc_flags = vec![if create_essential_files {
+            "--emit=unversioned-shared-resources,toolchain-shared-resources"
+        } else {
+            "--emit=invocation-specific"
+        }
+        .to_string()];
         rustdoc_flags.extend(vec![
             "--resource-suffix".to_string(),
             format!("-{}", parse_rustc_version(&self.rustc_version)?),
@@ -656,7 +616,7 @@ impl RustwideBuilder {
         }
 
         info!("{} {}", source.display(), dest.display());
-        copy_doc_dir(source, dest)
+        copy_dir_all(source, dest).map_err(Into::into)
     }
 
     fn upload_docs(

src/utils/copy.rs

@@ -1,39 +1,18 @@
-use crate::error::Result;
 use std::fs;
+use std::io;
 use std::path::Path;
 
-use regex::Regex;
-
-/// Copies documentation from a crate's target directory to destination.
-///
-/// Target directory must have doc directory.
-///
-/// This function is designed to avoid file duplications.
-pub fn copy_doc_dir<P: AsRef<Path>, Q: AsRef<Path>>(source: P, destination: Q) -> Result<()> {
-    let destination = destination.as_ref();
-
-    // Make sure destination directory exists
-    if !destination.exists() {
-        fs::create_dir_all(destination)?;
-    }
-
-    // Avoid copying common files
-    let dup_regex = Regex::new(
-        r"(\.lock|\.txt|\.woff|\.svg|\.css|main-.*\.css|main-.*\.js|normalize-.*\.js|rustdoc-.*\.css|storage-.*\.js|theme-.*\.js)$")
-        .unwrap();
-
-    for file in source.as_ref().read_dir()? {
-        let file = file?;
-        let destination_full_path = destination.join(file.file_name());
-
-        let metadata = file.metadata()?;
-
-        if metadata.is_dir() {
-            copy_doc_dir(file.path(), destination_full_path)?
-        } else if dup_regex.is_match(&file.file_name().into_string().unwrap()[..]) {
-            continue;
+/// cp -r src dst
+pub(crate) fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
+    let dst = dst.as_ref();
+    fs::create_dir_all(dst)?;
+    for entry in fs::read_dir(src)? {
+        let entry = entry?;
+        let filename = entry.file_name();
+        if entry.file_type()?.is_dir() {
+            copy_dir_all(entry.path(), dst.join(filename))?;
         } else {
-            fs::copy(&file.path(), &destination_full_path)?;
+            fs::copy(entry.path(), dst.join(filename))?;
         }
     }
     Ok(())
@@ -59,21 +38,11 @@ mod test {
         fs::create_dir(doc.join("inner")).unwrap();
 
         fs::write(doc.join("index.html"), "<html>spooky</html>").unwrap();
-        fs::write(doc.join("index.txt"), "spooky").unwrap();
         fs::write(doc.join("inner").join("index.html"), "<html>spooky</html>").unwrap();
-        fs::write(doc.join("inner").join("index.txt"), "spooky").unwrap();
-        fs::write(doc.join("inner").join("important.svg"), "<svg></svg>").unwrap();
 
         // lets try to copy a src directory to tempdir
-        copy_doc_dir(source.path().join("doc"), destination.path()).unwrap();
+        copy_dir_all(source.path().join("doc"), destination.path()).unwrap();
         assert!(destination.path().join("index.html").exists());
-        assert!(!destination.path().join("index.txt").exists());
         assert!(destination.path().join("inner").join("index.html").exists());
-        assert!(!destination.path().join("inner").join("index.txt").exists());
-        assert!(!destination
-            .path()
-            .join("inner")
-            .join("important.svg")
-            .exists());
     }
 }

src/utils/mod.rs

@@ -1,7 +1,7 @@
 //! Various utilities for docs.rs
 
 pub(crate) use self::cargo_metadata::{CargoMetadata, Package as MetadataPackage};
-pub(crate) use self::copy::copy_doc_dir;
+pub(crate) use self::copy::copy_dir_all;
 pub use self::daemon::start_daemon;
 pub use self::github_updater::GithubUpdater;
 pub(crate) use self::html::rewrite_lol;

src/web/mod.rs

@@ -162,9 +162,9 @@ impl Handler for CratesfyiHandler {
             handle: impl FnOnce() -> IronResult<Response>,
         ) -> IronResult<Response> {
             if e.response.status == Some(status::NotFound) {
-                // the routes are ordered from most specific to least; give precedence to the
-                // original error message.
-                handle().or(Err(e))
+                // the routes are ordered from least specific to most; give precedence to the
+                // new error message.
+                handle()
             } else {
                 Err(e)
             }
@@ -181,11 +181,11 @@ impl Handler for CratesfyiHandler {
         // specific path means that buggy docs from 2018 will have missing CSS (#1181) so until
         // that's fixed, we need to keep the current (buggy) behavior.
         //
-        // It's important that `router_handler` comes first so that local rustdoc files take
-        // precedence over global ones (see #1324).
-        self.router_handler
+        // It's important that `shared_resource_handler` comes first so that global rustdoc files take
+        // precedence over local ones (see #1327).
+        self.shared_resource_handler
             .handle(req)
-            .or_else(|e| if_404(e, || self.shared_resource_handler.handle(req)))
+            .or_else(|e| if_404(e, || self.router_handler.handle(req)))
             .or_else(|e| {
                 let err = if let Some(err) = e.error.downcast_ref::<error::Nope>() {
                     *err