Skip to content

Commit

Permalink
git: on import_refs(), respect tracking state of existing remote refs
Browse files Browse the repository at this point in the history
In this commit, new behavior is tested by using in-memory view data. Data
persistence and track/untrack commands will be implemented soon.
  • Loading branch information
yuja committed Oct 15, 2023
1 parent 4fdafe3 commit 0b391f9
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 25 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj config set` now interprets the value as TOML also if it's a valid TOML
array or table. For example, `jj config set --user 'aliases.n' '["new"]'`

* Remote branches now have tracking or non-tracking flags. The
`git.auto-local-branch` setting is applied only to newly fetched remote
branches. Existing remote branches are migrated as follows:

* If local branch exists, the corresponding remote branches are considered
tracking branches.
* Otherwise, the remote branches are non-tracking branches.

See [automatic local branch creation](docs/config.md#automatic-local-branch-creation)
for details.

### New features

* `jj workspace add` now takes a `--revision` argument.
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,7 @@ fn cmd_git_push(
),
_ => user_error(err.to_string()),
})?;
// TODO: mark pushed remote branches as tracking
let stats = git::import_refs(tx.mut_repo(), &git_repo, &command.settings().git_settings())?;
print_git_import_stats(ui, &stats)?;
tx.finish(ui)?;
Expand Down
12 changes: 8 additions & 4 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ conflict is considered fully resolved when there are no conflict markers left.

### Automatic local branch creation

By default, when `jj` imports a remote-tracking branch from Git, it also
By default, when `jj` imports new remote-tracking branch from Git, it also
creates a local branch with the same name. In some repositories, this
may be undesirable, e.g.:

Expand All @@ -502,10 +502,14 @@ may be undesirable, e.g.:
- There are multiple remotes with conflicting views of that branch,
resulting in an unhelpful conflicted state.

You can disable this behavior by setting `git.auto-local-branch` like
so,
You can disable this behavior by setting `git.auto-local-branch` like so,

git.auto-local-branch = false
```toml
git.auto-local-branch = false
```

This setting is applied only to new remote branches. Existing remote branches
can be tracked individually.

Note that this setting may make it easier to accidentally delete remote
branches. Since the local branch isn't created, the remote branch will be
Expand Down
48 changes: 32 additions & 16 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ fn resolve_git_ref_to_commit_id(
struct RefsToImport {
/// Git ref `(full_name, new_target)`s to be copied to the view.
changed_git_refs: Vec<(String, RefTarget)>,
/// Remote `(ref_name, (old_target, new_target))`s to be merged in to the
/// local refs.
changed_remote_refs: BTreeMap<RefName, (RefTarget, RefTarget)>,
/// Remote `(ref_name, (old_remote_ref, new_target))`s to be merged in to
/// the local refs.
changed_remote_refs: BTreeMap<RefName, (RemoteRef, RefTarget)>,
}

/// Reflect changes made in the underlying Git repo in the Jujutsu repo.
Expand Down Expand Up @@ -260,23 +260,27 @@ pub fn import_some_refs(
for (full_name, new_target) in changed_git_refs {
mut_repo.set_git_ref_target(&full_name, new_target);
}
for (ref_name, (old_target, new_target)) in &changed_remote_refs {
for (ref_name, (old_remote_ref, new_target)) in &changed_remote_refs {
let base_target = old_remote_ref.tracking_target();
let new_remote_ref = RemoteRef {
target: new_target.clone(),
// TODO: preserve the old state
state: default_remote_ref_state_for(ref_name, git_settings),
state: if old_remote_ref.is_present() {
old_remote_ref.state
} else {
default_remote_ref_state_for(ref_name, git_settings)
},
};
if let RefName::RemoteBranch { branch, remote } = ref_name {
if new_remote_ref.is_tracking() {
let local_ref_name = RefName::LocalBranch(branch.clone());
mut_repo.merge_single_ref(&local_ref_name, old_target, &new_remote_ref.target);
mut_repo.merge_single_ref(&local_ref_name, base_target, &new_remote_ref.target);
}
// Remote-tracking branch is the last known state of the branch in the remote.
// It shouldn't diverge even if we had inconsistent view.
mut_repo.set_remote_branch(branch, remote, new_remote_ref);
} else {
if new_remote_ref.is_tracking() {
mut_repo.merge_single_ref(ref_name, old_target, &new_remote_ref.target);
mut_repo.merge_single_ref(ref_name, base_target, &new_remote_ref.target);
}
if let RefName::LocalBranch(branch) = ref_name {
// Update Git-tracking branch like the other remote branches.
Expand All @@ -289,7 +293,7 @@ pub fn import_some_refs(
// in jj as well.
let hidable_git_heads = changed_remote_refs
.values()
.flat_map(|(old_target, _)| old_target.added_ids())
.flat_map(|(old_remote_ref, _)| old_remote_ref.target.added_ids())
.cloned()
.collect_vec();
if hidable_git_heads.is_empty() {
Expand Down Expand Up @@ -341,7 +345,8 @@ fn diff_refs_to_import(
git_ref_filter(&ref_name).then_some((full_name.as_ref(), target))
})
.collect();
let mut known_remote_refs: HashMap<RefName, &RefTarget> = itertools::chain(
// TODO: migrate tags to the remote view, and don't destructure &RemoteRef
let mut known_remote_refs: HashMap<RefName, (&RefTarget, RemoteRefState)> = itertools::chain(
view.all_remote_branches()
.map(|((branch, remote), remote_ref)| {
// TODO: want to abstract local ref as "git" tracking remote, but
Expand All @@ -354,13 +359,14 @@ fn diff_refs_to_import(
remote: remote.to_owned(),
}
};
(ref_name, &remote_ref.target)
let RemoteRef { target, state } = remote_ref;
(ref_name, (target, *state))
}),
// TODO: compare to tags stored in the "git" remote view. Since tags should never
// be moved locally in jj, we can consider local tags as merge base.
view.tags().iter().map(|(name, target)| {
let ref_name = RefName::Tag(name.to_owned());
(ref_name, target)
(ref_name, (target, RemoteRefState::Tracking))
}),
)
.filter(|(ref_name, _)| git_ref_filter(ref_name))
Expand Down Expand Up @@ -395,16 +401,26 @@ fn diff_refs_to_import(
}
// TODO: Make it configurable which remotes are publishing and update public
// heads here.
let old_remote_target = known_remote_refs.remove(&ref_name).flatten();
let (old_remote_target, old_remote_state) = known_remote_refs
.remove(&ref_name)
.unwrap_or_else(|| (RefTarget::absent_ref(), RemoteRefState::New));
if new_target != *old_remote_target {
changed_remote_refs.insert(ref_name, (old_remote_target.clone(), new_target));
let old_remote_ref = RemoteRef {
target: old_remote_target.clone(),
state: old_remote_state,
};
changed_remote_refs.insert(ref_name, (old_remote_ref, new_target));
}
}
for full_name in known_git_refs.into_keys() {
changed_git_refs.push((full_name.to_owned(), RefTarget::absent()));
}
for (ref_name, old_target) in known_remote_refs {
changed_remote_refs.insert(ref_name, (old_target.clone(), RefTarget::absent()));
for (ref_name, (old_target, old_state)) in known_remote_refs {
let old_remote_ref = RemoteRef {
target: old_target.clone(),
state: old_state,
};
changed_remote_refs.insert(ref_name, (old_remote_ref, RefTarget::absent()));
}
Ok(RefsToImport {
changed_git_refs,
Expand Down
12 changes: 12 additions & 0 deletions lib/src/op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,18 @@ impl RemoteRef {
pub fn is_tracking(&self) -> bool {
self.state == RemoteRefState::Tracking
}

/// Target that should have been merged in to the local ref.
///
/// Use this as the base or known target when merging new remote ref in to
/// local or pushing local ref to remote.
pub fn tracking_target(&self) -> &RefTarget {
if self.is_tracking() {
&self.target
} else {
RefTarget::absent_ref()
}
}
}

/// Whether the ref is tracked or not.
Expand Down
64 changes: 59 additions & 5 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,15 +1365,15 @@ fn test_export_import_sequence() {
}

#[test]
fn test_import_export_no_auto_local_branch() {
fn test_import_export_non_tracking_branch() {
// Import a remote tracking branch and export it. We should not create a git
// branch.
let test_data = GitRepoData::create();
let git_settings = GitSettings {
let mut git_settings = GitSettings {
auto_local_branch: false,
};
let git_repo = test_data.git_repo;
let git_commit = empty_git_commit(&git_repo, "refs/remotes/origin/main", &[]);
let commit_main_t0 = empty_git_commit(&git_repo, "refs/remotes/origin/main", &[]);

let mut tx = test_data
.repo
Expand All @@ -1386,18 +1386,72 @@ fn test_import_export_no_auto_local_branch() {
assert_eq!(
mut_repo.view().get_remote_branch("main", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&git_commit)),
target: RefTarget::normal(jj_id(&commit_main_t0)),
state: RemoteRefState::New,
},
);
assert_eq!(
mut_repo.get_git_ref("refs/remotes/origin/main"),
RefTarget::normal(jj_id(&git_commit))
RefTarget::normal(jj_id(&commit_main_t0))
);

// Export the branch to git
assert_eq!(git::export_refs(mut_repo, &git_repo), Ok(vec![]));
assert_eq!(mut_repo.get_git_ref("refs/heads/main"), RefTarget::absent());

// Reimport with auto-local-branch on. Local branch shouldn't be created for
// the known branch "main".
let commit_main_t1 =
empty_git_commit(&git_repo, "refs/remotes/origin/main", &[&commit_main_t0]);
let commit_feat_t1 = empty_git_commit(&git_repo, "refs/remotes/origin/feat", &[]);
git_settings.auto_local_branch = true;
git::import_refs(mut_repo, &git_repo, &git_settings).unwrap();
assert!(mut_repo.view().get_local_branch("main").is_absent());
assert_eq!(
mut_repo.view().get_local_branch("feat"),
&RefTarget::normal(jj_id(&commit_feat_t1))
);
assert_eq!(
mut_repo.view().get_remote_branch("main", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_main_t1)),
state: RemoteRefState::New,
},
);
assert_eq!(
mut_repo.view().get_remote_branch("feat", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat_t1)),
state: RemoteRefState::Tracking,
},
);

// Reimport with auto-local-branch off. Tracking branch should be imported.
let commit_main_t2 =
empty_git_commit(&git_repo, "refs/remotes/origin/main", &[&commit_main_t1]);
let commit_feat_t2 =
empty_git_commit(&git_repo, "refs/remotes/origin/feat", &[&commit_feat_t1]);
git_settings.auto_local_branch = false;
git::import_refs(mut_repo, &git_repo, &git_settings).unwrap();
assert!(mut_repo.view().get_local_branch("main").is_absent());
assert_eq!(
mut_repo.view().get_local_branch("feat"),
&RefTarget::normal(jj_id(&commit_feat_t2))
);
assert_eq!(
mut_repo.view().get_remote_branch("main", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_main_t2)),
state: RemoteRefState::New,
},
);
assert_eq!(
mut_repo.view().get_remote_branch("feat", "origin"),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat_t2)),
state: RemoteRefState::Tracking,
},
);
}

#[test]
Expand Down

0 comments on commit 0b391f9

Please sign in to comment.