From dfa54cb44a52bde20196b148d3ef07211753bf59 Mon Sep 17 00:00:00 2001 From: Laurent Wouters Date: Wed, 25 Sep 2024 14:42:26 +0200 Subject: [PATCH] fix: missing git user info after first clone (#33) On a first rust, when a git repo for the index is not found, but a remote one was specified and successfully cloned, the git username and email configuration was not written to disk and was therefore missing afterward. This ensures that the git username and email is also correctly configured in this scenario. Additionally, this catches an error scenario where we expected to successfully clone an index because the database contains packages but the clone failed and an empty index was created instead. This resulted in inconsistencies between the index and the database content. --- ...d7580b55e19d1e353484834c5df6f012b7a26.json | 20 +++++++ src/application.rs | 4 +- src/services/database/packages.rs | 8 +++ src/services/index/git.rs | 53 ++++++++++++++----- src/services/index/mod.rs | 4 +- src/services/mod.rs | 6 +-- src/tests/mocks.rs | 2 +- 7 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 .sqlx/query-aabde1059bb3bdd416e13e2bd84d7580b55e19d1e353484834c5df6f012b7a26.json diff --git a/.sqlx/query-aabde1059bb3bdd416e13e2bd84d7580b55e19d1e353484834c5df6f012b7a26.json b/.sqlx/query-aabde1059bb3bdd416e13e2bd84d7580b55e19d1e353484834c5df6f012b7a26.json new file mode 100644 index 0000000..d8e56b2 --- /dev/null +++ b/.sqlx/query-aabde1059bb3bdd416e13e2bd84d7580b55e19d1e353484834c5df6f012b7a26.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "SELECT id FROM PackageVersion LIMIT 1", + "describe": { + "columns": [ + { + "name": "id", + "ordinal": 0, + "type_info": "Integer" + } + ], + "parameters": { + "Right": 0 + }, + "nullable": [ + false + ] + }, + "hash": "aabde1059bb3bdd416e13e2bd84d7580b55e19d1e353484834c5df6f012b7a26" +} diff --git a/src/application.rs b/src/application.rs index 93c59b6..45d54e3 100644 --- a/src/application.rs +++ b/src/application.rs @@ -80,8 +80,10 @@ impl Application { }) .await?; + let db_is_empty = + db_transaction_read(&service_db_pool, |database| async move { database.get_is_empty().await }).await?; let service_storage = P::get_storage(&configuration.deref().clone()); - let service_index = P::get_index(&configuration).await?; + let service_index = P::get_index(&configuration, db_is_empty).await?; let service_rustsec = P::get_rustsec(&configuration); let service_deps_checker = P::get_deps_checker(configuration.clone(), service_index.clone(), service_rustsec.clone()); let service_email_sender = P::get_email_sender(configuration.clone()); diff --git a/src/services/database/packages.rs b/src/services/database/packages.rs index 1d08e3c..cde9ee3 100644 --- a/src/services/database/packages.rs +++ b/src/services/database/packages.rs @@ -73,6 +73,14 @@ impl Database { }) } + /// Gets whether the database does not contain any package at all + pub async fn get_is_empty(&self) -> Result { + Ok(sqlx::query!("SELECT id FROM PackageVersion LIMIT 1") + .fetch_optional(&mut *self.transaction.borrow().await) + .await? + .is_none()) + } + /// Gets the last version number for a package pub async fn get_crate_last_version(&self, package: &str) -> Result { let row = sqlx::query!( diff --git a/src/services/index/git.rs b/src/services/index/git.rs index b0b93d5..2509ec0 100644 --- a/src/services/index/git.rs +++ b/src/services/index/git.rs @@ -6,7 +6,7 @@ use std::path::{Path, PathBuf}; -use log::info; +use log::{error, info}; use tokio::fs::{create_dir_all, File, OpenOptions}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::sync::Mutex; @@ -24,8 +24,8 @@ pub struct GitIndex { impl GitIndex { /// When the application is launched - pub async fn new(config: IndexConfig) -> Result { - let inner = GitIndexImpl::new(config).await?; + pub async fn new(config: IndexConfig, expect_empty: bool) -> Result { + let inner = GitIndexImpl::new(config, expect_empty).await?; Ok(Self { inner: Mutex::new(inner), }) @@ -62,7 +62,7 @@ struct GitIndexImpl { impl GitIndexImpl { /// When the application is launched - async fn new(config: IndexConfig) -> Result { + async fn new(config: IndexConfig, expect_empty: bool) -> Result { let index = Self { config }; // check for the SSH key @@ -88,7 +88,7 @@ impl GitIndexImpl { if content.next_entry().await?.is_none() { // the folder is empty info!("index: initializing on empty index"); - index.initialize_index(location).await?; + index.initialize_on_empty(location, expect_empty).await?; } else if index.config.remote_origin.is_some() { // attempt to pull changes info!("index: pulling changes from origin"); @@ -97,29 +97,47 @@ impl GitIndexImpl { Ok(index) } - /// Initializes the index at the specified location - async fn initialize_index(&self, location: PathBuf) -> Result<(), ApiError> { + /// Initializes the index at the specified location when found empty + async fn initialize_on_empty(&self, location: PathBuf, expect_empty: bool) -> Result<(), ApiError> { if let Some(remote_origin) = &self.config.remote_origin { // attempts to clone info!("index: cloning from {remote_origin}"); - if execute_git(&location, &["clone", remote_origin, "."]).await.is_ok() { - return Ok(()); + match execute_git(&location, &["clone", remote_origin, "."]).await { + Ok(()) => { + self.configure_user(&location).await?; + // cloned and (re-)configured the git user + return Ok(()); + } + Err(error) => { + // failed to clone + if expect_empty { + // this could be normal if we expected an empty index + // fallback to creating an empty index + info!( + "index: clone failed on empty database, this could be normal: {}", + error.details.as_ref().unwrap() + ); + } else { + // we expected to successfully clone because the database is not empty + // so we have some packages in the database, but not in the index ... not good + error!("index: clone unexpectedly failed: {}", error.details.as_ref().unwrap()); + return Err(error); + } + } } - info!("index: clone failed!"); } // initializes an empty index - self.initialize_empty_index(location).await?; + self.initialize_new_index(location).await?; Ok(()) } /// Initializes an empty index at the specified location - async fn initialize_empty_index(&self, location: PathBuf) -> Result<(), ApiError> { + async fn initialize_new_index(&self, location: PathBuf) -> Result<(), ApiError> { // initialise an empty repo info!("index: initializing empty index"); execute_git(&location, &["init"]).await?; - execute_git(&location, &["config", "user.name", &self.config.user_name]).await?; - execute_git(&location, &["config", "user.email", &self.config.user_email]).await?; + self.configure_user(&location).await?; if let Some(remote_origin) = &self.config.remote_origin { execute_git(&location, &["remote", "add", "origin", remote_origin]).await?; } @@ -144,6 +162,13 @@ impl GitIndexImpl { Ok(()) } + /// Configures the git user + async fn configure_user(&self, location: &Path) -> Result<(), ApiError> { + execute_git(location, &["config", "user.name", &self.config.user_name]).await?; + execute_git(location, &["config", "user.email", &self.config.user_email]).await?; + Ok(()) + } + /// Gets the full path to a file in the bare git repository fn get_index_file(&self, file_path: &Path) -> Option { let mut full_path = PathBuf::from(&self.config.location); diff --git a/src/services/index/mod.rs b/src/services/index/mod.rs index ea6cbb7..a718a30 100644 --- a/src/services/index/mod.rs +++ b/src/services/index/mod.rs @@ -60,7 +60,7 @@ pub fn build_package_file_path(mut root: PathBuf, name: &str) -> PathBuf { } /// Gets the index service -pub async fn get_service(config: &Configuration) -> Result, ApiError> { - let index = git::GitIndex::new(config.get_index_git_config()).await?; +pub async fn get_service(config: &Configuration, expect_empty: bool) -> Result, ApiError> { + let index = git::GitIndex::new(config.get_index_git_config(), expect_empty).await?; Ok(Arc::new(index)) } diff --git a/src/services/mod.rs b/src/services/mod.rs index e569d0b..2237b95 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -28,7 +28,7 @@ pub trait ServiceProvider { fn get_storage(config: &Configuration) -> Arc; /// Gets the index service - async fn get_index(config: &Configuration) -> Result, ApiError>; + async fn get_index(config: &Configuration, expect_empty: bool) -> Result, ApiError>; /// Gets the rustsec service fn get_rustsec(config: &Configuration) -> Arc; @@ -68,8 +68,8 @@ impl ServiceProvider for StandardServiceProvider { } /// Gets the index service - async fn get_index(config: &Configuration) -> Result, ApiError> { - index::get_service(config).await + async fn get_index(config: &Configuration, expect_empty: bool) -> Result, ApiError> { + index::get_service(config, expect_empty).await } /// Gets the rustsec service diff --git a/src/tests/mocks.rs b/src/tests/mocks.rs index 740a600..435564e 100644 --- a/src/tests/mocks.rs +++ b/src/tests/mocks.rs @@ -52,7 +52,7 @@ impl ServiceProvider for MockService { Arc::new(MockService) } - async fn get_index(_config: &Configuration) -> Result, ApiError> { + async fn get_index(_config: &Configuration, _expect_empty: bool) -> Result, ApiError> { Ok(Arc::new(MockService)) }