diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 7a03b258e..22a38e74d 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -259,9 +259,22 @@ impl From for CommandError { impl From for CommandError { fn from(err: GitImportError) -> Self { - CommandError::InternalError(format!( - "Failed to import refs from underlying Git repo: {err}" - )) + let message = format!("Failed to import refs from underlying Git repo: {err}"); + let missing_object = matches!( + err, + GitImportError::MissingHeadTarget { .. } | GitImportError::MissingRefAncestor { .. } + ); + let hint = missing_object.then(|| { + "\ +Is this Git repository a shallow or partial clone (cloned with the --depth or --filter \ + argument)? +jj currently does not support shallow/partial clones. To use jj with this repository, \ + try +unshallowing the repository (https://stackoverflow.com/q/6802145) or re-cloning with the full +repository contents." + .to_string() + }); + CommandError::UserError { message, hint } } } diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 8d95b7a2f..0550ec513 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -492,6 +492,7 @@ fn do_git_clone( GitFetchError::NoSuchRemote(_) => { panic!("shouldn't happen as we just created the git remote") } + GitFetchError::GitImportError(err) => CommandError::from(err), GitFetchError::InternalGitError(err) => map_git_error(err), GitFetchError::InvalidGlob => { unreachable!("we didn't provide any globs") diff --git a/lib/src/git.rs b/lib/src/git.rs index 0b18f83b1..2c1110582 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -25,7 +25,7 @@ use itertools::Itertools; use tempfile::NamedTempFile; use thiserror::Error; -use crate::backend::{CommitId, ObjectId}; +use crate::backend::{BackendError, CommitId, ObjectId}; use crate::git_backend::NO_GC_REF_NAMESPACE; use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt}; use crate::repo::{MutableRepo, Repo}; @@ -33,8 +33,20 @@ use crate::revset; use crate::settings::GitSettings; use crate::view::{RefName, View}; -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug)] pub enum GitImportError { + #[error("Failed to read Git HEAD target commit {id}: {err}", id=id.hex())] + MissingHeadTarget { + id: CommitId, + #[source] + err: BackendError, + }, + #[error("Ancestor of Git ref {ref_name} is missing: {err}")] + MissingRefAncestor { + ref_name: String, + #[source] + err: BackendError, + }, #[error("Unexpected git error when importing refs: {0}")] InternalGitError(#[from] git2::Error), } @@ -196,7 +208,12 @@ pub fn import_some_refs( // doesn't automatically mean the old HEAD branch has been rewritten. let head_commit_id = CommitId::from_bytes(head_git_commit.id().as_bytes()); if !matches!(mut_repo.git_head().as_normal(), Some(id) if id == &head_commit_id) { - let head_commit = store.get_commit(&head_commit_id).unwrap(); + let head_commit = store.get_commit(&head_commit_id).map_err(|err| { + GitImportError::MissingHeadTarget { + id: head_commit_id.clone(), + err, + } + })?; prevent_gc(git_repo, &head_commit_id)?; mut_repo.add_head(&head_commit); mut_repo.set_git_head_target(RefTarget::normal(head_commit_id)); @@ -208,13 +225,18 @@ pub fn import_some_refs( let changed_git_refs = diff_refs_to_import(mut_repo.view(), git_repo, git_ref_filter)?; // Import new heads - for id in changed_git_refs - .values() - .flat_map(|(_, new_git_target)| new_git_target.added_ids()) - { - prevent_gc(git_repo, id)?; - let commit = store.get_commit(id).unwrap(); - mut_repo.add_head(&commit); + for (ref_name, (_, new_git_target)) in &changed_git_refs { + for id in new_git_target.added_ids() { + let commit = + store + .get_commit(id) + .map_err(|err| GitImportError::MissingRefAncestor { + ref_name: ref_name.to_string(), + err, + })?; + prevent_gc(git_repo, id)?; + mut_repo.add_head(&commit); + } } for (ref_name, (old_git_target, new_git_target)) in &changed_git_refs { @@ -582,12 +604,14 @@ pub fn rename_remote( Ok(()) } -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug)] pub enum GitFetchError { #[error("No git remote named '{0}'")] NoSuchRemote(String), #[error("Invalid glob provided. Globs may not contain the characters `:` or `^`.")] InvalidGlob, + #[error("Failec to import Git refs: {0}")] + GitImportError(#[from] GitImportError), // TODO: I'm sure there are other errors possible, such as transport-level errors. #[error("Unexpected git error when fetching: {0}")] InternalGitError(#[from] git2::Error), @@ -718,9 +742,6 @@ pub fn fetch( to_remote_branch(ref_name, remote_name) .map(&branch_name_filter) .unwrap_or(false) - }) - .map_err(|err| match err { - GitImportError::InternalGitError(source) => GitFetchError::InternalGitError(source), })?; Ok(default_branch) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index ad452b512..a846b9b92 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -15,8 +15,9 @@ use std::collections::{BTreeMap, HashSet}; use std::path::PathBuf; use std::sync::{mpsc, Arc, Barrier}; -use std::thread; +use std::{fs, thread}; +use assert_matches::assert_matches; use git2::Oid; use itertools::Itertools; use jj_lib::backend::{ @@ -25,7 +26,7 @@ use jj_lib::backend::{ use jj_lib::commit::Commit; use jj_lib::commit_builder::CommitBuilder; use jj_lib::git; -use jj_lib::git::{GitFetchError, GitPushError, GitRefUpdate, SubmoduleConfig}; +use jj_lib::git::{GitFetchError, GitImportError, GitPushError, GitRefUpdate, SubmoduleConfig}; use jj_lib::git_backend::GitBackend; use jj_lib::op_store::{BranchTarget, RefTarget}; use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo}; @@ -938,6 +939,66 @@ fn test_import_refs_empty_git_repo() { assert_eq!(repo.view().git_head(), RefTarget::absent_ref()); } +#[test] +fn test_import_refs_missing_git_commit() { + let settings = testutils::user_settings(); + let git_settings = GitSettings::default(); + let test_workspace = TestRepo::init(true); + let repo = &test_workspace.repo; + let git_repo = get_git_repo(repo); + + // Missing commit is ancestor of ref + let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]); + empty_git_commit(&git_repo, "refs/heads/main", &[&commit1]); + let shard = hex::encode(&commit1.id().as_bytes()[..1]); + let object_basename = hex::encode(&commit1.id().as_bytes()[1..]); + let object_store_path = git_repo.path().join("objects"); + let object_file = object_store_path.join(&shard).join(object_basename); + let backup_object_file = object_store_path.join(&shard).join("backup"); + assert!(object_file.exists()); + 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); + assert_matches!( + result, + Err(GitImportError::MissingRefAncestor { + ref_name, + err: BackendError::ObjectNotFound { .. } + }) if &ref_name == "main" + ); + + // Missing commit is pointed to by ref + fs::rename(&backup_object_file, &object_file).unwrap(); + git_repo + .reference("refs/heads/main", commit1.id(), true, "test") + .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); + assert_matches!( + result, + Err(GitImportError::MissingRefAncestor { + ref_name, + err: BackendError::ObjectNotFound { .. } + }) if &ref_name == "main" + ); + + // Missing commit is pointed to by HEAD + fs::rename(&backup_object_file, &object_file).unwrap(); + git_repo.set_head_detached(commit1.id()).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); + assert_matches!( + result, + Err(GitImportError::MissingHeadTarget { + id, + err: BackendError::ObjectNotFound { .. } + }) if id == jj_id(&commit1) + ); +} + #[test] fn test_import_refs_detached_head() { let test_data = GitRepoData::create();