view: add support for conflicting git refs in the model

This adds support for having conflicting git refs in the view, but we
never create conflicts yet. The `git_refs()` revset includes all "add"
sides of any conflicts. Similarly `origin/main` (for example) resolves
to all "adds" if it's conflicted (meaning that `jj co origin/main` and
many other commands will error out if `origin/main` is
conflicted). The `git_refs` template renders the reference for all
"adds" and adds a "?" as suffix for conflicted refs.

The reason I'm adding this now is not because it's high priority on
its own (it's likely extremely uncommon to run two concurrent `jj git
refresh` and *also* update refs in the underlying git repo at the same
time) but because it's a building block for the branch support I've
planned (issue libfuse#21).

lib/protos/op_store.proto

@@ -14,11 +14,25 @@
 
 syntax = "proto3";
 
+message RefConflict {
+  repeated bytes removes = 1;
+  repeated bytes adds = 2;
+}
+
+message RefTarget {
+  oneof value {
+    bytes commit_id = 1;
+    RefConflict conflict = 2;
+  }
+}
+
 message GitRef {
   string name = 1;
-  // Always a commit id. Refs pointing to a non-commit object are not
-  // included.
+  // This field is just for historical reasons (before we had the RefTarget
+  // type). New GitRefs have (only) the target field.
+  // TODO: Delete support for the old format.
   bytes commit_id = 2;
+  RefTarget target = 3;
 }
 
 message View {

lib/src/git.rs

@@ -16,6 +16,7 @@ use itertools::Itertools;
 use thiserror::Error;
 
 use crate::commit::Commit;
+use crate::op_store::RefTarget;
 use crate::repo::MutableRepo;
 use crate::store::CommitId;
 
@@ -61,7 +62,7 @@ pub fn import_refs(
         let id = CommitId(git_commit.id().as_bytes().to_vec());
         let commit = store.get_commit(&id).unwrap();
         mut_repo.add_head(&commit);
-        mut_repo.insert_git_ref(git_ref.name().unwrap().to_string(), id);
+        mut_repo.insert_git_ref(git_ref.name().unwrap().to_string(), RefTarget::Normal(id));
         // For now, we consider all remotes "publishing".
         // TODO: Make it configurable which remotes are publishing.
         if git_ref.is_remote() {

lib/src/op_store.rs

@@ -47,6 +47,37 @@ impl OperationId {
     }
 }
 
+#[derive(PartialEq, Eq, Clone, Debug)]
+pub enum RefTarget {
+    Normal(CommitId),
+    Conflict {
+        removes: Vec<CommitId>,
+        adds: Vec<CommitId>,
+    },
+}
+
+impl RefTarget {
+    pub fn is_conflict(&self) -> bool {
+        matches!(self, RefTarget::Conflict { .. })
+    }
+
+    pub fn adds(&self) -> Vec<CommitId> {
+        match self {
+            RefTarget::Normal(id) => {
+                vec![id.clone()]
+            }
+            RefTarget::Conflict { removes: _, adds } => adds.clone(),
+        }
+    }
+
+    pub fn has_add(&self, needle: &CommitId) -> bool {
+        match self {
+            RefTarget::Normal(id) => id == needle,
+            RefTarget::Conflict { removes: _, adds } => adds.contains(needle),
+        }
+    }
+}
+
 /// Represents the way the repo looks at a given time, just like how a Tree
 /// object represents how the file system looks at a given time.
 #[derive(Clone)]
@@ -55,7 +86,7 @@ pub struct View {
     pub head_ids: HashSet<CommitId>,
     /// Heads of the set of public commits.
     pub public_head_ids: HashSet<CommitId>,
-    pub git_refs: BTreeMap<String, CommitId>,
+    pub git_refs: BTreeMap<String, RefTarget>,
     // The commit that *should be* checked out in the (default) working copy. 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 a checkout to).

lib/src/repo.rs

@@ -31,7 +31,7 @@ use crate::index::{IndexRef, MutableIndex, ReadonlyIndex};
 use crate::index_store::IndexStore;
 use crate::local_store::LocalStore;
 use crate::op_heads_store::OpHeadsStore;
-use crate::op_store::{OpStore, OperationId};
+use crate::op_store::{OpStore, OperationId, RefTarget};
 use crate::operation::Operation;
 use crate::settings::{RepoSettings, UserSettings};
 use crate::simple_op_store::SimpleOpStore;
@@ -664,7 +664,17 @@ impl MutableRepo {
             .cloned()
             .collect();
         view.head_ids.extend(view.public_head_ids.iter().cloned());
-        view.head_ids.extend(view.git_refs.values().cloned());
+        for ref_target in view.git_refs.values() {
+            match ref_target {
+                RefTarget::Normal(id) => {
+                    view.head_ids.insert(id.clone());
+                }
+                RefTarget::Conflict { removes, adds } => {
+                    view.head_ids.extend(removes.iter().cloned());
+                    view.head_ids.extend(adds.iter().cloned());
+                }
+            }
+        }
         view.head_ids = self
             .index
             .heads(view.head_ids.iter())
@@ -731,8 +741,8 @@ impl MutableRepo {
         self.invalidate_evolution();
     }
 
-    pub fn insert_git_ref(&mut self, name: String, commit_id: CommitId) {
-        self.view.insert_git_ref(name, commit_id);
+    pub fn insert_git_ref(&mut self, name: String, target: RefTarget) {
+        self.view.insert_git_ref(name, target);
     }
 
     pub fn remove_git_ref(&mut self, name: &str) {

lib/src/revset.rs

@@ -44,8 +44,8 @@ pub enum RevsetError {
 fn resolve_git_ref(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, RevsetError> {
     let view = repo.view();
     for git_ref_prefix in &["", "refs/", "refs/heads/", "refs/tags/", "refs/remotes/"] {
-        if let Some(commit_id) = view.git_refs().get(&(git_ref_prefix.to_string() + symbol)) {
-            return Ok(vec![commit_id.clone()]);
+        if let Some(ref_target) = view.git_refs().get(&(git_ref_prefix.to_string() + symbol)) {
+            return Ok(ref_target.adds());
         }
     }
     Err(RevsetError::NoSuchRevision(symbol.to_owned()))
@@ -942,12 +942,12 @@ pub fn evaluate_expression<'repo>(
         }
         RevsetExpression::GitRefs => {
             let index = repo.index();
-            let mut index_entries = repo
-                .view()
-                .git_refs()
-                .values()
-                .map(|id| index.entry_by_id(id).unwrap())
-                .collect_vec();
+            let mut index_entries = vec![];
+            for ref_target in repo.view().git_refs().values() {
+                for id in ref_target.adds() {
+                    index_entries.push(index.entry_by_id(&id).unwrap());
+                }
+            }
             index_entries.sort_by_key(|b| Reverse(b.position()));
             index_entries.dedup();
             Ok(Box::new(EagerRevset { index_entries }))

lib/src/simple_op_store.rs

@@ -19,12 +19,14 @@ use std::io::{ErrorKind, Write};
 use std::path::PathBuf;
 
 use blake2::{Blake2b, Digest};
+use itertools::Itertools;
 use protobuf::{Message, ProtobufError};
 use tempfile::{NamedTempFile, PersistError};
 
 use crate::file_util::persist_content_addressed_temp_file;
 use crate::op_store::{
-    OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, View, ViewId,
+    OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, RefTarget,
+    View, ViewId,
 };
 use crate::store::{CommitId, MillisSinceEpoch, Timestamp};
 
@@ -204,10 +206,10 @@ fn view_to_proto(view: &View) -> crate::protos::op_store::View {
     for head_id in &view.public_head_ids {
         proto.public_head_ids.push(head_id.0.clone());
     }
-    for (git_ref_name, commit_id) in &view.git_refs {
+    for (git_ref_name, target) in &view.git_refs {
         let mut git_ref_proto = crate::protos::op_store::GitRef::new();
         git_ref_proto.set_name(git_ref_name.clone());
-        git_ref_proto.set_commit_id(commit_id.0.clone());
+        git_ref_proto.set_target(ref_target_to_proto(target));
         proto.git_refs.push(git_ref_proto);
     }
     proto
@@ -223,8 +225,59 @@ fn view_from_proto(proto: &crate::protos::op_store::View) -> View {
             .insert(CommitId(head_id_bytes.to_vec()));
     }
     for git_ref in proto.git_refs.iter() {
-        view.git_refs
-            .insert(git_ref.name.clone(), CommitId(git_ref.commit_id.to_vec()));
+        if git_ref.has_target() {
+            view.git_refs.insert(
+                git_ref.name.clone(),
+                ref_target_from_proto(git_ref.target.as_ref().unwrap()),
+            );
+        } else {
+            // Legacy format
+            view.git_refs.insert(
+                git_ref.name.clone(),
+                RefTarget::Normal(CommitId(git_ref.commit_id.to_vec())),
+            );
+        }
     }
     view
 }
+
+fn ref_target_to_proto(value: &RefTarget) -> crate::protos::op_store::RefTarget {
+    let mut proto = crate::protos::op_store::RefTarget::new();
+    match value {
+        RefTarget::Normal(id) => {
+            proto.set_commit_id(id.0.clone());
+        }
+        RefTarget::Conflict { removes, adds } => {
+            let mut ref_conflict_proto = crate::protos::op_store::RefConflict::new();
+            for id in removes {
+                ref_conflict_proto.removes.push(id.0.clone());
+            }
+            for id in adds {
+                ref_conflict_proto.adds.push(id.0.clone());
+            }
+            proto.set_conflict(ref_conflict_proto);
+        }
+    }
+    proto
+}
+
+fn ref_target_from_proto(proto: &crate::protos::op_store::RefTarget) -> RefTarget {
+    match proto.value.as_ref().unwrap() {
+        crate::protos::op_store::RefTarget_oneof_value::commit_id(id) => {
+            RefTarget::Normal(CommitId(id.to_vec()))
+        }
+        crate::protos::op_store::RefTarget_oneof_value::conflict(conflict) => {
+            let removes = conflict
+                .removes
+                .iter()
+                .map(|id_bytes| CommitId(id_bytes.to_vec()))
+                .collect_vec();
+            let adds = conflict
+                .adds
+                .iter()
+                .map(|id_bytes| CommitId(id_bytes.to_vec()))
+                .collect_vec();
+            RefTarget::Conflict { removes, adds }
+        }
+    }
+}

lib/src/view.rs

@@ -15,6 +15,7 @@
 use std::collections::{BTreeMap, HashSet};
 
 use crate::op_store;
+use crate::op_store::RefTarget;
 use crate::store::CommitId;
 
 pub struct View {
@@ -47,7 +48,7 @@ impl View {
         &self.data.public_head_ids
     }
 
-    pub fn git_refs(&self) -> &BTreeMap<String, CommitId> {
+    pub fn git_refs(&self) -> &BTreeMap<String, RefTarget> {
         &self.data.git_refs
     }
 
@@ -71,8 +72,8 @@ impl View {
         self.data.public_head_ids.remove(head_id);
     }
 
-    pub fn insert_git_ref(&mut self, name: String, commit_id: CommitId) {
-        self.data.git_refs.insert(name, commit_id);
+    pub fn insert_git_ref(&mut self, name: String, target: RefTarget) {
+        self.data.git_refs.insert(name, target);
     }
 
     pub fn remove_git_ref(&mut self, name: &str) {
@@ -122,17 +123,14 @@ impl View {
         let base_git_ref_names: HashSet<_> = base.git_refs().keys().clone().collect();
         let other_git_ref_names: HashSet<_> = other.git_refs().keys().clone().collect();
         for maybe_modified_git_ref_name in other_git_ref_names.intersection(&base_git_ref_names) {
-            let base_commit_id = base.git_refs().get(*maybe_modified_git_ref_name).unwrap();
-            let other_commit_id = other.git_refs().get(*maybe_modified_git_ref_name).unwrap();
-            if base_commit_id == other_commit_id {
+            let base_target = base.git_refs().get(*maybe_modified_git_ref_name).unwrap();
+            let other_target = other.git_refs().get(*maybe_modified_git_ref_name).unwrap();
+            if base_target == other_target {
                 continue;
             }
             // TODO: Handle modify/modify conflict (i.e. if self and base are different
             // here)
-            self.insert_git_ref(
-                (*maybe_modified_git_ref_name).clone(),
-                other_commit_id.clone(),
-            );
+            self.insert_git_ref((*maybe_modified_git_ref_name).clone(), other_target.clone());
         }
         for added_git_ref_name in other_git_ref_names.difference(&base_git_ref_names) {
             // TODO: Handle add/add conflict (i.e. if self also has the ref here)

lib/tests/test_git.rs

@@ -19,6 +19,7 @@ use std::sync::Arc;
 use git2::Oid;
 use jujutsu_lib::commit::Commit;
 use jujutsu_lib::git::{GitFetchError, GitPushError};
+use jujutsu_lib::op_store::RefTarget;
 use jujutsu_lib::repo::ReadonlyRepo;
 use jujutsu_lib::settings::UserSettings;
 use jujutsu_lib::store::CommitId;
@@ -84,19 +85,19 @@ fn test_import_refs() {
     assert_eq!(view.git_refs().len(), 4);
     assert_eq!(
         view.git_refs().get("refs/heads/main"),
-        Some(commit_id(&commit2)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit2))).as_ref()
     );
     assert_eq!(
         view.git_refs().get("refs/heads/feature1"),
-        Some(commit_id(&commit3)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit3))).as_ref()
     );
     assert_eq!(
         view.git_refs().get("refs/heads/feature2"),
-        Some(commit_id(&commit4)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit4))).as_ref()
     );
     assert_eq!(
         view.git_refs().get("refs/remotes/origin/main"),
-        Some(commit_id(&commit5)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit5))).as_ref()
     );
     tx.discard();
 }
@@ -144,11 +145,11 @@ fn test_import_refs_reimport() {
     assert_eq!(view.git_refs().len(), 2);
     assert_eq!(
         view.git_refs().get("refs/heads/main"),
-        Some(commit_id(&commit2)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit2))).as_ref()
     );
     assert_eq!(
         view.git_refs().get("refs/heads/feature2"),
-        Some(commit_id(&commit5)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit5))).as_ref()
     );
     tx.discard();
 }
@@ -235,11 +236,11 @@ fn test_import_refs_merge() {
     assert_eq!(git_refs.len(), 9);
     assert_eq!(
         git_refs.get("refs/heads/sideways-unchanged"),
-        Some(commit_id(&commit4)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit4))).as_ref()
     );
     assert_eq!(
         git_refs.get("refs/heads/unchanged-sideways"),
-        Some(commit_id(&commit4)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit4))).as_ref()
     );
     assert_eq!(git_refs.get("refs/heads/remove-unchanged"), None);
     assert_eq!(git_refs.get("refs/heads/unchanged-remove"), None);
@@ -248,22 +249,22 @@ fn test_import_refs_merge() {
     // let the later operation overwrite.)
     assert_eq!(
         git_refs.get("refs/heads/forward-forward"),
-        Some(commit_id(&commit3)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit3))).as_ref()
     );
     // TODO: The rest of these should be conflicts (however we decide to represent
     // that).
     assert_eq!(
         git_refs.get("refs/heads/sideways-sideways"),
-        Some(commit_id(&commit5)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit5))).as_ref()
     );
     assert_eq!(git_refs.get("refs/heads/forward-remove"), None);
     assert_eq!(
         git_refs.get("refs/heads/remove-forward"),
-        Some(commit_id(&commit2)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit2))).as_ref()
     );
     assert_eq!(
         git_refs.get("refs/heads/add-add"),
-        Some(commit_id(&commit4)).as_ref()
+        Some(RefTarget::Normal(commit_id(&commit4))).as_ref()
     );
 }