fetch publisher data from crates.io, start build artifact caching

Author: syphar

docker-compose.yml

@@ -16,13 +16,15 @@ services:
             - "/var/run/docker.sock:/var/run/docker.sock"
             - ".rustwide-docker:/opt/docsrs/rustwide"
             - "cratesio-index:/opt/docsrs/prefix/crates.io-index"
+            - "artifact-cache:/opt/docsrs/prefix/artifact_cache"
         environment:
             DOCSRS_RUSTWIDE_WORKSPACE: /opt/docsrs/rustwide
             DOCSRS_DATABASE_URL: postgresql://cratesfyi:password@db
             DOCSRS_STORAGE_BACKEND: s3
             S3_ENDPOINT: http://s3:9000
             AWS_ACCESS_KEY_ID: cratesfyi
             AWS_SECRET_ACCESS_KEY: secret_key
+            DOCSRS_PREFIX: /opt/docsrs/prefix
         env_file:
             - .env
         healthcheck:
@@ -94,3 +96,4 @@ volumes:
     postgres-data: {}
     minio-data: {}
     cratesio-index: {}
+    artifact-cache: {}

src/db/add_package.rs

@@ -2,7 +2,7 @@ use crate::{
     db::types::Feature,
     docbuilder::{BuildResult, DocCoverage},
     error::Result,
-    index::api::{CrateData, CrateOwner, ReleaseData},
+    index::api::{CrateData, GithubUser, ReleaseData},
     storage::CompressionAlgorithm,
     utils::MetadataPackage,
     web::crate_details::CrateDetails,
@@ -371,7 +371,7 @@ pub fn update_crate_data_in_database(
 /// Adds owners into database
 fn update_owners_in_database(
     conn: &mut Client,
-    owners: &[CrateOwner],
+    owners: &[GithubUser],
     crate_id: i32,
 ) -> Result<()> {
     // Update any existing owner data since it is mutable and could have changed since last
@@ -562,9 +562,10 @@ mod test {
                 },
             )?;
 
-            let owner1 = CrateOwner {
+            let owner1 = GithubUser {
                 avatar: "avatar".into(),
                 login: "login".into(),
+                ..Default::default()
             };
 
             update_owners_in_database(&mut conn, &[owner1.clone()], crate_id)?;
@@ -600,16 +601,18 @@ mod test {
             // set initial owner details
             update_owners_in_database(
                 &mut conn,
-                &[CrateOwner {
+                &[GithubUser {
                     login: "login".into(),
                     avatar: "avatar".into(),
+                    ..Default::default()
                 }],
                 crate_id,
             )?;
 
-            let updated_owner = CrateOwner {
+            let updated_owner = GithubUser {
                 login: "login".into(),
                 avatar: "avatar2".into(),
+                ..Default::default()
             };
             update_owners_in_database(&mut conn, &[updated_owner.clone()], crate_id)?;
 
@@ -645,17 +648,19 @@ mod test {
             // set initial owner details
             update_owners_in_database(
                 &mut conn,
-                &[CrateOwner {
+                &[GithubUser {
                     login: "login".into(),
                     avatar: "avatar".into(),
+                    ..Default::default()
                 }],
                 crate_id,
             )?;
 
-            let new_owners: Vec<CrateOwner> = (1..5)
-                .map(|i| CrateOwner {
+            let new_owners: Vec<GithubUser> = (1..5)
+                .map(|i| GithubUser {
                     login: format!("login{i}"),
                     avatar: format!("avatar{i}"),
+                    ..Default::default()
                 })
                 .collect();
 

src/db/delete.rs

@@ -187,7 +187,7 @@ fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> R
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::index::api::CrateOwner;
+    use crate::index::api::GithubUser;
     use crate::test::{assert_success, wrapper};
     use postgres::Client;
     use test_case::test_case;
@@ -313,9 +313,10 @@ mod tests {
                 .name("a")
                 .version("1.0.0")
                 .archive_storage(archive_storage)
-                .add_owner(CrateOwner {
+                .add_owner(GithubUser {
                     login: "malicious actor".into(),
                     avatar: "https://example.org/malicious".into(),
+                    ..Default::default()
                 })
                 .create()?;
             assert!(release_exists(&mut db.conn(), v1)?);
@@ -342,9 +343,10 @@ mod tests {
                 .name("a")
                 .version("2.0.0")
                 .archive_storage(archive_storage)
-                .add_owner(CrateOwner {
+                .add_owner(GithubUser {
                     login: "Peter Rabbit".into(),
                     avatar: "https://example.org/peter".into(),
+                    ..Default::default()
                 })
                 .create()?;
             assert!(release_exists(&mut db.conn(), v2)?);

src/docbuilder/caching.rs

@@ -0,0 +1,112 @@
+use crate::utils::copy_dir_all;
+use anyhow::{Context as _, Result};
+use std::{
+    fs, io,
+    path::{Path, PathBuf},
+};
+use tracing::{debug, instrument, warn};
+
+/// move cache folder to target, falling back to copy + delete on error.
+fn move_or_copy<P: AsRef<Path> + std::fmt::Debug, Q: AsRef<Path> + std::fmt::Debug>(
+    source: P,
+    dest: Q,
+) -> io::Result<()> {
+    if let Some(parent) = dest.as_ref().parent() {
+        fs::create_dir_all(parent)?;
+    }
+    if let Err(err) = fs::rename(&source, &dest) {
+        warn!(
+            ?err,
+            ?source,
+            ?dest,
+            "could not move target directory, fall back to copy"
+        );
+        copy_dir_all(&source, &dest)?;
+        fs::remove_dir_all(&source)?;
+    }
+    Ok(())
+}
+
+/// artifact caching with cleanup
+#[derive(Debug)]
+pub(crate) struct ArtifactCache {
+    cache_dir: PathBuf,
+}
+
+impl ArtifactCache {
+    pub(crate) fn new(cache_dir: PathBuf) -> Result<Self> {
+        Ok(Self { cache_dir })
+    }
+
+    pub(crate) fn purge(&self) -> Result<()> {
+        fs::remove_dir_all(&self.cache_dir)?;
+        Ok(())
+    }
+
+    /// clean up a target directory.
+    ///
+    /// Should delete all things that shouldn't leak between
+    /// builds, so:
+    /// - doc-output
+    /// - ...?
+    #[instrument(skip(self))]
+    fn cleanup(&self, target_dir: &Path) -> Result<()> {
+        for item in fs::read_dir(target_dir)? {
+            // the first level of directories are the targets in most cases,
+            // delete their doc-directories
+            let item = item?;
+            let doc_dir = item.path().join("doc");
+            if doc_dir.is_dir() {
+                debug!(?doc_dir, "cache dir cleanup");
+                fs::remove_dir_all(doc_dir)?;
+            }
+        }
+        Ok(())
+    }
+
+    /// restore a cached target directory.
+    ///
+    /// Will just move the cache folder into the rustwide
+    /// target path. If that fails, will use `copy_dir_all`.
+    #[instrument(skip(self))]
+    pub(crate) fn restore_to<P: AsRef<Path> + std::fmt::Debug>(
+        &self,
+        cache_key: &str,
+        target_dir: P,
+    ) -> Result<()> {
+        let target_dir = target_dir.as_ref();
+        if target_dir.exists() {
+            // to be safe, while most of the time the dir doesn't exist,
+            // or is empty.
+            fs::remove_dir_all(target_dir).context("could not clean target directory")?;
+        }
+
+        let cache_dir = self.cache_dir.join(cache_key);
+        if !cache_dir.exists() {
+            // when there is no existing cache dir,
+            // we can just create an empty target.
+            fs::create_dir_all(target_dir).context("could not create target directory")?;
+            return Ok(());
+        }
+
+        move_or_copy(cache_dir, target_dir).context("could not move cache directory to target")?;
+        Ok(())
+    }
+
+    #[instrument(skip(self))]
+    pub(crate) fn save<P: AsRef<Path> + std::fmt::Debug>(
+        &self,
+        cache_key: &str,
+        target_dir: P,
+    ) -> Result<()> {
+        let cache_dir = self.cache_dir.join(cache_key);
+        if !cache_dir.exists() {
+            fs::create_dir_all(&cache_dir)?;
+        }
+
+        move_or_copy(&target_dir, &cache_dir)
+            .context("could not move target directory to cache")?;
+        self.cleanup(&cache_dir)?;
+        Ok(())
+    }
+}

src/docbuilder/mod.rs

@@ -1,3 +1,4 @@
+mod caching;
 mod crates;
 mod limits;
 mod rustwide_builder;

src/docbuilder/rustwide_builder.rs

@@ -1,3 +1,4 @@
+use super::caching::ArtifactCache;
 use crate::db::file::add_path_into_database;
 use crate::db::{
     add_build_into_database, add_doc_coverage, add_package_into_database,
@@ -27,12 +28,13 @@ use rustwide::{AlternativeRegistry, Build, Crate, Toolchain, Workspace, Workspac
 use std::collections::{HashMap, HashSet};
 use std::path::Path;
 use std::sync::Arc;
-use tracing::{debug, info, warn};
+use tracing::{debug, info, instrument, warn};
 
 const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
 const DUMMY_CRATE_NAME: &str = "empty-library";
 const DUMMY_CRATE_VERSION: &str = "1.0.0";
 
+#[derive(Debug)]
 pub enum PackageKind<'a> {
     Local(&'a Path),
     CratesIo,
@@ -47,6 +49,7 @@ pub struct RustwideBuilder {
     storage: Arc<Storage>,
     metrics: Arc<Metrics>,
     index: Arc<Index>,
+    artifact_cache: ArtifactCache,
     rustc_version: String,
     repository_stats_updater: Arc<RepositoryStatsUpdater>,
     skip_build_if_exists: bool,
@@ -89,6 +92,7 @@ impl RustwideBuilder {
         Ok(RustwideBuilder {
             workspace,
             toolchain,
+            artifact_cache: ArtifactCache::new(config.prefix.join("artifact_cache"))?,
             config,
             db: context.pool()?,
             storage: context.storage()?,
@@ -197,6 +201,7 @@ impl RustwideBuilder {
 
         let has_changed = old_version.as_deref() != Some(&self.rustc_version);
         if has_changed {
+            self.artifact_cache.purge()?;
             self.add_essential_files()?;
         }
         Ok(has_changed)
@@ -319,6 +324,7 @@ impl RustwideBuilder {
         self.build_package(&package.name, &package.version, PackageKind::Local(path))
     }
 
+    #[instrument(skip(self))]
     pub fn build_package(
         &mut self,
         name: &str,
@@ -383,6 +389,34 @@ impl RustwideBuilder {
                 (|| -> Result<bool> {
                     use docsrs_metadata::BuildTargets;
 
+                    let release_data = match self
+                        .index
+                        .api()
+                        .get_release_data(name, version)
+                        .context("error fetching release data from crates.io")
+                    {
+                        Ok(data) => data,
+                        Err(err) => {
+                            warn!("{:#?}", err);
+                            ReleaseData::default()
+                        }
+                    };
+
+                    if let Some(ref published_by) = release_data.published_by {
+                        info!(
+                            host_target_dir=?build.host_target_dir(),
+                            published_by_id=published_by.id,
+                            published_by_login=published_by.login,
+                            "restoring artifact cache",
+                        );
+                        if let Err(err) = self
+                            .artifact_cache
+                            .restore_to(&published_by.id.to_string(), build.host_target_dir())
+                        {
+                            warn!(?err, "could not restore artifact cache");
+                        }
+                    }
+
                     let mut has_docs = false;
                     let mut successful_targets = Vec::new();
                     let metadata = Metadata::from_crate_root(build.host_source_dir())?;
@@ -534,6 +568,22 @@ impl RustwideBuilder {
                         Err(err) => warn!("{:#?}", err),
                     }
 
+                    if let Some(ref published_by) = release_data.published_by {
+                        info!(
+                            host_target_dir=?build.host_target_dir(),
+                            published_by_id=published_by.id,
+                            published_by_login=published_by.login,
+                            "saving artifact cache",
+                        );
+                        if let Err(err) = self
+                            .artifact_cache
+                            .save(&published_by.id.to_string(), build.host_target_dir())
+                            .context("error giving back artifact cache")
+                        {
+                            warn!(?err, "could not give back artifact cache");
+                        };
+                    }
+
                     if res.result.successful {
                         // delete eventually existing files from pre-archive storage.
                         // we're doing this in the end so eventual problems in the build