From 6c98dfcdcbbf302cf6f04c0c8ce436a74ea7e831 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 8 Nov 2023 19:20:46 +0900 Subject: [PATCH] git: have import_refs() obtain git2::Repository instance from store This helps gitoxide migration. It's theoretically possible to import Git refs from non-Git backend, but I don't think such API flexibility is needed. --- cli/src/cli_util.rs | 13 ++-- cli/src/commands/git.rs | 4 +- cli/src/commands/init.rs | 8 --- lib/src/git.rs | 27 ++++---- lib/tests/test_git.rs | 131 ++++++++++++++++++--------------------- lib/tests/test_revset.rs | 2 +- 6 files changed, 81 insertions(+), 104 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 04a2e1b65..261fc430b 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -28,7 +28,6 @@ use std::{iter, str}; use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory}; use clap::{Arg, ArgAction, ArgMatches, Command, FromArgMatches}; -use git2::Repository; use indexmap::IndexSet; use itertools::Itertools; use jj_lib::backend::{BackendError, ChangeId, CommitId, MergedTreeId, ObjectId}; @@ -293,6 +292,7 @@ repository contents." Some("Run `jj git remote rename` to give different name.".to_string()) } GitImportError::InternalGitError(_) => None, + GitImportError::UnexpectedBackend => None, }; CommandError::UserError { message, hint } } @@ -782,8 +782,7 @@ impl WorkspaceCommandHelper { pub fn snapshot(&mut self, ui: &mut Ui) -> Result<(), CommandError> { if self.may_update_working_copy { if self.working_copy_shared_with_git { - let git_repo = self.git_backend().unwrap().open_git_repo()?; - self.import_git_refs_and_head(ui, &git_repo)?; + self.import_git_refs_and_head(ui)?; } self.snapshot_working_copy(ui)?; } @@ -791,15 +790,11 @@ impl WorkspaceCommandHelper { } #[instrument(skip_all)] - fn import_git_refs_and_head( - &mut self, - ui: &mut Ui, - git_repo: &Repository, - ) -> Result<(), CommandError> { + fn import_git_refs_and_head(&mut self, ui: &mut Ui) -> Result<(), CommandError> { let git_settings = self.settings.git_settings(); let mut tx = self.start_transaction("import git refs"); // Automated import shouldn't fail because of reserved remote name. - let stats = git::import_some_refs(tx.mut_repo(), git_repo, &git_settings, |ref_name| { + let stats = git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { !git::is_reserved_git_remote_ref(ref_name) })?; if !tx.mut_repo().has_changes() { diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 8ac695926..496524e19 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -1082,10 +1082,8 @@ fn cmd_git_import( _args: &GitImportArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let repo = workspace_command.repo(); - let git_repo = get_git_repo(repo.store())?; let mut tx = workspace_command.start_transaction("import git refs"); - let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?; + let stats = git::import_refs(tx.mut_repo(), &command.settings().git_settings())?; print_git_import_stats(ui, &stats)?; tx.finish(ui)?; Ok(()) diff --git a/cli/src/commands/init.rs b/cli/src/commands/init.rs index 686d5fec7..aff919b55 100644 --- a/cli/src/commands/init.rs +++ b/cli/src/commands/init.rs @@ -17,7 +17,6 @@ use std::io::Write; use clap::ArgGroup; use jj_lib::file_util; -use jj_lib::git_backend::GitBackend; use jj_lib::repo::Repo; use jj_lib::workspace::Workspace; use tracing::instrument; @@ -79,12 +78,6 @@ pub(crate) fn cmd_init( } let (workspace, repo) = Workspace::init_external_git(command.settings(), &wc_path, &git_store_path)?; - let git_repo = repo - .store() - .backend_impl() - .downcast_ref::() - .unwrap() - .open_git_repo()?; let mut workspace_command = command.for_loaded_repo(ui, workspace, repo)?; git::maybe_add_gitignore(&workspace_command)?; workspace_command.snapshot(ui)?; @@ -92,7 +85,6 @@ pub(crate) fn cmd_init( let mut tx = workspace_command.start_transaction("import git refs"); let stats = jj_lib::git::import_some_refs( tx.mut_repo(), - &git_repo, &command.settings().git_settings(), |ref_name| !jj_lib::git::is_reserved_git_remote_ref(ref_name), )?; diff --git a/lib/src/git.rs b/lib/src/git.rs index 60fbcd858..b5bc5956f 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -33,6 +33,7 @@ use crate::refs::BranchPushUpdate; use crate::repo::{MutableRepo, Repo}; use crate::revset::{self, RevsetExpression}; use crate::settings::GitSettings; +use crate::store::Store; use crate::str_util::StringPattern; use crate::view::View; @@ -105,6 +106,10 @@ pub fn is_reserved_git_remote_ref(parsed_ref: &RefName) -> bool { to_remote_branch(parsed_ref, REMOTE_NAME_FOR_LOCAL_GIT_REPO).is_some() } +fn get_git_backend(store: &Store) -> Option<&GitBackend> { + store.backend_impl().downcast_ref() +} + /// Checks if `git_ref` points to a Git commit object, and returns its id. /// /// If the ref points to the previously `known_target` (i.e. unchanged), this @@ -164,6 +169,8 @@ pub enum GitImportError { RemoteReservedForLocalGitRepo, #[error("Unexpected git error when importing refs: {0}")] InternalGitError(#[from] git2::Error), + #[error("The repo is not backed by a Git repo")] + UnexpectedBackend, } /// Describes changes made by `import_refs()` or `fetch()`. @@ -188,10 +195,9 @@ struct RefsToImport { /// records them in JJ's view. pub fn import_refs( mut_repo: &mut MutableRepo, - git_repo: &git2::Repository, git_settings: &GitSettings, ) -> Result { - import_some_refs(mut_repo, git_repo, git_settings, |_| true) + import_some_refs(mut_repo, git_settings, |_| true) } /// Reflect changes made in the underlying Git repo in the Jujutsu repo. @@ -200,10 +206,13 @@ pub fn import_refs( /// considered for addition, update, or deletion. pub fn import_some_refs( mut_repo: &mut MutableRepo, - git_repo: &git2::Repository, git_settings: &GitSettings, git_ref_filter: impl Fn(&RefName) -> bool, ) -> Result { + let store = mut_repo.store(); + let git_backend = get_git_backend(store).ok_or(GitImportError::UnexpectedBackend)?; + let git_repo = git_backend.open_git_repo()?; // TODO: use gix::Repository + // TODO: Should this be a separate function? We may not always want to import // the Git HEAD (and add it to our set of heads). let old_git_head = mut_repo.view().git_head(); @@ -223,14 +232,10 @@ pub fn import_some_refs( let RefsToImport { changed_git_refs, changed_remote_refs, - } = diff_refs_to_import(mut_repo.view(), git_repo, git_ref_filter)?; + } = diff_refs_to_import(mut_repo.view(), &git_repo, git_ref_filter)?; - // Import new Git commits to the backend - let store = mut_repo.store(); - // TODO: It might be better to obtain both git_repo and git_backend from - // mut_repo, and return error if the repo isn't backed by Git. - let git_backend = store.backend_impl().downcast_ref::().unwrap(); - // Bulk-import all reachable commits to reduce overhead of table merging. + // Bulk-import all reachable Git commits to the backend to reduce overhead of + // table merging. let head_ids = itertools::chain( &changed_git_head, changed_git_refs.iter().map(|(_, new_target)| new_target), @@ -1076,7 +1081,7 @@ pub fn fetch( // local branches. We also import local tags since remote tags should have // been merged by Git. tracing::debug!("import_refs"); - let import_stats = import_some_refs(mut_repo, git_repo, git_settings, |ref_name| { + let import_stats = import_some_refs(mut_repo, git_settings, |ref_name| { to_remote_branch(ref_name, remote_name) .map(|branch| branch_names.iter().any(|pattern| pattern.matches(branch))) .unwrap_or_else(|| matches!(ref_name, RefName::Tag(_))) diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index e27b107b7..8aa44ecc6 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -105,9 +105,8 @@ fn test_import_refs() { git_repo.set_head("refs/heads/main").unwrap(); - let git_repo = get_git_repo(repo); let mut tx = repo.start_transaction(&settings, "test"); - let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); let view = repo.view(); @@ -226,7 +225,7 @@ fn test_import_refs_reimport() { .unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -254,7 +253,7 @@ fn test_import_refs_reimport() { let repo = tx.commit(); let mut tx = repo.start_transaction(&settings, "test"); - let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -330,7 +329,7 @@ fn test_import_refs_reimport_head_removed() { let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let commit_id = jj_id(&commit); // Test the setup @@ -338,7 +337,7 @@ fn test_import_refs_reimport_head_removed() { // Remove the head and re-import tx.mut_repo().remove_head(&commit_id); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(!tx.mut_repo().view().heads().contains(&commit_id)); } @@ -357,7 +356,7 @@ fn test_import_refs_reimport_git_head_counts() { git_repo.set_head_detached(commit.id()).unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); // Delete the branch and re-import. The commit should still be there since HEAD @@ -367,7 +366,7 @@ fn test_import_refs_reimport_git_head_counts() { .unwrap() .delete() .unwrap(); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit))); } @@ -388,7 +387,7 @@ fn test_import_refs_reimport_git_head_without_ref() { git_repo.set_head_detached(git_id(&commit1)).unwrap(); // Import HEAD. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -400,7 +399,7 @@ fn test_import_refs_reimport_git_head_without_ref() { // would be moved by `git checkout` command. This isn't always true because the // detached HEAD commit could be rewritten by e.g. `git commit --amend` command, // but it should be safer than abandoning old checkout branch. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -425,7 +424,7 @@ fn test_import_refs_reimport_git_head_with_moved_ref() { git_repo.set_head_detached(git_id(&commit1)).unwrap(); // Import HEAD and main. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -437,7 +436,7 @@ fn test_import_refs_reimport_git_head_with_moved_ref() { git_repo.set_head_detached(git_id(&commit2)).unwrap(); // Reimport HEAD and main, which abandons the old main branch. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(!tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -470,7 +469,7 @@ fn test_import_refs_reimport_with_deleted_remote_ref() { ); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -526,7 +525,7 @@ fn test_import_refs_reimport_with_deleted_remote_ref() { delete_git_ref(&git_repo, "refs/remotes/origin/feature-remote-and-local"); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -586,7 +585,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() { ); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -652,7 +651,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() { ); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); @@ -719,7 +718,7 @@ fn test_import_refs_reimport_with_moved_untracked_remote_ref() { let commit_base = empty_git_commit(&git_repo, remote_ref_name, &[]); let commit_remote_t0 = empty_git_commit(&git_repo, remote_ref_name, &[&commit_base]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); let view = repo.view(); @@ -739,7 +738,7 @@ fn test_import_refs_reimport_with_moved_untracked_remote_ref() { delete_git_ref(&git_repo, remote_ref_name); let commit_remote_t1 = empty_git_commit(&git_repo, remote_ref_name, &[&commit_base]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); let view = repo.view(); @@ -777,7 +776,7 @@ fn test_import_refs_reimport_git_head_with_fixed_ref() { git_repo.set_head_detached(git_id(&commit1)).unwrap(); // Import HEAD and main. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -786,7 +785,7 @@ fn test_import_refs_reimport_git_head_with_fixed_ref() { git_repo.set_head_detached(git_id(&commit2)).unwrap(); // Reimport HEAD, which shouldn't abandon the old HEAD branch. - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(commit1.id())); assert!(tx.mut_repo().view().heads().contains(commit2.id())); @@ -804,7 +803,7 @@ fn test_import_refs_reimport_all_from_root_removed() { let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); // Test the setup assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit))); @@ -815,7 +814,7 @@ fn test_import_refs_reimport_all_from_root_removed() { .unwrap() .delete() .unwrap(); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(!tx.mut_repo().view().heads().contains(&jj_id(&commit))); } @@ -835,7 +834,7 @@ fn test_import_refs_reimport_abandoning_disabled() { let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]); let commit2 = empty_git_commit(&git_repo, "refs/heads/delete-me", &[&commit1]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); // Test the setup assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit2))); @@ -846,7 +845,7 @@ fn test_import_refs_reimport_abandoning_disabled() { .unwrap() .delete() .unwrap(); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit2))); } @@ -862,12 +861,12 @@ fn test_import_refs_reimport_conflicted_remote_branch() { let commit1 = empty_git_commit(&git_repo, "refs/heads/commit1", &[]); git_ref(&git_repo, "refs/remotes/origin/main", commit1.id()); let mut tx1 = repo.start_transaction(&settings, "test"); - git::import_refs(tx1.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx1.mut_repo(), &git_settings).unwrap(); let commit2 = empty_git_commit(&git_repo, "refs/heads/commit2", &[]); git_ref(&git_repo, "refs/remotes/origin/main", commit2.id()); let mut tx2 = repo.start_transaction(&settings, "test"); - git::import_refs(tx2.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx2.mut_repo(), &git_settings).unwrap(); // Remote branch can diverge by concurrent operations (like `jj git fetch`) let repo = commit_transactions(&settings, vec![tx1, tx2]); @@ -885,7 +884,7 @@ fn test_import_refs_reimport_conflicted_remote_branch() { // The conflict can be resolved by importing the current Git state let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); let repo = tx.commit(); assert_eq!( repo.view().get_git_ref("refs/remotes/origin/main"), @@ -911,7 +910,7 @@ fn test_import_refs_reserved_remote_name() { empty_git_commit(&git_repo, "refs/remotes/git/main", &[]); let mut tx = repo.start_transaction(&settings, "test"); - let result = git::import_refs(tx.mut_repo(), &git_repo, &git_settings); + let result = git::import_refs(tx.mut_repo(), &git_settings); assert_matches!(result, Err(GitImportError::RemoteReservedForLocalGitRepo)); } @@ -944,7 +943,7 @@ fn test_import_some_refs() { // Import branches feature1, feature2, and feature3. let mut tx = repo.start_transaction(&settings, "test"); - git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { get_remote_branch(ref_name) .map(|branch| branch.starts_with("feature")) .unwrap_or(false) @@ -1023,7 +1022,7 @@ fn test_import_some_refs() { delete_git_ref(&git_repo, "refs/remotes/origin/feature3"); delete_git_ref(&git_repo, "refs/remotes/origin/feature4"); let mut tx = repo.start_transaction(&settings, "test"); - git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { get_remote_branch(ref_name) == Some("feature2") }) .unwrap(); @@ -1039,7 +1038,7 @@ fn test_import_some_refs() { // Import feature1: this should cause the branch to be deleted, but the // corresponding commit should stay because it is reachable from feature2. let mut tx = repo.start_transaction(&settings, "test"); - git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { get_remote_branch(ref_name) == Some("feature1") }) .unwrap(); @@ -1056,7 +1055,7 @@ fn test_import_some_refs() { // Import feature3: this should cause the branch to be deleted, but // feature4 should be left alone even though it is no longer in git. let mut tx = repo.start_transaction(&settings, "test"); - git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { get_remote_branch(ref_name) == Some("feature3") }) .unwrap(); @@ -1072,7 +1071,7 @@ fn test_import_some_refs() { // Import feature4: both the head and the branch will disappear. let mut tx = repo.start_transaction(&settings, "test"); - git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + git::import_some_refs(tx.mut_repo(), &git_settings, |ref_name| { get_remote_branch(ref_name) == Some("feature4") }) .unwrap(); @@ -1150,7 +1149,7 @@ fn test_import_refs_empty_git_repo() { let mut tx = test_data .repo .start_transaction(&test_data.settings, "test"); - git::import_refs(tx.mut_repo(), &test_data.git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo() .rebase_descendants(&test_data.settings) .unwrap(); @@ -1183,7 +1182,7 @@ fn test_import_refs_missing_git_commit() { git_repo.set_head("refs/heads/unborn").unwrap(); fs::rename(&object_file, &backup_object_file).unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - let result = git::import_refs(tx.mut_repo(), &git_repo, &git_settings); + let result = git::import_refs(tx.mut_repo(), &git_settings); assert_matches!( result, Err(GitImportError::MissingRefAncestor { @@ -1200,7 +1199,7 @@ fn test_import_refs_missing_git_commit() { .unwrap(); git_repo.set_head_detached(commit2.id()).unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - let result = git::import_refs(tx.mut_repo(), &git_repo, &git_settings); + let result = git::import_refs(tx.mut_repo(), &git_settings); assert_matches!( result, Err(GitImportError::MissingHeadTarget { @@ -1209,7 +1208,8 @@ fn test_import_refs_missing_git_commit() { }) if id == jj_id(&commit2) ); - // Missing commit is pointed to by ref + // Missing commit is pointed to by ref: the ref is ignored as we don't know + // if the missing object is a commit or not. fs::rename(&backup_object_file, &object_file).unwrap(); git_repo .reference("refs/heads/main", commit1.id(), true, "test") @@ -1217,17 +1217,11 @@ fn test_import_refs_missing_git_commit() { git_repo.set_head("refs/heads/unborn").unwrap(); fs::rename(&object_file, &backup_object_file).unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - // TODO: The ref is ignored if we passed fresh new git2 repo object. - let result = git::import_refs(tx.mut_repo(), &git_repo, &git_settings); - assert_matches!( - result, - Err(GitImportError::MissingRefAncestor { - ref_name, - err: BackendError::ObjectNotFound { .. } - }) if &ref_name == "main" - ); + let result = git::import_refs(tx.mut_repo(), &git_settings); + assert!(result.is_ok()); - // Missing commit is pointed to by HEAD + // Missing commit is pointed to by HEAD: the ref is ignored as we don't know + // if the missing object is a commit or not. fs::rename(&backup_object_file, &object_file).unwrap(); git_repo .find_reference("refs/heads/main") @@ -1237,15 +1231,8 @@ fn test_import_refs_missing_git_commit() { git_repo.set_head_detached(commit1.id()).unwrap(); fs::rename(&object_file, &backup_object_file).unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - // TODO: The ref is ignored if we passed fresh new git2 repo object. - let result = git::import_refs(tx.mut_repo(), &git_repo, &git_settings); - assert_matches!( - result, - Err(GitImportError::MissingHeadTarget { - id, - err: BackendError::ObjectNotFound { .. } - }) if id == jj_id(&commit1) - ); + let result = git::import_refs(tx.mut_repo(), &git_settings); + assert!(result.is_ok()); } #[test] @@ -1266,7 +1253,7 @@ fn test_import_refs_detached_head() { let mut tx = test_data .repo .start_transaction(&test_data.settings, "test"); - git::import_refs(tx.mut_repo(), &test_data.git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo() .rebase_descendants(&test_data.settings) .unwrap(); @@ -1291,7 +1278,7 @@ fn test_export_refs_no_detach() { .repo .start_transaction(&test_data.settings, "test"); let mut_repo = tx.mut_repo(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); mut_repo.rebase_descendants(&test_data.settings).unwrap(); // Do an initial export to make sure `main` is considered @@ -1323,7 +1310,7 @@ fn test_export_refs_branch_changed() { .repo .start_transaction(&test_data.settings, "test"); let mut_repo = tx.mut_repo(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); mut_repo.rebase_descendants(&test_data.settings).unwrap(); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(vec![])); @@ -1362,7 +1349,7 @@ fn test_export_refs_current_branch_changed() { .repo .start_transaction(&test_data.settings, "test"); let mut_repo = tx.mut_repo(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); mut_repo.rebase_descendants(&test_data.settings).unwrap(); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(vec![])); @@ -1399,7 +1386,7 @@ fn test_export_refs_unborn_git_branch() { .repo .start_transaction(&test_data.settings, "test"); let mut_repo = tx.mut_repo(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); mut_repo.rebase_descendants(&test_data.settings).unwrap(); assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(vec![])); @@ -1445,7 +1432,7 @@ fn test_export_import_sequence() { git_repo .reference("refs/heads/main", git_id(&commit_a), true, "test") .unwrap(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); assert_eq!( mut_repo.get_git_ref("refs/heads/main"), RefTarget::normal(commit_a.id().clone()) @@ -1467,7 +1454,7 @@ fn test_export_import_sequence() { .unwrap(); // Import from git - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); assert_eq!( mut_repo.get_git_ref("refs/heads/main"), RefTarget::normal(commit_c.id().clone()) @@ -1495,7 +1482,7 @@ fn test_import_export_non_tracking_branch() { .start_transaction(&test_data.settings, "test"); let mut_repo = tx.mut_repo(); - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); assert!(mut_repo.view().get_local_branch("main").is_absent()); assert_eq!( @@ -1520,7 +1507,7 @@ fn test_import_export_non_tracking_branch() { empty_git_commit(&git_repo, "refs/remotes/origin/main", &[&commit_main_t0]); let commit_feat_t1 = empty_git_commit(&git_repo, "refs/remotes/origin/feat", &[]); git_settings.auto_local_branch = true; - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); assert!(mut_repo.view().get_local_branch("main").is_absent()); assert_eq!( mut_repo.view().get_local_branch("feat"), @@ -1547,7 +1534,7 @@ fn test_import_export_non_tracking_branch() { let commit_feat_t2 = empty_git_commit(&git_repo, "refs/remotes/origin/feat", &[&commit_feat_t1]); git_settings.auto_local_branch = false; - git::import_refs(mut_repo, &git_repo, &git_settings).unwrap(); + git::import_refs(mut_repo, &git_settings).unwrap(); assert!(mut_repo.view().get_local_branch("main").is_absent()); assert_eq!( mut_repo.view().get_local_branch("feat"), @@ -2278,7 +2265,7 @@ fn test_fetch_empty_refspecs() { .get_remote_branch("main", "origin") .is_absent()); // No remote refs should have been fetched - git::import_refs(tx.mut_repo(), &test_data.git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); assert!(tx .mut_repo() .get_remote_branch("main", "origin") @@ -2434,7 +2421,7 @@ fn test_push_branches_success() { // Check that the repo view reflects the changes in the Git repo setup.jj_repo = tx.commit(); let mut tx = setup.jj_repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap(); + git::import_refs(tx.mut_repo(), &GitSettings::default()).unwrap(); assert!(!tx.mut_repo().has_changes()); } @@ -2487,7 +2474,7 @@ fn test_push_branches_deletion() { // Check that the repo view reflects the changes in the Git repo setup.jj_repo = tx.commit(); let mut tx = setup.jj_repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap(); + git::import_refs(tx.mut_repo(), &GitSettings::default()).unwrap(); assert!(!tx.mut_repo().has_changes()); } @@ -2557,7 +2544,7 @@ fn test_push_branches_mixed_deletion_and_addition() { // Check that the repo view reflects the changes in the Git repo setup.jj_repo = tx.commit(); let mut tx = setup.jj_repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &clone_repo, &GitSettings::default()).unwrap(); + git::import_refs(tx.mut_repo(), &GitSettings::default()).unwrap(); assert!(!tx.mut_repo().has_changes()); } @@ -2722,7 +2709,7 @@ fn test_bulk_update_extra_on_import_refs() { }; let import_refs = |repo: &Arc| { let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); tx.commit() }; @@ -2761,7 +2748,7 @@ fn test_rewrite_imported_commit() { // Import git commit, which generates change id from the commit id. let git_commit = empty_git_commit(&git_repo, "refs/heads/main", &[]); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); let imported_commit = repo.store().get_commit(&jj_id(&git_commit)).unwrap(); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 3d54a89e0..2b9acff4a 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -234,7 +234,7 @@ fn test_resolve_symbol_change_id(readonly: bool) { } let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); // Test the test setup assert_eq!(