Skip to content

Commit

Permalink
Start gitoxide migration (#69)
Browse files Browse the repository at this point in the history
See #62. The idea is that gitoxide is under active development, and is
advertised as being fast, so I want to switch to it.

This is partly motivated by speed, and partly by struggling to work with
git2's diff API (I also couldn't figure out gix's, so that's not a
completely justified reason to migrate).

I have not yet decided if I want to build parallel implementations, or
migrate the existing APIs over to gix. (Implement the git2 APIs in terms
of gix with a translation shim).

So instead, this PR is focused on filling in some gaps that I noticed in
another branch starting to do some migration. Of particular note is the
fetch/pull tests. Switching fetch to gix revealed some shortcomings in
my own understanding, the test coverage, and not necessarily a 1-1
mapping between git2 and gix's fetch APIs.
  • Loading branch information
Notgnoshi authored Dec 30, 2024
2 parents 3efe0f6 + 8b76346 commit a747482
Show file tree
Hide file tree
Showing 11 changed files with 1,812 additions and 102 deletions.
1,445 changes: 1,391 additions & 54 deletions Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ctor = "0.2.7"
directories = "5.0.1"
eyre = "0.6.12"
git2 = "0.19"
gix = { version = "0.67.0", features = ["blob-diff", "blocking-network-client", "credentials", "excludes", "index", "mailmap", "revision", "tracing", "tree-editor"] }
inventory = "0.3.15"
lazy_static = "1.4.0"
predicates = "3.1.0"
Expand Down
1 change: 1 addition & 0 deletions herostratus-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ assert_cmd.workspace = true
ctor.workspace = true
eyre.workspace = true
git2.workspace = true
gix.workspace = true
lazy_static.workspace = true
tempfile.workspace = true
tracing.workspace = true
Expand Down
156 changes: 139 additions & 17 deletions herostratus-tests/src/fixtures/repository.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,51 @@
use git2::{Repository, Signature, Time};
use tempfile::{tempdir, TempDir};

pub struct TempRepository {
pub struct TempRepository<R = git2::Repository> {
pub tempdir: TempDir,
pub repo: Repository,
pub repo: R,
}

pub fn add_empty_commit(repo: &Repository, message: &str) -> eyre::Result<()> {
impl<R> TempRepository<R> {
/// Consume the TempDir without deleting the on-disk repository
///
/// You probably don't want to use this in the final state of a test, but it can be useful for
/// troubleshooting when things aren't working as you think they should.
pub fn forget(self) -> R {
let repo = self.repo;
// consumes the TempDir without deleting it
let _path = self.tempdir.into_path();
repo
}
}

impl TempRepository<git2::Repository> {
pub fn git2(&self) -> git2::Repository {
git2::Repository::discover(self.tempdir.path()).unwrap()
}
pub fn gix(&self) -> gix::Repository {
gix::discover(self.tempdir.path()).unwrap()
}
}
impl TempRepository<gix::Repository> {
pub fn git2(&self) -> git2::Repository {
git2::Repository::discover(self.tempdir.path()).unwrap()
}
pub fn gix(&self) -> gix::Repository {
self.repo.clone()
}
}

pub fn add_empty_commit<'r>(repo: &'r Repository, message: &str) -> eyre::Result<git2::Commit<'r>> {
let time = Time::new(1711656630, -500);
add_empty_commit_time(repo, message, time)
}

