diff --git a/lib/src/git.rs b/lib/src/git.rs index 32eaa35d6..b8167d97d 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -173,6 +173,10 @@ pub fn push_commit( target: &Commit, remote_name: &str, remote_branch: &str, + // TODO: We want this to be an Option for the expected current commit on the remote. + // It's a blunt "force" option instead until git2-rs supports the "push negotiation" callback + // (https://github.com/rust-lang/git2-rs/issues/733). + force: bool, ) -> Result<(), GitPushError> { // Create a temporary ref to work around https://github.com/libgit2/libgit2/issues/3178 let temp_ref_name = format!("refs/jj/git-push/{}", target.id().hex()); @@ -184,7 +188,12 @@ pub fn push_commit( )?; // Need to add "refs/heads/" prefix due to https://github.com/libgit2/libgit2/issues/1125 let qualified_remote_branch = format!("refs/heads/{}", remote_branch); - let refspec = format!("{}:{}", temp_ref_name, qualified_remote_branch); + let refspec = format!( + "{}{}:{}", + (if force { "+" } else { "" }), + temp_ref_name, + qualified_remote_branch + ); let result = push_ref(git_repo, remote_name, &qualified_remote_branch, &refspec); // TODO: Figure out how to do the equivalent of absl::Cleanup for // temp_ref.delete(). @@ -196,6 +205,8 @@ pub fn delete_remote_branch( git_repo: &git2::Repository, remote_name: &str, remote_branch: &str, + /* TODO: Similar to push_commit(), we want an CommitId for the expected current commit on + * the remote. */ ) -> Result<(), GitPushError> { // Need to add "refs/heads/" prefix due to https://github.com/libgit2/libgit2/issues/1125 let qualified_remote_branch = format!("refs/heads/{}", remote_branch); diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index a0030b7b9..86aa42a03 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -353,7 +353,7 @@ fn test_push_commit_success() { let temp_dir = tempfile::tempdir().unwrap(); let setup = set_up_push_repos(&settings, &temp_dir); let clone_repo = setup.jj_repo.store().git_repo().unwrap(); - let result = git::push_commit(&clone_repo, &setup.new_commit, "origin", "main"); + let result = git::push_commit(&clone_repo, &setup.new_commit, "origin", "main", false); assert_eq!(result, Ok(())); // Check that the ref got updated in the source repo @@ -388,10 +388,38 @@ fn test_push_commit_not_fast_forward() { &new_commit, "origin", "main", + false, ); assert_eq!(result, Err(GitPushError::NotFastForward)); } +#[test] +fn test_push_commit_not_fast_forward_with_force() { + let settings = testutils::user_settings(); + let temp_dir = tempfile::tempdir().unwrap(); + let mut setup = set_up_push_repos(&settings, &temp_dir); + let new_commit = testutils::create_random_commit(&settings, &setup.jj_repo) + .write_to_new_transaction(&setup.jj_repo, "test"); + setup.jj_repo = setup.jj_repo.reload(); + let result = git::push_commit( + &setup.jj_repo.store().git_repo().unwrap(), + &new_commit, + "origin", + "main", + true, + ); + assert_eq!(result, Ok(())); + + // Check that the ref got updated in the source repo + let source_repo = git2::Repository::open(&setup.source_repo_dir).unwrap(); + let new_target = source_repo + .find_reference("refs/heads/main") + .unwrap() + .target(); + let new_oid = Oid::from_bytes(&new_commit.id().0).unwrap(); + assert_eq!(new_target, Some(new_oid)); +} + #[test] fn test_push_commit_no_such_remote() { let settings = testutils::user_settings(); @@ -402,6 +430,7 @@ fn test_push_commit_no_such_remote() { &setup.new_commit, "invalid-remote", "main", + false, ); assert!(matches!(result, Err(GitPushError::NoSuchRemote(_)))); } @@ -416,6 +445,7 @@ fn test_push_commit_invalid_remote() { &setup.new_commit, "http://invalid-remote", "main", + false, ); assert!(matches!(result, Err(GitPushError::NoSuchRemote(_)))); } diff --git a/src/commands.rs b/src/commands.rs index 5d3d55b46..33c43e7b4 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2585,7 +2585,8 @@ fn cmd_git_push( } let branch_target = maybe_branch_target.unwrap(); - if branch_target.local_target.as_ref() == branch_target.remote_targets.get(remote_name) { + let maybe_remote_target = branch_target.remote_targets.get(remote_name); + if branch_target.local_target.as_ref() == maybe_remote_target { writeln!( ui, "Branch {}@{} already matches {}", @@ -2610,8 +2611,27 @@ fn cmd_git_push( "Won't push open commit".to_string(), )); } - git::push_commit(&git_repo, &new_target_commit, remote_name, branch_name) - .map_err(|err| CommandError::UserError(err.to_string()))?; + let force = match maybe_remote_target { + None => false, + Some(RefTarget::Conflict { .. }) => { + return Err(CommandError::UserError(format!( + "Branch {}@{} is conflicted", + branch_name, remote_name + ))); + } + Some(RefTarget::Normal(old_target_id)) => { + !repo.index().is_ancestor(old_target_id, new_target_id) + } + }; + + git::push_commit( + &git_repo, + &new_target_commit, + remote_name, + branch_name, + force, + ) + .map_err(|err| CommandError::UserError(err.to_string()))?; } } } else {