Skip to content

Commit

Permalink
fix: missing git user info after first clone (#33)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
woutersl committed Sep 25, 2024
1 parent d82fd10 commit dfa54cb
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 21 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
8 changes: 8 additions & 0 deletions src/services/database/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool, ApiError> {
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<String, ApiError> {
let row = sqlx::query!(
Expand Down
53 changes: 39 additions & 14 deletions src/services/index/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,8 +24,8 @@ pub struct GitIndex {

impl GitIndex {
/// When the application is launched
pub async fn new(config: IndexConfig) -> Result<Self, ApiError> {
let inner = GitIndexImpl::new(config).await?;
pub async fn new(config: IndexConfig, expect_empty: bool) -> Result<Self, ApiError> {
let inner = GitIndexImpl::new(config, expect_empty).await?;
Ok(Self {
inner: Mutex::new(inner),
})
Expand Down Expand Up @@ -62,7 +62,7 @@ struct GitIndexImpl {

impl GitIndexImpl {
/// When the application is launched
async fn new(config: IndexConfig) -> Result<Self, ApiError> {
async fn new(config: IndexConfig, expect_empty: bool) -> Result<Self, ApiError> {
let index = Self { config };

// check for the SSH key
Expand All @@ -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");
Expand All @@ -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?;
}
Expand All @@ -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<PathBuf> {
let mut full_path = PathBuf::from(&self.config.location);
Expand Down
4 changes: 2 additions & 2 deletions src/services/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<dyn Index + Send + Sync>, ApiError> {
let index = git::GitIndex::new(config.get_index_git_config()).await?;
pub async fn get_service(config: &Configuration, expect_empty: bool) -> Result<Arc<dyn Index + Send + Sync>, ApiError> {
let index = git::GitIndex::new(config.get_index_git_config(), expect_empty).await?;
Ok(Arc::new(index))
}
6 changes: 3 additions & 3 deletions src/services/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub trait ServiceProvider {
fn get_storage(config: &Configuration) -> Arc<dyn storage::Storage + Send + Sync>;

/// Gets the index service
async fn get_index(config: &Configuration) -> Result<Arc<dyn index::Index + Send + Sync>, ApiError>;
async fn get_index(config: &Configuration, expect_empty: bool) -> Result<Arc<dyn index::Index + Send + Sync>, ApiError>;

/// Gets the rustsec service
fn get_rustsec(config: &Configuration) -> Arc<dyn rustsec::RustSecChecker + Send + Sync>;
Expand Down Expand Up @@ -68,8 +68,8 @@ impl ServiceProvider for StandardServiceProvider {
}

/// Gets the index service
async fn get_index(config: &Configuration) -> Result<Arc<dyn index::Index + Send + Sync>, ApiError> {
index::get_service(config).await
async fn get_index(config: &Configuration, expect_empty: bool) -> Result<Arc<dyn index::Index + Send + Sync>, ApiError> {
index::get_service(config, expect_empty).await
}

/// Gets the rustsec service
Expand Down
2 changes: 1 addition & 1 deletion src/tests/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ServiceProvider for MockService {
Arc::new(MockService)
}

async fn get_index(_config: &Configuration) -> Result<Arc<dyn Index + Send + Sync>, ApiError> {
async fn get_index(_config: &Configuration, _expect_empty: bool) -> Result<Arc<dyn Index + Send + Sync>, ApiError> {
Ok(Arc::new(MockService))
}

Expand Down

0 comments on commit dfa54cb

Please sign in to comment.