pub fn add_empty_commit_time<'r>(
repo: &'r Repository,
message: &str,
time: Time,
) -> eyre::Result<git2::Commit<'r>> {
let mut index = repo.index()?;
let head = repo.find_reference("HEAD")?;
let parent = head.peel_to_commit().ok();
Expand All @@ -19,7 +58,6 @@ pub fn add_empty_commit(repo: &Repository, message: &str) -> eyre::Result<()> {
let oid = index.write_tree()?;
let tree = repo.find_tree(oid)?;

let time = Time::new(1711656630, -500);
let signature = Signature::new("Herostratus", "[email protected]", &time)?;

let oid = repo.commit(
Expand All @@ -30,9 +68,13 @@ pub fn add_empty_commit(repo: &Repository, message: &str) -> eyre::Result<()> {
&tree,
&parents,
)?;
tracing::debug!("Created commit {oid:?}");
let commit = repo.find_commit(oid)?;
tracing::debug!(
"Created commit {oid:?} with message {message:?} in repo {:?}",
repo.path()
);

Ok(())
Ok(commit)
}

pub fn simplest() -> eyre::Result<TempRepository> {
Expand All @@ -52,22 +94,80 @@ pub fn with_empty_commits(messages: &[&str]) -> eyre::Result<TempRepository> {
Ok(TempRepository { tempdir, repo })
}

/// Return a pair of empty [TempRepository]s with the upstream configured as the "origin" remote of
/// the downstream
pub fn upstream_downstream_empty() -> eyre::Result<(TempRepository, TempRepository)> {
let upstream = with_empty_commits(&[])?;
let downstream = with_empty_commits(&[])?;
tracing::debug!(
"Setting {:?} as upstream remote of {:?}",
upstream.tempdir.path(),
downstream.tempdir.path()
);
downstream.repo.remote_set_url(
"origin",
&format!("file://{}", upstream.tempdir.path().display()),
)?;
Ok((upstream, downstream))
}

pub fn upstream_downstream() -> eyre::Result<(TempRepository, TempRepository)> {
let (upstream, downstream) = upstream_downstream_empty()?;
add_empty_commit(&upstream.repo, "Initial upstream commit")?;
add_empty_commit(&downstream.repo, "Initial downstream commit")?;
Ok((upstream, downstream))
}

/// Switch to the specified branch, creating it at the current HEAD if necessary
pub fn switch_branch(repo: &git2::Repository, branch_name: &str) -> eyre::Result<()> {
tracing::debug!(
"Switching to branch {branch_name:?} in repo {:?}",
repo.path()
);
// NOTE: gix can create a branch, but can't (yet?) switch to it in the working tree
//
// See: https://github.com/GitoxideLabs/gitoxide/discussions/879
// See: https://github.com/GitoxideLabs/gitoxide/issues/301 (maybe it _is_ supported?)

if repo.find_reference(branch_name).is_err() {
tracing::debug!(
"Failed to find {branch_name:?} in repo {:?} ... creating",
repo.path()
);
let head = repo.head()?;
let head = head.peel_to_commit()?;
// If the branch exists, replace it. If it doesn't exist, make it.
let _branch = repo.branch(branch_name, &head, true)?;
}

repo.set_head(format!("refs/heads/{branch_name}").as_str())?;

Ok(())
}

#[cfg(test)]
mod tests {
use git2::{Index, Odb, Repository};
use herostratus::git;

use super::*;

#[test]
fn test_forget() {
let temp = simplest().unwrap();
let repo = temp.forget();

assert!(repo.path().exists());
std::fs::remove_dir_all(repo.path()).unwrap();
assert!(!repo.path().exists());
}

#[test]
fn test_in_memory() {
let odb = Odb::new().unwrap();
let repo = Repository::from_odb(odb).unwrap();
let odb = git2::Odb::new().unwrap();
let repo = git2::Repository::from_odb(odb).unwrap();

// This fails with in-memory Repository / Odb's
assert!(repo.index().is_err());

let mut index = Index::new().unwrap();
let mut index = git2::Index::new().unwrap();
repo.set_index(&mut index).unwrap();
let mut index = repo.index().unwrap();

Expand All @@ -79,12 +179,34 @@ mod tests {
fn test_new_repository() {
let temp_repo = simplest().unwrap();

let rev = git::rev::parse("HEAD", &temp_repo.repo).unwrap();
let commits: Vec<_> = git::rev::walk(rev, &temp_repo.repo)
let branches: Vec<_> = temp_repo.repo.branches(None).unwrap().collect();
assert_eq!(branches.len(), 1);
let (branch, branch_type) = branches[0].as_ref().unwrap();
assert_eq!(branch_type, &git2::BranchType::Local);
assert!(branch.is_head());
assert_eq!(branch.name().unwrap().unwrap(), "master");
}

#[test]
fn test_switch_branch() {
let temp_repo = simplest().unwrap();

// Create two branches pointing at HEAD
switch_branch(&temp_repo.repo, "branch1").unwrap();
switch_branch(&temp_repo.repo, "branch2").unwrap();

switch_branch(&temp_repo.repo, "branch1").unwrap();
add_empty_commit(&temp_repo.repo, "commit on branch1").unwrap();

switch_branch(&temp_repo.repo, "branch2").unwrap();
add_empty_commit(&temp_repo.repo, "commit on branch2").unwrap();

let branches: Vec<_> = temp_repo
.repo
.branches(None)
.unwrap()
.map(|oid| temp_repo.repo.find_commit(oid.unwrap()).unwrap())
.map(|b| b.unwrap().0.name().unwrap().unwrap().to_string())
.collect();
assert_eq!(commits.len(), 1);
assert_eq!(commits[0].summary().unwrap(), "Initial commit");
assert_eq!(branches, ["branch1", "branch2", "master"]);
}
}
1 change: 1 addition & 0 deletions herostratus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ color-eyre.workspace = true
directories.workspace = true
eyre.workspace = true
git2.workspace = true
gix.workspace = true
inventory.workspace = true
serde.workspace = true
toml.workspace = true
Expand Down
5 changes: 2 additions & 3 deletions herostratus/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ pub fn check(args: &CheckArgs) -> eyre::Result<()> {

pub fn check_all(args: &CheckAllArgs, config: &Config, data_dir: &Path) -> eyre::Result<()> {
if !args.no_fetch {
crate::commands::fetch_all(&args.into(), config, data_dir)?
let _newly_fetched = crate::commands::fetch_all(&args.into(), config, data_dir)?;
}

tracing::info!("Checking repositories ...");
let start = Instant::now();
for (name, repo_config) in config.repositories.iter() {
let span = tracing::debug_span!("check", name = name);
let _enter = span.enter();
let _span = tracing::debug_span!("check", name = name).entered();
let repo = find_local_repository(&repo_config.path)?;
let reference = repo_config
.branch
Expand Down
12 changes: 6 additions & 6 deletions herostratus/src/commands/fetch_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ use std::time::Instant;

use crate::cli::FetchAllArgs;
use crate::config::Config;
use crate::git::clone::{clone_repository, fetch_remote, find_local_repository};
use crate::git::clone::{clone_repository, find_local_repository, pull_branch};

pub fn fetch_all(_args: &FetchAllArgs, config: &Config, _data_dir: &Path) -> eyre::Result<()> {
pub fn fetch_all(_args: &FetchAllArgs, config: &Config, _data_dir: &Path) -> eyre::Result<usize> {
tracing::info!("Fetching repositories ...");
let start = Instant::now();
let mut fetched_commits = 0;
for (name, config) in config.repositories.iter() {
let span = tracing::debug_span!("fetch", name = name);
let _enter = span.enter();
let _span = tracing::debug_span!("fetch", name = name).entered();
let mut skip_fetch = false;
let repo = match find_local_repository(&config.path) {
Ok(repo) => repo,
Expand All @@ -27,7 +27,7 @@ pub fn fetch_all(_args: &FetchAllArgs, config: &Config, _data_dir: &Path) -> eyr
};

if !skip_fetch {
fetch_remote(config, &repo)?
fetched_commits += pull_branch(config, &repo)?;
}
}
tracing::info!(
Expand All @@ -36,5 +36,5 @@ pub fn fetch_all(_args: &FetchAllArgs, config: &Config, _data_dir: &Path) -> eyr
start.elapsed()
);

Ok(())
Ok(fetched_commits)
}
Loading

0 comments on commit a747482

Please sign in to comment.