Skip to content

Commit

Permalink
refs: migrate classify_branch_push_action() to local/remote targets pair
Browse files Browse the repository at this point in the history
  • Loading branch information
@yuja
yuja committed Oct 12, 2023
1 parent 68545a6 commit 69a30b4
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 72 deletions.
59 changes: 33 additions & 26 deletions cli/src/commands/git.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ use jj_lib::git::{
self, parse_gitmodules, GitFetchError, GitFetchStats, GitPushError, GitRefUpdate,
};
use jj_lib::git_backend::GitBackend;
use jj_lib::op_store::{BranchTarget, RefTarget};
use jj_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
use jj_lib::op_store::RefTarget;
use jj_lib::refs::{
classify_branch_push_action, BranchPushAction, BranchPushUpdate, TrackingRefPair,
};
use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::revset::{self, RevsetExpression, RevsetIteratorExt as _, StringPattern};
Expand Down Expand Up @@ -700,21 +702,21 @@ fn cmd_git_push(
let tx_description;
let mut branch_updates = vec![];
if args.all {
for (branch_name, branch_target) in repo.view().branches() {
match classify_branch_update(branch_name, branch_target, &remote) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
for (branch_name, targets) in repo.view().local_remote_branches(&remote) {
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)),
Ok(None) => {}
Err(message) => writeln!(ui.warning(), "{message}")?,
}
}
tx_description = format!("push all branches to git remote {remote}");
} else if args.deleted {
for (branch_name, branch_target) in repo.view().branches() {
if branch_target.local_target.is_present() {
for (branch_name, targets) in repo.view().local_remote_branches(&remote) {
if targets.local_target.is_present() {
continue;
}
match classify_branch_update(branch_name, branch_target, &remote) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)),
Ok(None) => {}
Err(message) => writeln!(ui.warning(), "{message}")?,
}
Expand All @@ -726,11 +728,14 @@ fn cmd_git_push(
if !seen_branches.insert(branch_name.clone()) {
continue;
}
let branch_target = repo
.view()
.get_branch(branch_name)
.ok_or_else(|| user_error(format!("Branch {branch_name} doesn't exist")))?;
match classify_branch_update(branch_name, branch_target, &remote) {
let targets = TrackingRefPair {
local_target: repo.view().get_local_branch(branch_name),
remote_target: repo.view().get_remote_branch(branch_name, &remote),
};
if targets.local_target.is_absent() && targets.remote_target.is_absent() {
return Err(user_error(format!("Branch {branch_name} doesn't exist")));
}
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
Ok(None) => writeln!(
ui.stderr(),
Expand Down Expand Up @@ -777,8 +782,11 @@ fn cmd_git_push(
}
tx.mut_repo()
.set_local_branch_target(&branch_name, RefTarget::normal(commit.id().clone()));
let branch_target = tx.repo().view().get_branch(&branch_name).unwrap();
match classify_branch_update(&branch_name, branch_target, &remote) {
let targets = TrackingRefPair {
local_target: tx.repo().view().get_local_branch(&branch_name),
remote_target: tx.repo().view().get_remote_branch(&branch_name, &remote),
};
match classify_branch_update(&branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
Ok(None) => writeln!(
ui.stderr(),
Expand Down Expand Up @@ -814,19 +822,18 @@ fn cmd_git_push(
};
let branches_targeted = repo
.view()
.branches()
.iter()
.filter(|(_, branch_target)| {
let mut local_ids = branch_target.local_target.added_ids();
.local_remote_branches(&remote)
.filter(|(_, targets)| {
let mut local_ids = targets.local_target.added_ids();
local_ids.any(|id| revision_commit_ids.contains(id))
})
.collect_vec();
for &(branch_name, branch_target) in &branches_targeted {
if !seen_branches.insert(branch_name.clone()) {
for &(branch_name, targets) in &branches_targeted {
if !seen_branches.insert(branch_name.to_owned()) {
continue;
}
match classify_branch_update(branch_name, branch_target, &remote) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),
match classify_branch_update(branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)),
Ok(None) => {}
Err(message) => writeln!(ui.warning(), "{message}")?,
}
Expand Down Expand Up @@ -1010,10 +1017,10 @@ fn get_default_push_remote(

fn classify_branch_update(
branch_name: &str,
branch_target: &BranchTarget,
remote_name: &str,
targets: TrackingRefPair,
) -> Result<Option<BranchPushUpdate>, String> {
let push_action = classify_branch_push_action(branch_target, remote_name);
let push_action = classify_branch_push_action(targets);
match push_action {
BranchPushAction::AlreadyMatches => Ok(None),
BranchPushAction::LocalConflicted => Err(format!("Branch {branch_name} is conflicted")),
Expand Down
74 changes: 28 additions & 46 deletions lib/src/refs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use itertools::EitherOrBoth;
use crate::backend::CommitId;
use crate::index::Index;
use crate::merge::{trivial_merge, Merge};
use crate::op_store::{BranchTarget, RefTarget, RefTargetOptionExt};
use crate::op_store::RefTarget;

/// Compares `refs1` and `refs2` targets, yields entry if they differ.
///
Expand Down Expand Up @@ -136,12 +136,9 @@ pub enum BranchPushAction {

/// Figure out what changes (if any) need to be made to the remote when pushing
/// this branch.
pub fn classify_branch_push_action(
branch_target: &BranchTarget,
remote_name: &str,
) -> BranchPushAction {
let local_target = &branch_target.local_target;
let remote_target = branch_target.remote_targets.get(remote_name).flatten();
pub fn classify_branch_push_action(targets: TrackingRefPair) -> BranchPushAction {
let local_target = targets.local_target;
let remote_target = targets.remote_target;
if local_target == remote_target {
BranchPushAction::AlreadyMatches
} else if local_target.has_conflict() {
Expand All @@ -158,35 +155,31 @@ pub fn classify_branch_push_action(

#[cfg(test)]
mod tests {
use maplit::btreemap;

use super::*;
use crate::backend::ObjectId;

#[test]
fn test_classify_branch_push_action_unchanged() {
let commit_id1 = CommitId::from_hex("11");
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id1.clone()),
remote_targets: btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1),
},
let targets = TrackingRefPair {
local_target: &RefTarget::normal(commit_id1.clone()),
remote_target: &RefTarget::normal(commit_id1),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::AlreadyMatches
);
}

#[test]
fn test_classify_branch_push_action_added() {
let commit_id1 = CommitId::from_hex("11");
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id1.clone()),
remote_targets: btreemap! {},
let targets = TrackingRefPair {
local_target: &RefTarget::normal(commit_id1.clone()),
remote_target: RefTarget::absent_ref(),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::Update(BranchPushUpdate {
old_target: None,
new_target: Some(commit_id1),
Expand All @@ -197,14 +190,12 @@ mod tests {
#[test]
fn test_classify_branch_push_action_removed() {
let commit_id1 = CommitId::from_hex("11");
let branch = BranchTarget {
local_target: RefTarget::absent(),
remote_targets: btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1.clone()),
},
let targets = TrackingRefPair {
local_target: RefTarget::absent_ref(),
remote_target: &RefTarget::normal(commit_id1.clone()),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::Update(BranchPushUpdate {
old_target: Some(commit_id1),
new_target: None,
Expand All @@ -216,14 +207,12 @@ mod tests {
fn test_classify_branch_push_action_updated() {
let commit_id1 = CommitId::from_hex("11");
let commit_id2 = CommitId::from_hex("22");
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id2.clone()),
remote_targets: btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1.clone()),
},
let targets = TrackingRefPair {
local_target: &RefTarget::normal(commit_id2.clone()),
remote_target: &RefTarget::normal(commit_id1.clone()),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::Update(BranchPushUpdate {
old_target: Some(commit_id1),
new_target: Some(commit_id2),
Expand All @@ -235,14 +224,12 @@ mod tests {
fn test_classify_branch_push_action_local_conflicted() {
let commit_id1 = CommitId::from_hex("11");
let commit_id2 = CommitId::from_hex("22");
let branch = BranchTarget {
local_target: RefTarget::from_legacy_form([], [commit_id1.clone(), commit_id2]),
remote_targets: btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1),
},
let targets = TrackingRefPair {
local_target: &RefTarget::from_legacy_form([], [commit_id1.clone(), commit_id2]),
remote_target: &RefTarget::normal(commit_id1),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::LocalConflicted
);
}
Expand All @@ -251,17 +238,12 @@ mod tests {
fn test_classify_branch_push_action_remote_conflicted() {
let commit_id1 = CommitId::from_hex("11");
let commit_id2 = CommitId::from_hex("22");
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id1.clone()),
remote_targets: btreemap! {
"origin".to_string() => RefTarget::from_legacy_form(
[],
[commit_id1, commit_id2],
),
},
let targets = TrackingRefPair {
local_target: &RefTarget::normal(commit_id1.clone()),
remote_target: &RefTarget::from_legacy_form([], [commit_id1, commit_id2]),
};
assert_eq!(
classify_branch_push_action(&branch, "origin"),
classify_branch_push_action(targets),
BranchPushAction::RemoteConflicted
);
}
Expand Down

0 comments on commit 69a30b4

Please sign in to comment.