diff --git a/CHANGELOG.md b/CHANGELOG.md index f6bfa988f..dde5790f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The `author`/`committer` templates now display both name and email. Use `author.name()`/`committer.name()` to extract the name. +* Storage of the "HEAD@git" reference changed and can now have conflicts. + Operations written by a new `jj` binary will have a "HEAD@git" reference that + is not visible to older binaries. + ### New features * The default log format now uses the committer timestamp instead of the author diff --git a/lib/src/git.rs b/lib/src/git.rs index 22c2ad99a..cca2fd653 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -75,7 +75,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(); @@ -90,7 +90,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(); } diff --git a/lib/src/legacy_thrift_op_store.rs b/lib/src/legacy_thrift_op_store.rs index 527a90806..731991659 100644 --- a/lib/src/legacy_thrift_op_store.rs +++ b/lib/src/legacy_thrift_op_store.rs @@ -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 } diff --git a/lib/src/op_store.rs b/lib/src/op_store.rs index ad89b668c..7d891105d 100644 --- a/lib/src/op_store.rs +++ b/lib/src/op_store.rs @@ -199,7 +199,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, + pub git_head: Option, // 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). diff --git a/lib/src/proto_op_store.rs b/lib/src/proto_op_store.rs index bc500fe65..7ee768b69 100644 --- a/lib/src/proto_op_store.rs +++ b/lib/src/proto_op_store.rs @@ -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 @@ -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 diff --git a/lib/src/protos/op_store.proto b/lib/src/protos/op_store.proto index 60727418a..7edd263fd 100644 --- a/lib/src/protos/op_store.proto +++ b/lib/src/protos/op_store.proto @@ -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 { diff --git a/lib/src/protos/op_store.rs b/lib/src/protos/op_store.rs index 3ed9b1ae6..4a9f3d011 100644 --- a/lib/src/protos/op_store.rs +++ b/lib/src/protos/op_store.rs @@ -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, + /// 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, + pub git_head_legacy: ::prost::alloc::vec::Vec, + #[prost(message, optional, tag = "9")] + pub git_head: ::core::option::Option, } #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 008fb2872..69b5b840e 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -980,8 +980,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) { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 8374b01f5..6eeb92126 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -2039,7 +2039,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 { diff --git a/lib/src/simple_op_store.rs b/lib/src/simple_op_store.rs index e6600fdcb..3a4305fcb 100644 --- a/lib/src/simple_op_store.rs +++ b/lib/src/simple_op_store.rs @@ -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, @@ -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" ); } diff --git a/lib/src/view.rs b/lib/src/view.rs index 0fd052fbb..ce3843e79 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -84,8 +84,8 @@ impl View { &self.data.git_refs } - pub fn git_head(&self) -> Option { - 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) { @@ -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) { diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 9189dacdc..6b11a6cd8 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -161,7 +161,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] @@ -432,7 +432,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] diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 1eb86312c..45ca78074 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -1324,7 +1324,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()] diff --git a/lib/tests/test_view.rs b/lib/tests/test_view.rs index ffe30a3b2..04cf07633 100644 --- a/lib/tests/test_view.rs +++ b/lib/tests/test_view.rs @@ -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 tx0_head = write_random_commit(tx0.mut_repo(), &settings); + tx0.mut_repo() + .set_git_head(RefTarget::Normal(tx0_head.id().clone())); + let repo = tx0.commit(); + + let mut tx1 = repo.start_transaction(&settings, "test"); + let tx1_head = write_random_commit(tx1.mut_repo(), &settings); + tx1.mut_repo() + .set_git_head(RefTarget::Normal(tx1_head.id().clone())); + tx1.commit(); + + let mut tx2 = repo.start_transaction(&settings, "test"); + let tx2_head = write_random_commit(tx2.mut_repo(), &settings); + tx2.mut_repo() + .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) -> Arc { let repo_loader = txs[0].base_repo().loader(); let mut op_ids = vec![]; diff --git a/src/cli_util.rs b/src/cli_util.rs index 72f7d2da2..dd45e61e3 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -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::{self, OpHeadResolutionError, 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::{ CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, RepoLoader, RepoRef, @@ -507,40 +507,42 @@ impl WorkspaceCommandHelper { let mut tx = self.start_transaction("import git refs").into_inner(); git::import_refs(tx.mut_repo(), git_repo, &self.settings.git_settings())?; 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().to_owned(); - 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().to_owned(); + 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(()) @@ -567,7 +569,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 diff --git a/src/commands/mod.rs b/src/commands/mod.rs index b474864ef..e08cc9bcd 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -32,7 +32,7 @@ use jujutsu_lib::commit::Commit; use jujutsu_lib::dag_walk::topo_order_reverse; use jujutsu_lib::index::IndexEntry; use jujutsu_lib::matchers::EverythingMatcher; -use jujutsu_lib::op_store::WorkspaceId; +use jujutsu_lib::op_store::{RefTarget, WorkspaceId}; use jujutsu_lib::repo::ReadonlyRepo; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression}; @@ -985,7 +985,7 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(), &git_repo, &command.settings().git_settings(), )?; - 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.check_out(&git_head_commit)?; } diff --git a/src/templater.rs b/src/templater.rs index 4dea86b9a..fbf151502 100644 --- a/src/templater.rs +++ b/src/templater.rs @@ -362,10 +362,15 @@ impl TemplateProperty for GitHeadProperty<'_> { type Output = String; 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(), } } }