Skip to content

Commit

Permalink
cli: prevent pushing commits with conflicts
Browse files Browse the repository at this point in the history
It rarely makes sense to push commits with conflicts to a remote Git
repo (which is the only kind of remote we support so far), so let's
just error out instead of pushing a commit that others pulling from
the remote probably can't make sense of.

I've only added a simple test for the error case for now. `libgit2`
doesn't support pushing to a local repo, so it's harder to test the
success case. I suppose we'll have to have the regular `git` binary
running local servers in test eventually.

Closes libfuse#60.
  • Loading branch information
martinvonz committed Mar 10, 2022
1 parent a9eebe7 commit ea05f8f
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4365,9 +4365,11 @@ fn cmd_git_push(
}

let mut ref_updates = vec![];
let mut new_heads = vec![];
for (branch_name, update) in branch_updates {
let qualified_name = format!("refs/heads/{}", branch_name);
if let Some(new_target) = update.new_target {
new_heads.push(new_target.clone());
let force = match update.old_target {
None => false,
Some(old_target) => !repo.index().is_ancestor(&old_target, &new_target),
Expand All @@ -4386,6 +4388,24 @@ fn cmd_git_push(
}
}

// Check if there are conflicts in any commits we're about to push that haven't
// already been pushed.
let mut old_heads = vec![];
for branch_target in repo.view().branches().values() {
if let Some(old_head) = branch_target.remote_targets.get(remote_name) {
old_heads.extend(old_head.adds());
}
}
for index_entry in repo.index().walk_revs(&new_heads, &old_heads) {
let commit = repo.store().get_commit(&index_entry.commit_id())?;
if commit.tree().has_conflict() {
return Err(UserError(format!(
"Won't push commit {} since it has conflicts",
short_commit_hash(commit.id())
)));
}
}

let git_repo = get_git_repo(repo.store())?;
git::push_updates(&git_repo, remote_name, &ref_updates)
.map_err(|err| CommandError::UserError(err.to_string()))?;
Expand Down
70 changes: 70 additions & 0 deletions tests/test_git_push.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use jujutsu::testutils::{get_stdout_string, TestEnvironment};

#[test]
fn test_git_push() {
let test_env = TestEnvironment::default();
let git_repo_path = test_env.env_root().join("git-repo");
git2::Repository::init(&git_repo_path).unwrap();

test_env
.jj_cmd(
test_env.env_root(),
&["git", "clone", "git-repo", "jj-repo"],
)
.assert()
.success();
let workspace_root = test_env.env_root().join("jj-repo");

// No branches to push yet
let assert = test_env
.jj_cmd(&workspace_root, &["git", "push"])
.assert()
.success();
insta::assert_snapshot!(get_stdout_string(&assert), @"Nothing changed.
");

// Try pushing a conflict
std::fs::write(workspace_root.join("file"), "first").unwrap();
test_env
.jj_cmd(&workspace_root, &["close", "-m", "first"])
.assert()
.success();
std::fs::write(workspace_root.join("file"), "second").unwrap();
test_env
.jj_cmd(&workspace_root, &["close", "-m", "second"])
.assert()
.success();
std::fs::write(workspace_root.join("file"), "third").unwrap();
test_env
.jj_cmd(&workspace_root, &["rebase", "-d", "@--"])
.assert()
.success();
test_env
.jj_cmd(&workspace_root, &["branch", "my-branch"])
.assert()
.success();
test_env
.jj_cmd(&workspace_root, &["close", "-m", "third"])
.assert()
.success();
let assert = test_env
.jj_cmd(&workspace_root, &["git", "push"])
.assert()
.failure();
insta::assert_snapshot!(get_stdout_string(&assert), @"Error: Won't push commit 56e09a8ca383 since it has conflicts
");
}

0 comments on commit ea05f8f

Please sign in to comment.