Unify DocBuilder and BuildQueue

There was no reason to have these be separate. This also gets rid of
some extra open database connections.

Author: jyn514

src/bin/cratesfyi.rs

@@ -8,8 +8,7 @@ use docs_rs::db::{self, add_path_into_database, Pool, PoolClient};
 use docs_rs::repositories::RepositoryStatsUpdater;
 use docs_rs::utils::{remove_crate_priority, set_crate_priority};
 use docs_rs::{
-    BuildQueue, Config, Context, DocBuilder, Index, Metrics, PackageKind, RustwideBuilder, Server,
-    Storage,
+    BuildQueue, Config, Context, Index, Metrics, PackageKind, RustwideBuilder, Server, Storage,
 };
 use once_cell::sync::OnceCell;
 use structopt::StructOpt;
@@ -289,7 +288,7 @@ enum BuildSubcommand {
 
 impl BuildSubcommand {
     pub fn handle_args(self, ctx: BinContext, skip_if_exists: bool) -> Result<()> {
-        let docbuilder = DocBuilder::new(ctx.build_queue()?);
+        let build_queue = ctx.build_queue()?;
 
         let rustwide_builder = || -> Result<RustwideBuilder> {
             let mut builder = RustwideBuilder::init(&ctx)?;
@@ -358,8 +357,8 @@ impl BuildSubcommand {
                     .context("failed to add essential files")?;
             }
 
-            Self::Lock => docbuilder.lock().context("Failed to lock")?,
-            Self::Unlock => docbuilder.unlock().context("Failed to unlock")?,
+            Self::Lock => build_queue.lock().context("Failed to lock")?,
+            Self::Unlock => build_queue.unlock().context("Failed to unlock")?,
         }
 
         Ok(())

src/build_queue.rs

@@ -1,7 +1,13 @@
 use crate::db::Pool;
+use crate::docbuilder::PackageKind;
 use crate::error::Result;
-use crate::{Config, Metrics};
-use log::error;
+use crate::utils::get_crate_priority;
+use crate::{Config, Index, Metrics, RustwideBuilder};
+
+use crates_index_diff::ChangeKind;
+use log::{debug, error};
+
+use std::fs;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -33,15 +39,6 @@ impl BuildQueue {
         }
     }
 
-    pub(crate) fn lock_path(&self) -> PathBuf {
-        self.config.prefix.join("docsrs.lock")
-    }
-
-    /// Checks for the lock file and returns whether it currently exists.
-    pub fn is_locked(&self) -> bool {
-        self.lock_path().exists()
-    }
-
     pub fn add_crate(
         &self,
         name: &str,
@@ -145,6 +142,130 @@ impl BuildQueue {
     }
 }
 
+/// Locking functions.
+impl BuildQueue {
+    pub(crate) fn lock_path(&self) -> PathBuf {
+        self.config.prefix.join("docsrs.lock")
+    }
+
+    /// Checks for the lock file and returns whether it currently exists.
+    pub fn is_locked(&self) -> bool {
+        self.lock_path().exists()
+    }
+
+    /// Creates a lock file. Daemon will check this lock file and stop operating if its exists.
+    pub fn lock(&self) -> Result<()> {
+        let path = self.lock_path();
+        if !path.exists() {
+            fs::OpenOptions::new().write(true).create(true).open(path)?;
+        }
+
+        Ok(())
+    }
+
+    /// Removes lock file.
+    pub fn unlock(&self) -> Result<()> {
+        let path = self.lock_path();
+        if path.exists() {
+            fs::remove_file(path)?;
+        }
+
+        Ok(())
+    }
+}
+
+/// Index methods.
+impl BuildQueue {
+    /// Updates registry index repository and adds new crates into build queue.
+    ///
+    /// Returns the number of crates added
+    pub fn get_new_crates(&self, index: &Index) -> Result<usize> {
+        let mut conn = self.db.get()?;
+        let diff = index.diff()?;
+        let (mut changes, oid) = diff.peek_changes()?;
+        let mut crates_added = 0;
+
+        // I believe this will fix ordering of queue if we get more than one crate from changes
+        changes.reverse();
+
+        for krate in &changes {
+            match krate.kind {
+                ChangeKind::Yanked => {
+                    let res = conn.execute(
+                        "
+                        UPDATE releases
+                            SET yanked = TRUE
+                        FROM crates
+                        WHERE crates.id = releases.crate_id
+                            AND name = $1
+                            AND version = $2
+                        ",
+                        &[&krate.name, &krate.version],
+                    );
+                    match res {
+                        Ok(_) => debug!("{}-{} yanked", krate.name, krate.version),
+                        Err(err) => error!(
+                            "error while setting {}-{} to yanked: {}",
+                            krate.name, krate.version, err
+                        ),
+                    }
+                }
+
+                ChangeKind::Added => {
+                    let priority = get_crate_priority(&mut conn, &krate.name)?;
+
+                    match self.add_crate(
+                        &krate.name,
+                        &krate.version,
+                        priority,
+                        index.repository_url(),
+                    ) {
+                        Ok(()) => {
+                            debug!("{}-{} added into build queue", krate.name, krate.version);
+                            crates_added += 1;
+                        }
+                        Err(err) => error!(
+                            "failed adding {}-{} into build queue: {}",
+                            krate.name, krate.version, err
+                        ),
+                    }
+                }
+            }
+        }
+
+        diff.set_last_seen_reference(oid)?;
+
+        Ok(crates_added)
+    }
+
+    /// Builds the top package from the queue. Returns whether there was a package in the queue.
+    ///
+    /// Note that this will return `Ok(true)` even if the package failed to build.
+    pub(crate) fn build_next_queue_package(&self, builder: &mut RustwideBuilder) -> Result<bool> {
+        let mut processed = false;
+        self.process_next_crate(|krate| {
+            processed = true;
+
+            let kind = krate
+                .registry
+                .as_ref()
+                .map(|r| PackageKind::Registry(r.as_str()))
+                .unwrap_or(PackageKind::CratesIo);
+
+            if let Err(err) = builder.update_toolchain() {
+                log::error!("Updating toolchain failed, locking queue: {}", err);
+                self.lock()?;
+                return Err(err);
+            }
+
+            builder.build_package(&krate.name, &krate.version, kind)?;
+            Ok(())
+        })?;
+
+        Ok(processed)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/docbuilder/mod.rs

@@ -1,44 +1,7 @@
 mod crates;
 mod limits;
-mod queue;
 mod rustwide_builder;
 
 pub(crate) use self::limits::Limits;
 pub(crate) use self::rustwide_builder::{BuildResult, DocCoverage};
 pub use self::rustwide_builder::{PackageKind, RustwideBuilder};
-
-use crate::error::Result;
-use crate::BuildQueue;
-use std::fs;
-use std::sync::Arc;
-
-/// chroot based documentation builder
-pub struct DocBuilder {
-    pub(crate) build_queue: Arc<BuildQueue>,
-}
-
-impl DocBuilder {
-    pub fn new(build_queue: Arc<BuildQueue>) -> DocBuilder {
-        DocBuilder { build_queue }
-    }
-
-    /// Creates a lock file. Daemon will check this lock file and stop operating if its exists.
-    pub fn lock(&self) -> Result<()> {
-        let path = self.build_queue.lock_path();
-        if !path.exists() {
-            fs::OpenOptions::new().write(true).create(true).open(path)?;
-        }
-
-        Ok(())
-    }
-
-    /// Removes lock file.
-    pub fn unlock(&self) -> Result<()> {
-        let path = self.build_queue.lock_path();
-        if path.exists() {
-            fs::remove_file(path)?;
-        }
-
-        Ok(())
-    }
-}

src/docbuilder/queue.rs

@@ -5,7 +5,6 @@
 pub use self::build_queue::BuildQueue;
 pub use self::config::Config;
 pub use self::context::Context;
-pub use self::docbuilder::DocBuilder;
 pub use self::docbuilder::PackageKind;
 pub use self::docbuilder::RustwideBuilder;
 pub use self::index::Index;

src/lib.rs

@@ -187,7 +187,7 @@ impl TestEnvironment {
                 Arc::new(BuildQueue::new(
                     self.db().pool(),
                     self.metrics(),
-                    &self.config(),
+                    self.config(),
                 ))
             })
             .clone()

src/test/mod.rs

@@ -2,7 +2,7 @@
 //!
 //! This daemon will start web server, track new packages and build them
 
-use crate::{utils::queue_builder, Context, DocBuilder, RustwideBuilder};
+use crate::{utils::queue_builder, Context, RustwideBuilder};
 use anyhow::Error;
 use log::{debug, error, info};
 use std::thread;
@@ -21,13 +21,11 @@ fn start_registry_watcher(context: &dyn Context) -> Result<(), Error> {
 
             let mut last_gc = Instant::now();
             loop {
-                let mut doc_builder = DocBuilder::new(build_queue.clone());
-
-                if doc_builder.build_queue.is_locked() {
+                if build_queue.is_locked() {
                     debug!("Lock file exists, skipping checking new crates");
                 } else {
                     debug!("Checking new crates");
-                    match doc_builder.get_new_crates(&index) {
+                    match build_queue.get_new_crates(&index) {
                         Ok(n) => debug!("{} crates added to queue", n),
                         Err(e) => error!("Failed to get new crates: {}", e),
                     }
@@ -62,8 +60,7 @@ pub fn start_daemon(context: &dyn Context, enable_registry_watcher: bool) -> Res
     thread::Builder::new()
         .name("build queue reader".to_string())
         .spawn(move || {
-            let doc_builder = DocBuilder::new(build_queue.clone());
-            queue_builder(doc_builder, rustwide_builder, build_queue).unwrap();
+            queue_builder(rustwide_builder, build_queue).unwrap();
         })
         .unwrap();