context: move Index to the context

Author: pietroalbini

src/bin/cratesfyi.rs

@@ -4,10 +4,9 @@ use std::path::PathBuf;
 use std::sync::Arc;
 
 use cratesfyi::db::{self, add_path_into_database, Pool, PoolClient};
-use cratesfyi::index::Index;
 use cratesfyi::utils::{remove_crate_priority, set_crate_priority};
 use cratesfyi::{
-    BuildQueue, Config, Context, DocBuilder, Metrics, RustwideBuilder, Server, Storage,
+    BuildQueue, Config, Context, DocBuilder, Index, Metrics, RustwideBuilder, Server, Storage,
 };
 use failure::{err_msg, Error, ResultExt};
 use once_cell::sync::OnceCell;
@@ -282,19 +281,18 @@ enum BuildSubcommand {
 
 impl BuildSubcommand {
     pub fn handle_args(self, ctx: BinContext, skip_if_exists: bool) -> Result<(), Error> {
-        let mut docbuilder = DocBuilder::new(ctx.config()?, ctx.pool()?, ctx.build_queue()?);
+        let docbuilder = DocBuilder::new(ctx.config()?, ctx.pool()?, ctx.build_queue()?);
 
         let rustwide_builder = || -> Result<RustwideBuilder, Error> {
-            let mut builder =
-                RustwideBuilder::init(ctx.config()?, ctx.pool()?, ctx.metrics()?, ctx.storage()?)?;
+            let mut builder = RustwideBuilder::init(&ctx)?;
             builder.set_skip_build_if_exists(skip_if_exists);
             Ok(builder)
         };
 
         match self {
             Self::World => {
                 rustwide_builder()?
-                    .build_world(&mut docbuilder)
+                    .build_world()
                     .context("Failed to build world")?;
             }
 
@@ -307,12 +305,11 @@ impl BuildSubcommand {
 
                 if let Some(path) = local {
                     builder
-                        .build_local_package(&mut docbuilder, &path)
+                        .build_local_package(&path)
                         .context("Building documentation failed")?;
                 } else {
                     builder
                         .build_package(
-                            &mut docbuilder,
                             &crate_name.ok_or_else(|| err_msg("must specify name if not local"))?,
                             &crate_version
                                 .ok_or_else(|| err_msg("must specify version if not local"))?,
@@ -420,7 +417,7 @@ impl DatabaseSubcommand {
             }
 
             Self::UpdateCrateRegistryFields { name } => {
-                let index = Index::new(&ctx.config()?.registry_index_path)?;
+                let index = ctx.index()?;
 
                 db::update_crate_data_in_database(
                     &mut *ctx.conn()?,
@@ -452,8 +449,8 @@ impl DatabaseSubcommand {
 
             Self::Synchronize { dry_run } => {
                 cratesfyi::utils::consistency::run_check(
-                    &*ctx.config()?,
                     &mut *ctx.conn()?,
+                    &*ctx.index()?,
                     dry_run,
                 )?;
             }
@@ -529,6 +526,7 @@ struct BinContext {
     config: OnceCell<Arc<Config>>,
     pool: OnceCell<Pool>,
     metrics: OnceCell<Arc<Metrics>>,
+    index: OnceCell<Arc<Index>>,
 }
 
 impl BinContext {
@@ -539,6 +537,7 @@ impl BinContext {
             config: OnceCell::new(),
             pool: OnceCell::new(),
             metrics: OnceCell::new(),
+            index: OnceCell::new(),
         }
     }
 
@@ -594,4 +593,11 @@ impl Context for BinContext {
             .get_or_try_init::<_, Error>(|| Ok(Arc::new(Metrics::new()?)))?
             .clone())
     }
+
+    fn index(&self) -> Result<Arc<Index>, Error> {
+        Ok(self
+            .index
+            .get_or_try_init::<_, Error>(|| Ok(Arc::new(Index::new(&*self.config()?)?)))?
+            .clone())
+    }
 }

src/context.rs

@@ -1,5 +1,5 @@
 use crate::db::Pool;
-use crate::{BuildQueue, Config, Metrics, Storage};
+use crate::{BuildQueue, Config, Index, Metrics, Storage};
 use failure::Error;
 use std::sync::Arc;
 
@@ -9,4 +9,5 @@ pub trait Context {
     fn storage(&self) -> Result<Arc<Storage>, Error>;
     fn pool(&self) -> Result<Pool, Error>;
     fn metrics(&self) -> Result<Arc<Metrics>, Error>;
+    fn index(&self) -> Result<Arc<Index>, Error>;
 }

src/docbuilder/mod.rs

@@ -9,7 +9,6 @@ pub(crate) use self::rustwide_builder::{BuildResult, DocCoverage};
 
 use crate::db::Pool;
 use crate::error::Result;
-use crate::index::Index;
 use crate::{BuildQueue, Config};
 use std::fs;
 use std::path::PathBuf;
@@ -18,18 +17,15 @@ use std::sync::Arc;
 /// chroot based documentation builder
 pub struct DocBuilder {
     config: Arc<Config>,
-    index: Index,
     db: Pool,
     build_queue: Arc<BuildQueue>,
 }
 
 impl DocBuilder {
     pub fn new(config: Arc<Config>, db: Pool, build_queue: Arc<BuildQueue>) -> DocBuilder {
-        let index = Index::new(&config.registry_index_path).expect("valid index");
         DocBuilder {
             config,
             build_queue,
-            index,
             db,
         }
     }

src/docbuilder/queue.rs

@@ -3,15 +3,16 @@
 use super::{DocBuilder, RustwideBuilder};
 use crate::error::Result;
 use crate::utils::get_crate_priority;
+use crate::Index;
 use crates_index_diff::ChangeKind;
 use log::{debug, error};
 
 impl DocBuilder {
     /// Updates registry index repository and adds new crates into build queue.
     /// Returns the number of crates added
-    pub fn get_new_crates(&mut self) -> Result<usize> {
+    pub fn get_new_crates(&mut self, index: &Index) -> Result<usize> {
         let mut conn = self.db.get()?;
-        let diff = self.index.diff()?;
+        let diff = index.diff()?;
         let (mut changes, oid) = diff.peek_changes()?;
         let mut crates_added = 0;
 
@@ -78,14 +79,10 @@ impl DocBuilder {
         queue.process_next_crate(|krate| {
             processed = true;
 
-            builder.build_package(self, &krate.name, &krate.version, None)?;
+            builder.build_package(&krate.name, &krate.version, None)?;
             Ok(())
         })?;
 
         Ok(processed)
     }
-
-    pub fn run_git_gc(&self) {
-        self.index.run_git_gc();
-    }
 }

src/docbuilder/rustwide_builder.rs

@@ -1,4 +1,3 @@
-use super::DocBuilder;
 use crate::db::blacklist::is_blacklisted;
 use crate::db::file::add_path_into_database;
 use crate::db::{
@@ -10,7 +9,7 @@ use crate::error::Result;
 use crate::index::api::ReleaseData;
 use crate::storage::CompressionAlgorithms;
 use crate::utils::{copy_doc_dir, parse_rustc_version, CargoMetadata};
-use crate::{Config, Metrics, Storage};
+use crate::{Config, Context, Index, Metrics, Storage};
 use docsrs_metadata::{Metadata, DEFAULT_TARGETS, HOST_TARGET};
 use failure::ResultExt;
 use log::{debug, info, warn, LevelFilter};
@@ -63,17 +62,15 @@ pub struct RustwideBuilder {
     db: Pool,
     storage: Arc<Storage>,
     metrics: Arc<Metrics>,
+    index: Arc<Index>,
     rustc_version: String,
     skip_build_if_exists: bool,
 }
 
 impl RustwideBuilder {
-    pub fn init(
-        config: Arc<Config>,
-        db: Pool,
-        metrics: Arc<Metrics>,
-        storage: Arc<Storage>,
-    ) -> Result<Self> {
+    pub fn init(context: &dyn Context) -> Result<Self> {
+        let config = context.config()?;
+
         let mut builder = WorkspaceBuilder::new(&config.rustwide_workspace, USER_AGENT)
             .running_inside_docker(config.inside_docker);
         if let Some(custom_image) = &config.local_docker_image {
@@ -89,9 +86,10 @@ impl RustwideBuilder {
             workspace,
             toolchain,
             config,
-            db,
-            storage,
-            metrics,
+            db: context.pool()?,
+            storage: context.storage()?,
+            metrics: context.metrics()?,
+            index: context.index()?,
             rustc_version: String::new(),
             skip_build_if_exists: false,
         })
@@ -256,34 +254,29 @@ impl RustwideBuilder {
         Ok(())
     }
 
-    pub fn build_world(&mut self, doc_builder: &mut DocBuilder) -> Result<()> {
+    pub fn build_world(&mut self) -> Result<()> {
         crates_from_path(
             &self.config.registry_index_path.clone(),
             &mut |name, version| {
-                if let Err(err) = self.build_package(doc_builder, name, version, None) {
+                if let Err(err) = self.build_package(name, version, None) {
                     warn!("failed to build package {} {}: {}", name, version, err);
                 }
             },
         )
     }
 
-    pub fn build_local_package(
-        &mut self,
-        doc_builder: &mut DocBuilder,
-        path: &Path,
-    ) -> Result<bool> {
+    pub fn build_local_package(&mut self, path: &Path) -> Result<bool> {
         self.update_toolchain()?;
         let metadata =
             CargoMetadata::load(&self.workspace, &self.toolchain, path).map_err(|err| {
                 err.context(format!("failed to load local package {}", path.display()))
             })?;
         let package = metadata.root();
-        self.build_package(doc_builder, &package.name, &package.version, Some(path))
+        self.build_package(&package.name, &package.version, Some(path))
     }
 
     pub fn build_package(
         &mut self,
-        doc_builder: &mut DocBuilder,
         name: &str,
         version: &str,
         local: Option<&Path>,
@@ -379,7 +372,7 @@ impl RustwideBuilder {
                     self.metrics.non_library_builds.inc();
                 }
 
-                let release_data = match doc_builder.index.api().get_release_data(name, version) {
+                let release_data = match self.index.api().get_release_data(name, version) {
                     Ok(data) => data,
                     Err(err) => {
                         warn!("{:#?}", err);
@@ -408,7 +401,7 @@ impl RustwideBuilder {
                 add_build_into_database(&mut conn, release_id, &res.result)?;
 
                 // Some crates.io crate data is mutable, so we proactively update it during a release
-                match doc_builder.index.api().get_crate_data(name) {
+                match self.index.api().get_crate_data(name) {
                     Ok(crate_data) => update_crate_data_in_database(&mut conn, name, &crate_data)?,
                     Err(err) => warn!("{:#?}", err),
                 }

src/index/mod.rs

@@ -1,12 +1,9 @@
-use std::{
-    path::{Path, PathBuf},
-    process::Command,
-};
+use std::{path::PathBuf, process::Command};
 
 use url::Url;
 
 use self::{api::Api, crates::Crates};
-use crate::error::Result;
+use crate::{error::Result, Config};
 use failure::ResultExt;
 
 pub(crate) mod api;
@@ -43,8 +40,8 @@ fn load_config(repo: &git2::Repository) -> Result<IndexConfig> {
 }
 
 impl Index {
-    pub fn new(path: impl AsRef<Path>) -> Result<Self> {
-        let path = path.as_ref().to_owned();
+    pub fn new(app_config: &Config) -> Result<Self> {
+        let path = app_config.registry_index_path.clone();
         // This initializes the repository, then closes it afterwards to avoid leaking file descriptors.
         // See https://github.com/rust-lang/docs.rs/pull/847
         let diff = crates_index_diff::Index::from_path_or_cloned(&path)
@@ -91,9 +88,3 @@ impl Index {
         }
     }
 }
-
-impl Clone for Index {
-    fn clone(&self) -> Self {
-        Self::new(&self.path).expect("we already loaded this registry successfully once")
-    }
-}

src/lib.rs

@@ -7,6 +7,7 @@ pub use self::config::Config;
 pub use self::context::Context;
 pub use self::docbuilder::DocBuilder;
 pub use self::docbuilder::RustwideBuilder;
+pub use self::index::Index;
 pub use self::metrics::Metrics;
 pub use self::storage::Storage;
 pub use self::web::Server;

src/test/mod.rs

@@ -3,7 +3,7 @@ mod fakes;
 use crate::db::{Pool, PoolClient};
 use crate::storage::{Storage, StorageKind};
 use crate::web::Server;
-use crate::{BuildQueue, Config, Context, Metrics};
+use crate::{BuildQueue, Config, Context, Index, Metrics};
 use failure::Error;
 use log::error;
 use once_cell::unsync::OnceCell;
@@ -96,6 +96,7 @@ pub(crate) struct TestEnvironment {
     config: OnceCell<Arc<Config>>,
     db: OnceCell<TestDatabase>,
     storage: OnceCell<Arc<Storage>>,
+    index: OnceCell<Arc<Index>>,
     metrics: OnceCell<Arc<Metrics>>,
     frontend: OnceCell<TestFrontend>,
 }
@@ -115,6 +116,7 @@ impl TestEnvironment {
             config: OnceCell::new(),
             db: OnceCell::new(),
             storage: OnceCell::new(),
+            index: OnceCell::new(),
             metrics: OnceCell::new(),
             frontend: OnceCell::new(),
         }
@@ -192,6 +194,14 @@ impl TestEnvironment {
             .clone()
     }
 
+    pub(crate) fn index(&self) -> Arc<Index> {
+        self.index
+            .get_or_init(|| {
+                Arc::new(Index::new(&*self.config()).expect("failed to initialize the index"))
+            })
+            .clone()
+    }
+
     pub(crate) fn db(&self) -> &TestDatabase {
         self.db.get_or_init(|| {
             TestDatabase::new(&self.config(), self.metrics()).expect("failed to initialize the db")
@@ -227,6 +237,10 @@ impl Context for TestEnvironment {
     fn metrics(&self) -> Result<Arc<Metrics>, Error> {
         Ok(self.metrics())
     }
+
+    fn index(&self) -> Result<Arc<Index>, Error> {
+        Ok(self.index())
+    }
 }
 
 pub(crate) struct TestDatabase {

src/utils/consistency/index.rs

@@ -1,9 +1,7 @@
 use super::data::{Crate, CrateName, Data, Release, Version};
-use crate::{config::Config, index::Index};
-
-pub(crate) fn load(config: &Config) -> Result<Data, failure::Error> {
-    let index = Index::new(&config.registry_index_path)?;
+use crate::Index;
 
+pub(crate) fn load(index: &Index) -> Result<Data, failure::Error> {
     let mut data = Data::default();
 
     index.crates()?.walk(|krate| {