docbuilder: switch to a more robust implementation of should_build

Author: pietroalbini

src/docbuilder/mod.rs

@@ -13,8 +13,6 @@ use crate::error::Result;
 use crate::index::Index;
 use crate::BuildQueue;
 use crate::DocBuilderOptions;
-use log::debug;
-use std::collections::BTreeSet;
 use std::fs;
 use std::path::PathBuf;
 use std::sync::Arc;
@@ -25,7 +23,6 @@ pub struct DocBuilder {
     index: Index,
     db: Pool,
     build_queue: Arc<BuildQueue>,
-    db_cache: BTreeSet<String>,
 }
 
 impl DocBuilder {
@@ -36,28 +33,9 @@ impl DocBuilder {
             options,
             index,
             db,
-            db_cache: BTreeSet::new(),
         }
     }
 
-    fn load_database_cache(&mut self) -> Result<()> {
-        debug!("Loading database cache");
-
-        let mut conn = self.db.get()?;
-        for row in &mut conn.query(
-            "SELECT name, version FROM crates, releases \
-             WHERE crates.id = releases.crate_id",
-            &[],
-        )? {
-            let name: String = row.get(0);
-            let version: String = row.get(1);
-
-            self.db_cache.insert(format!("{}-{}", name, version));
-        }
-
-        Ok(())
-    }
-
     fn lock_path(&self) -> PathBuf {
         self.options.prefix.join("cratesfyi.lock")
     }
@@ -91,10 +69,4 @@ impl DocBuilder {
     pub fn options(&self) -> &DocBuilderOptions {
         &self.options
     }
-
-    fn should_build(&mut self, name: &str, version: &str) -> Result<bool> {
-        self.load_database_cache()?;
-        Ok(!(self.options.skip_if_exists
-            && self.db_cache.contains(&format!("{}-{}", name, version))))
-    }
 }

src/docbuilder/rustwide_builder.rs

@@ -14,6 +14,7 @@ use crate::{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::{Command, SandboxBuilder};
 use rustwide::logging::{self, LogStorage};
 use rustwide::toolchain::ToolchainError;
@@ -297,16 +298,16 @@ impl RustwideBuilder {
         version: &str,
         local: Option<&Path>,
     ) -> Result<bool> {
-        if !doc_builder.should_build(name, version)? {
+        let mut conn = self.db.get()?;
+
+        if !self.should_build(&mut conn, &doc_builder, name, version)? {
             return Ok(false);
         }
 
         self.update_toolchain()?;
 
         info!("building package {} {}", name, version);
 
-        let mut conn = self.db.get()?;
-
         if is_blacklisted(&mut conn, name)? {
             info!("skipping build of {}, crate has been blacklisted", name);
             return Ok(false);
@@ -648,6 +649,29 @@ impl RustwideBuilder {
         )
         .map(|t| t.1)
     }
+
+    fn should_build(
+        &self,
+        conn: &mut Client,
+        docbuilder: &DocBuilder,
+        name: &str,
+        version: &str,
+    ) -> Result<bool> {
+        if docbuilder.options().skip_if_exists {
+            // Check whether no successful builds are present in the database.
+            Ok(conn
+                .query(
+                    "SELECT 1 FROM crates, releases, builds
+                     WHERE crates.id = releases.crate_id AND releases.id = builds.rid
+                       AND crates.name = $1 AND releases.version = $2
+                       AND builds.build_status = TRUE;",
+                    &[&name, &version],
+                )?
+                .is_empty())
+        } else {
+            Ok(true)
+        }
+    }
 }
 
 struct FullBuildResult {