Skip to content

Commit

Permalink
git-push: process --change branches first to avoid pushing old branch…
Browse files Browse the repository at this point in the history
… state

This fixes --change/--branch conflicts by making --change precede --branch. I
don't think this is the most obvious behavior, but it's the easiest workaround.
  • Loading branch information
yuja committed Mar 24, 2024
1 parent 538bfbc commit f312307
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 21 deletions.
23 changes: 7 additions & 16 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,37 +836,28 @@ fn cmd_git_push(
tx_description = format!("push all deleted branches to git remote {remote}");
} else {
let mut seen_branches: HashSet<&str> = HashSet::new();
let branches_by_name = find_branches_to_push(repo.view(), &args.branch, &remote)?;
for &(branch_name, targets) in &branches_by_name {
if !seen_branches.insert(branch_name) {
continue;
}
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)),
Ok(None) => writeln!(
ui.stderr(),
"Branch {branch_name}@{remote} already matches {branch_name}",
)?,
Err(reason) => return Err(reason.into()),
}
}

// Process --change branches first because matching branches can be moved.
let change_branch_names = update_change_branches(
ui,
&mut tx,
&args.change,
&command.settings().push_branch_prefix(),
)?;
for branch_name in &change_branch_names {
let change_branches = change_branch_names.iter().map(|branch_name| {
let targets = LocalAndRemoteRef {
local_target: tx.repo().view().get_local_branch(branch_name),
remote_ref: tx.repo().view().get_remote_branch(branch_name, &remote),
};
(branch_name.as_ref(), targets)
});
let branches_by_name = find_branches_to_push(repo.view(), &args.branch, &remote)?;
for (branch_name, targets) in change_branches.chain(branches_by_name.iter().copied()) {
if !seen_branches.insert(branch_name) {
continue;
}
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)),
Ok(None) => writeln!(
ui.stderr(),
"Branch {branch_name}@{remote} already matches {branch_name}",
Expand Down
10 changes: 5 additions & 5 deletions cli/tests/test_git_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ fn test_git_push_changes() {
Force branch push-yostqsxwqrlt from b5f030322b1d to 4df62cec2ee4
"###);

// FIXME: try again with --change that moves the branch forward
// try again with --change that moves the branch forward
std::fs::write(workspace_root.join("file"), "modified5").unwrap();
test_env.jj_cmd_ok(
&workspace_root,
Expand All @@ -460,14 +460,14 @@ fn test_git_push_changes() {
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Branch changes to push to origin:
Force branch push-yostqsxwqrlt from 4df62cec2ee4 to fa16a14170fb
Force branch push-yostqsxwqrlt from 4df62cec2ee4 to 3e2ce808759b
"###);
let stdout = test_env.jj_cmd_success(&workspace_root, &["status"]);
insta::assert_snapshot!(stdout, @r###"
Working copy changes:
M file
Working copy : yostqsxw 3e2ce808 push-yostqsxwqrlt* | bar
Parent commit: yqosqzyt fa16a141 push-yostqsxwqrlt@origin push-yqosqzytrlsw | foo
Working copy : yostqsxw 3e2ce808 push-yostqsxwqrlt | bar
Parent commit: yqosqzyt fa16a141 push-yqosqzytrlsw | foo
"###);

// Test changing `git.push-branch-prefix`. It causes us to push again.
Expand Down Expand Up @@ -580,8 +580,8 @@ fn test_git_push_mixed() {
insta::assert_snapshot!(stderr, @r###"
Creating branch push-yqosqzytrlsw for revision @--
Branch changes to push to origin:
Add branch branch-1 to 7decc7932d9c
Add branch push-yqosqzytrlsw to fa16a14170fb
Add branch branch-1 to 7decc7932d9c
Add branch branch-2a to 1b45449e18d0
Add branch branch-2b to 1b45449e18d0
"###);
Expand Down

0 comments on commit f312307

Please sign in to comment.