move builder temp-dir to prefix to avoid conflicts with second builder

Author: syphar

Cargo.toml

@@ -97,7 +97,6 @@ tower-http = { version = "0.4.0", features = ["fs", "trace", "timeout", "catch-p
 mime = "0.3.16"
 percent-encoding = "2.2.0"
 
-# NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.
 tempfile = "3.1.0"
 fn-error-context = "0.2.0"
 

src/bin/cratesfyi.rs

@@ -179,8 +179,9 @@ impl CommandLine {
                 start_background_metrics_webserver(Some(metric_server_socket_addr), &ctx)?;
 
                 let build_queue = ctx.build_queue()?;
+                let config = ctx.config()?;
                 let rustwide_builder = RustwideBuilder::init(&ctx)?;
-                queue_builder(rustwide_builder, build_queue)?;
+                queue_builder(rustwide_builder, build_queue, config)?;
             }
             Self::StartWebServer { socket_addr } => {
                 // Blocks indefinitely

src/config.rs

@@ -97,6 +97,7 @@ pub struct Config {
     // Build params
     pub(crate) build_attempts: u16,
     pub(crate) rustwide_workspace: PathBuf,
+    pub(crate) temp_dir: PathBuf,
     pub(crate) inside_docker: bool,
     pub(crate) docker_image: Option<String>,
     pub(crate) build_cpu_limit: Option<u32>,
@@ -127,6 +128,7 @@ impl Config {
         }
 
         let prefix: PathBuf = require_env("DOCSRS_PREFIX")?;
+        let temp_dir = prefix.join("tmp");
 
         Ok(Self {
             build_attempts: env("DOCSRS_BUILD_ATTEMPTS", 5)?,
@@ -196,6 +198,8 @@ impl Config {
                 prefix.join("archive_cache"),
             )?,
 
+            temp_dir,
+
             rustwide_workspace: env("DOCSRS_RUSTWIDE_WORKSPACE", PathBuf::from(".workspace"))?,
             inside_docker: env("DOCSRS_DOCKER", false)?,
             docker_image: maybe_env("DOCSRS_LOCAL_DOCKER_IMAGE")?

src/docbuilder/rustwide_builder.rs

@@ -8,8 +8,8 @@ use crate::error::Result;
 use crate::repositories::RepositoryStatsUpdater;
 use crate::storage::{rustdoc_archive_path, source_archive_path};
 use crate::utils::{
-    copy_dir_all, get_config, parse_rustc_version, queue_builder, report_error, set_config,
-    CargoMetadata, ConfigName,
+    copy_dir_all, get_config, parse_rustc_version, report_error, set_config, CargoMetadata,
+    ConfigName,
 };
 use crate::RUSTDOC_STATIC_STORAGE_PREFIX;
 use crate::{db::blacklist::is_blacklisted, utils::MetadataPackage};
@@ -24,6 +24,7 @@ use rustwide::logging::{self, LogStorage};
 use rustwide::toolchain::ToolchainError;
 use rustwide::{AlternativeRegistry, Build, Crate, Toolchain, Workspace, WorkspaceBuilder};
 use std::collections::{HashMap, HashSet};
+use std::fs;
 use std::path::Path;
 use std::sync::Arc;
 use tracing::{debug, info, warn};
@@ -381,9 +382,8 @@ impl RustwideBuilder {
         };
         krate.fetch(&self.workspace).map_err(FailureError::compat)?;
 
-        let local_storage = tempfile::Builder::new()
-            .prefix(queue_builder::TEMPDIR_PREFIX)
-            .tempdir()?;
+        fs::create_dir_all(&self.config.temp_dir)?;
+        let local_storage = tempfile::tempdir_in(&self.config.temp_dir)?;
 
         let successful = build_dir
             .build(&self.toolchain, &krate, self.prepare_sandbox(&limits))

src/utils/daemon.rs

@@ -134,11 +134,12 @@ pub fn start_daemon<C: Context + Send + Sync + 'static>(
 
     // build new crates every minute
     let build_queue = context.build_queue()?;
+    let config = context.config()?;
     let rustwide_builder = RustwideBuilder::init(&*context)?;
     thread::Builder::new()
         .name("build queue reader".to_string())
         .spawn(move || {
-            queue_builder(rustwide_builder, build_queue).unwrap();
+            queue_builder(rustwide_builder, build_queue, config).unwrap();
         })
         .unwrap();
 

src/utils/queue_builder.rs

@@ -1,20 +1,22 @@
-use crate::{docbuilder::RustwideBuilder, utils::report_error, BuildQueue};
+use crate::{docbuilder::RustwideBuilder, utils::report_error, BuildQueue, Config};
 use anyhow::{Context, Error};
 use std::panic::{catch_unwind, AssertUnwindSafe};
 use std::sync::Arc;
 use std::time::Duration;
-use std::{fs, io, thread};
+use std::{fs, io, path::Path, thread};
 use tracing::{debug, error, warn};
 
-pub(crate) const TEMPDIR_PREFIX: &str = "docsrs-docs";
-
 pub fn queue_builder(
     mut builder: RustwideBuilder,
     build_queue: Arc<BuildQueue>,
+    config: Arc<Config>,
 ) -> Result<(), Error> {
     loop {
-        if let Err(e) = remove_tempdirs() {
-            report_error(&anyhow::anyhow!(e).context("failed to remove temporary directories"));
+        if let Err(e) = remove_tempdirs(&config.temp_dir) {
+            report_error(&anyhow::anyhow!(e).context(format!(
+                "failed to clean temporary directory {:?}",
+                &config.temp_dir
+            )));
         }
 
         // check lock file
@@ -58,57 +60,8 @@ pub fn queue_builder(
 /// Sometimes, when the server hits a hard crash or a build thread panics,
 /// rustwide_builder won't actually remove the temporary directories it creates.
 /// Remove them now to avoid running out of disk space.
-fn remove_tempdirs() -> Result<(), io::Error> {
-    // NOTE: hardcodes that `tempfile::tempdir()` uses `std::env::temp_dir`.
-    for entry in std::fs::read_dir(std::env::temp_dir())? {
-        let entry = entry?;
-        if !entry.metadata()?.is_dir() {
-            continue;
-        }
-
-        if let Some(dir_name) = entry.path().file_name() {
-            if dir_name.to_string_lossy().starts_with(TEMPDIR_PREFIX) {
-                fs::remove_dir_all(entry.path())?;
-            }
-        }
-    }
-
+fn remove_tempdirs<P: AsRef<Path>>(path: P) -> Result<(), io::Error> {
+    fs::remove_dir_all(&path)?;
+    fs::create_dir_all(&path)?;
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn remove_existing_tempdirs() {
-        let file_with_prefix = tempfile::Builder::new()
-            .prefix(TEMPDIR_PREFIX)
-            .tempfile()
-            .unwrap();
-
-        let dir_with_prefix = tempfile::Builder::new()
-            .prefix(TEMPDIR_PREFIX)
-            .tempdir()
-            .unwrap();
-
-        let file_inside = dir_with_prefix.path().join("some_file_name");
-        fs::File::create(&file_inside).unwrap();
-
-        let other_file = tempfile::Builder::new().tempfile().unwrap();
-
-        let other_dir = tempfile::Builder::new().tempdir().unwrap();
-
-        assert!(dir_with_prefix.path().exists());
-
-        remove_tempdirs().unwrap();
-
-        assert!(!dir_with_prefix.path().exists());
-        assert!(!file_inside.exists());
-
-        // all these still exist
-        assert!(file_with_prefix.path().exists());
-        assert!(other_file.path().exists());
-        assert!(other_dir.path().exists());
-    }
-}