Skip to content

Commit

Permalink
git: allow conflicts in "HEAD@git"
Browse files Browse the repository at this point in the history
Git's HEAD ref is similar to other refs and can logically have
conflicts just like the other refs in `git_refs`. As with the other
refs, it can happen if you run concurrent commands importing two
different updates from Git. So let's treat `git_head` the same as
`git_refs` by making it an `Option<RefTarget>`.
  • Loading branch information
martinvonz committed Jan 10, 2023
1 parent 63ee611 commit 5db0d84
Show file tree
Hide file tree
Showing 16 changed files with 118 additions and 57 deletions.
4 changes: 2 additions & 2 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub fn import_refs(
.flat_map(|old_target| old_target.adds())
.collect_vec();
if let Some(old_git_head) = mut_repo.view().git_head() {
old_git_heads.push(old_git_head);
old_git_heads.extend(old_git_head.adds());
}

let mut new_git_heads = HashSet::new();
Expand All @@ -88,7 +88,7 @@ pub fn import_refs(
new_git_heads.insert(head_commit_id.clone());
prevent_gc(git_repo, &head_commit_id);
mut_repo.add_head(&head_commit);
mut_repo.set_git_head(head_commit_id);
mut_repo.set_git_head(RefTarget::Normal(head_commit_id));
} else {
mut_repo.clear_git_head();
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/legacy_thrift_op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl From<&simple_op_store_model::View> for View {
view.git_head = thrift_view
.git_head
.as_ref()
.map(|head| CommitId::new(head.clone()));
.map(|head| RefTarget::Normal(CommitId::new(head.clone())));

view
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ content_hash! {
/// The commit the Git HEAD points to.
// TODO: Support multiple Git worktrees?
// TODO: Do we want to store the current branch name too?
pub git_head: Option<CommitId>,
pub git_head: Option<RefTarget>,
// The commit that *should be* checked out in the workspace. Note that the working copy
// (.jj/working_copy/) has the source of truth about which commit *is* checked out (to be
// precise: the commit to which we most recently completed an update to).
Expand Down
9 changes: 6 additions & 3 deletions lib/src/proto_op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ fn view_to_proto(view: &View) -> crate::protos::op_store::View {
}

if let Some(git_head) = &view.git_head {
proto.git_head = git_head.to_bytes();
proto.git_head = Some(ref_target_to_proto(git_head));
}

proto
Expand Down Expand Up @@ -290,8 +290,11 @@ fn view_from_proto(proto: crate::protos::op_store::View) -> View {
}
}

if !proto.git_head.is_empty() {
view.git_head = Some(CommitId::new(proto.git_head));
#[allow(deprecated)]
if let Some(git_head) = proto.git_head.as_ref() {
view.git_head = Some(ref_target_from_proto(git_head.clone()));
} else if !proto.git_head_legacy.is_empty() {
view.git_head = Some(RefTarget::Normal(CommitId::new(proto.git_head_legacy)));
}

view
Expand Down
6 changes: 5 additions & 1 deletion lib/src/protos/op_store.proto
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,11 @@ message View {
repeated Tag tags = 6;
// Only a subset of the refs. For example, does not include refs/notes/.
repeated GitRef git_refs = 3;
bytes git_head = 7;
// This field is just for historical reasons (before we had the RefTarget
// type). New Views have (only) the target field.
// TODO: Delete support for the old format.
bytes git_head_legacy = 7 [deprecated = true];
RefTarget git_head = 9;
}

message Operation {
Expand Down
8 changes: 7 additions & 1 deletion lib/src/protos/op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,14 @@ pub struct View {
/// Only a subset of the refs. For example, does not include refs/notes/.
#[prost(message, repeated, tag = "3")]
pub git_refs: ::prost::alloc::vec::Vec<GitRef>,
/// This field is just for historical reasons (before we had the RefTarget
/// type). New Views have (only) the target field.
/// TODO: Delete support for the old format.
#[deprecated]
#[prost(bytes = "vec", tag = "7")]
pub git_head: ::prost::alloc::vec::Vec<u8>,
pub git_head_legacy: ::prost::alloc::vec::Vec<u8>,
#[prost(message, optional, tag = "9")]
pub git_head: ::core::option::Option<RefTarget>,
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
Expand Down
4 changes: 2 additions & 2 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,8 @@ impl MutableRepo {
self.view_mut().remove_git_ref(name);
}

pub fn set_git_head(&mut self, head_id: CommitId) {
self.view_mut().set_git_head(head_id);
pub fn set_git_head(&mut self, target: RefTarget) {
self.view_mut().set_git_head(target);
}

pub fn clear_git_head(&mut self) {
Expand Down
5 changes: 4 additions & 1 deletion lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1989,7 +1989,10 @@ pub fn evaluate_expression<'repo>(
Ok(revset_for_commit_ids(repo, &commit_ids))
}
RevsetExpression::GitHead => {
let commit_ids = repo.view().git_head().into_iter().collect_vec();
let mut commit_ids = vec![];
if let Some(ref_target) = repo.view().git_head() {
commit_ids.extend(ref_target.adds());
}
Ok(revset_for_commit_ids(repo, &commit_ids))
}
RevsetExpression::Filter(predicate) => Ok(Box::new(FilterRevset {
Expand Down
4 changes: 2 additions & 2 deletions lib/src/simple_op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ mod tests {
"refs/heads/main".to_string() => git_refs_main_target,
"refs/heads/feature".to_string() => git_refs_feature_target
},
git_head: Some(CommitId::from_hex("fff111")),
git_head: Some(RefTarget::Normal(CommitId::from_hex("fff111"))),
wc_commit_ids: hashmap! {
WorkspaceId::default() => default_wc_commit_id,
WorkspaceId::new("test".to_string()) => test_wc_commit_id,
Expand Down Expand Up @@ -244,7 +244,7 @@ mod tests {
// Test exact output so we detect regressions in compatibility
assert_snapshot!(
ViewId::new(blake2b_hash(&create_view()).to_vec()).hex(),
@"2a026b6a091219a3d8ca43d822984cf9be0c53438225d76a5ba5e6d3724fab15104579fb08fa949977c4357b1806d240bef28d958cbcd7d786962ac88c15df31"
@"7f47fa81494d7189cb1827b83b3f834662f0f61b4c4090298067e85cdc60f773bf639c4e6a3554a4e401650218ca240291ce591f45a1c501ade1d2b9f97e1a37"
);
}

Expand Down
8 changes: 4 additions & 4 deletions lib/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ impl View {
&self.data.git_refs
}

pub fn git_head(&self) -> Option<CommitId> {
self.data.git_head.clone()
pub fn git_head(&self) -> Option<&RefTarget> {
self.data.git_head.as_ref()
}

pub fn set_wc_commit(&mut self, workspace_id: WorkspaceId, commit_id: CommitId) {
Expand Down Expand Up @@ -244,8 +244,8 @@ impl View {
self.data.git_refs.remove(name);
}

pub fn set_git_head(&mut self, head_id: CommitId) {
self.data.git_head = Some(head_id);
pub fn set_git_head(&mut self, target: RefTarget) {
self.data.git_head = Some(target);
}

pub fn clear_git_head(&mut self) {
Expand Down
7 changes: 5 additions & 2 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ fn test_import_refs() {
view.git_refs().get("refs/tags/v1.0"),
Some(RefTarget::Normal(jj_id(&commit5))).as_ref()
);
assert_eq!(view.git_head(), Some(jj_id(&commit2)));
assert_eq!(view.git_head(), Some(&RefTarget::Normal(jj_id(&commit2))));
}

#[test]
Expand Down Expand Up @@ -409,7 +409,10 @@ fn test_import_refs_detached_head() {
let expected_heads = hashset! { jj_id(&commit1) };
assert_eq!(*repo.view().heads(), expected_heads);
assert_eq!(repo.view().git_refs().len(), 0);
assert_eq!(repo.view().git_head(), Some(jj_id(&commit1)));
assert_eq!(
repo.view().git_head(),
Some(&RefTarget::Normal(jj_id(&commit1)))
);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,7 @@ fn test_evaluate_expression_git_head(use_git: bool) {
resolve_commit_ids(mut_repo.as_repo_ref(), "git_head()"),
vec![]
);
mut_repo.set_git_head(commit1.id().clone());
mut_repo.set_git_head(RefTarget::Normal(commit1.id().clone()));
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), "git_head()"),
vec![commit1.id().clone()]
Expand Down
35 changes: 35 additions & 0 deletions lib/tests/test_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,41 @@ fn test_merge_views_git_refs() {
);
}

#[test]
fn test_merge_views_git_heads() {
// Tests merging of git heads (by performing concurrent operations). See
// test_refs.rs for tests of merging of individual ref targets.
let settings = testutils::user_settings();
let test_repo = TestRepo::init(false);
let repo = &test_repo.repo;

let mut tx0 = repo.start_transaction(&settings, "test");
let mut_repo0 = tx0.mut_repo();
let tx0_head = create_random_commit(mut_repo0, &settings).write().unwrap();
mut_repo0.set_git_head(RefTarget::Normal(tx0_head.id().clone()));
let repo = tx0.commit();

let mut tx1 = repo.start_transaction(&settings, "test");
let mut_repo1 = tx1.mut_repo();
let tx1_head = create_random_commit(mut_repo1, &settings).write().unwrap();
mut_repo1.set_git_head(RefTarget::Normal(tx1_head.id().clone()));
tx1.commit();

let mut tx2 = repo.start_transaction(&settings, "test");
let mut_repo2 = tx2.mut_repo();
let tx2_head = create_random_commit(mut_repo2, &settings).write().unwrap();
mut_repo2.set_git_head(RefTarget::Normal(tx2_head.id().clone()));
tx2.commit();

let repo = repo.reload_at_head(&settings).unwrap();
let expected_git_head = RefTarget::Conflict {
removes: vec![tx0_head.id().clone()],
adds: vec![tx1_head.id().clone(), tx2_head.id().clone()],
};
// TODO: Should be equal
assert_ne!(repo.view().git_head(), Some(&expected_git_head));
}

fn commit_transactions(settings: &UserSettings, txs: Vec<Transaction>) -> Arc<ReadonlyRepo> {
let repo_loader = txs[0].base_repo().loader();
let mut op_ids = vec![];
Expand Down
64 changes: 33 additions & 31 deletions src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use jujutsu_lib::git::{GitExportError, GitImportError};
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
use jujutsu_lib::op_heads_store::{OpHeadResolutionError, OpHeads, OpHeadsStore};
use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, WorkspaceId};
use jujutsu_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget, WorkspaceId};
use jujutsu_lib::operation::Operation;
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, RepoRef, RewriteRootCommit, StoreFactories};
use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
Expand Down Expand Up @@ -476,40 +476,42 @@ impl WorkspaceCommandHelper {
let mut tx = self.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), git_repo)?;
if tx.mut_repo().has_changes() {
let old_git_head = self.repo.view().git_head();
let new_git_head = tx.mut_repo().view().git_head();
let old_git_head = self.repo.view().git_head().cloned();
let new_git_head = tx.mut_repo().view().git_head().cloned();
// If the Git HEAD has changed, abandon our old checkout and check out the new
// Git HEAD.
if new_git_head != old_git_head && new_git_head.is_some() {
let workspace_id = self.workspace_id();
let mut locked_working_copy = self.workspace.working_copy_mut().start_mutation();
if let Some(old_wc_commit_id) = self.repo.view().get_wc_commit_id(&workspace_id) {
match new_git_head {
Some(RefTarget::Normal(new_git_head_id)) if new_git_head != old_git_head => {
let workspace_id = self.workspace_id();
let mut locked_working_copy =
self.workspace.working_copy_mut().start_mutation();
if let Some(old_wc_commit_id) = self.repo.view().get_wc_commit_id(&workspace_id)
{
tx.mut_repo()
.record_abandoned_commit(old_wc_commit_id.clone());
}
let new_git_head_commit = tx.mut_repo().store().get_commit(&new_git_head_id)?;
tx.mut_repo()
.record_abandoned_commit(old_wc_commit_id.clone());
.check_out(workspace_id, &self.settings, &new_git_head_commit)?;
// The working copy was presumably updated by the git command that updated
// HEAD, so we just need to reset our working copy
// state to it without updating working copy files.
locked_working_copy.reset(&new_git_head_commit.tree())?;
tx.mut_repo().rebase_descendants(&self.settings)?;
self.repo = tx.commit();
locked_working_copy.finish(self.repo.op_id().clone());
}
let new_checkout = self
.repo
.store()
.get_commit(new_git_head.as_ref().unwrap())?;
tx.mut_repo()
.check_out(workspace_id, &self.settings, &new_checkout)?;
// The working copy was presumably updated by the git command that updated HEAD,
// so we just need to reset our working copy state to it without updating
// working copy files.
locked_working_copy.reset(&new_checkout.tree())?;
tx.mut_repo().rebase_descendants(&self.settings)?;
self.repo = tx.commit();
locked_working_copy.finish(self.repo.op_id().clone());
} else {
let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?;
if num_rebased > 0 {
writeln!(
ui,
"Rebased {num_rebased} descendant commits off of commits rewritten from \
git"
)?;
_ => {
let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?;
if num_rebased > 0 {
writeln!(
ui,
"Rebased {num_rebased} descendant commits off of commits rewritten \
from git"
)?;
}
self.finish_transaction(ui, tx)?;
}
self.finish_transaction(ui, tx)?;
}
}
Ok(())
Expand All @@ -536,7 +538,7 @@ impl WorkspaceCommandHelper {
let new_git_commit_id = Oid::from_bytes(first_parent_id.as_bytes()).unwrap();
let new_git_commit = git_repo.find_commit(new_git_commit_id)?;
git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?;
mut_repo.set_git_head(first_parent_id);
mut_repo.set_git_head(RefTarget::Normal(first_parent_id));
}
} else {
// The workspace was removed (maybe the user undid the
Expand Down
2 changes: 1 addition & 1 deletion src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,7 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
} else {
let mut tx = workspace_command.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo)?;
if let Some(git_head_id) = tx.mut_repo().view().git_head() {
if let Some(RefTarget::Normal(git_head_id)) = tx.mut_repo().view().git_head().cloned() {
let git_head_commit = tx.mut_repo().store().get_commit(&git_head_id)?;
tx.mut_repo().check_out(
workspace_command.workspace_id(),
Expand Down
13 changes: 9 additions & 4 deletions src/templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,15 @@ impl<'a> GitHeadProperty<'a> {

impl TemplateProperty<Commit, String> for GitHeadProperty<'_> {
fn extract(&self, context: &Commit) -> String {
if self.repo.view().git_head().as_ref() == Some(context.id()) {
"HEAD@git".to_string()
} else {
"".to_string()
match self.repo.view().git_head() {
Some(ref_target) if ref_target.has_add(context.id()) => {
if ref_target.is_conflict() {
"HEAD@git?".to_string()
} else {
"HEAD@git".to_string()
}
}
_ => "".to_string(),
}
}
}
Expand Down

0 comments on commit 5db0d84

Please sign in to comment.