revsets: resolve symbol as change id if nothing else matches

This patch makes it so we attempt to resolve a symbol as the
non-obsolete commits in a change id if all other resolutions
fail.

This addresses issue libfuse#15. I decided to not require any operator for
looking up by change id. I want to make it as easy as possible to use
change ids instead of commit ids to see how well it works to interact
mostly with change ids instead of commit ids (I'll try to test that by
using it myself).

lib/src/evolution.rs

@@ -18,7 +18,7 @@ use std::sync::Arc;
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
 use crate::dag_walk::{bfs, closest_common_node, leaves};
-use crate::index::IndexPosition;
+use crate::index::{HexPrefix, IndexPosition, PrefixResolution};
 use crate::repo::{MutableRepo, ReadonlyRepo, RepoRef};
 use crate::repo_path::RepoPath;
 use crate::rewrite::{merge_commit_trees, rebase_commit};
@@ -139,6 +139,28 @@ impl State {
             .map_or(false, |non_obsoletes| non_obsoletes.len() > 1)
     }
 
+    // TODO: We should probably add a change id table to the commit index and move
+    // this there
+    fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
+        let mut result = PrefixResolution::NoMatch;
+        for change_id in self.non_obsoletes_by_changeid.keys() {
+            if change_id.hex().starts_with(prefix.hex()) {
+                if result != PrefixResolution::NoMatch {
+                    return PrefixResolution::AmbiguousMatch;
+                }
+                result = PrefixResolution::SingleMatch(change_id.clone());
+            }
+        }
+        result
+    }
+
+    fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
+        self.non_obsoletes_by_changeid
+            .get(change_id)
+            .cloned()
+            .unwrap_or_else(HashSet::new)
+    }
+
     fn add_commit(&mut self, commit: &Commit) {
         self.add_commit_data(
             commit.id(),
@@ -339,13 +361,27 @@ impl EvolutionRef<'_> {
         }
     }
 
+    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
+        match self {
+            EvolutionRef::Readonly(evolution) => evolution.resolve_change_id_prefix(prefix),
+            EvolutionRef::Mutable(evolution) => evolution.resolve_change_id_prefix(prefix),
+        }
+    }
+
     pub fn is_divergent(&self, change_id: &ChangeId) -> bool {
         match self {
             EvolutionRef::Readonly(evolution) => evolution.is_divergent(change_id),
             EvolutionRef::Mutable(evolution) => evolution.is_divergent(change_id),
         }
     }
 
+    pub fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
+        match self {
+            EvolutionRef::Readonly(evolution) => evolution.non_obsoletes(change_id),
+            EvolutionRef::Mutable(evolution) => evolution.non_obsoletes(change_id),
+        }
+    }
+
     /// Given a current parent, finds the new parent candidates. If the current
     /// parent is not obsolete, then a singleton set of that commit will be
     /// returned.
@@ -402,6 +438,14 @@ impl ReadonlyEvolution {
         self.state.is_divergent(change_id)
     }
 
+    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
+        self.state.resolve_change_id_prefix(prefix)
+    }
+
+    pub fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
+        self.state.non_obsoletes(change_id)
+    }
+
     pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet<CommitId> {
         self.state.new_parent(repo, old_parent_id)
     }
@@ -434,6 +478,14 @@ impl MutableEvolution {
         self.state.is_divergent(change_id)
     }
 
+    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
+        self.state.resolve_change_id_prefix(prefix)
+    }
+
+    pub fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
+        self.state.non_obsoletes(change_id)
+    }
+
     pub fn new_parent(&self, repo: RepoRef, old_parent_id: &CommitId) -> HashSet<CommitId> {
         self.state.new_parent(repo, old_parent_id)
     }

lib/src/revset.rs

@@ -34,6 +34,8 @@ pub enum RevsetError {
     NoSuchRevision(String),
     #[error("Commit id prefix \"{0}\" is ambiguous")]
     AmbiguousCommitIdPrefix(String),
+    #[error("Change id prefix \"{0}\" is ambiguous")]
+    AmbiguousChangeIdPrefix(String),
     #[error("Unexpected error from store: {0}")]
     StoreError(#[from] StoreError),
 }
@@ -74,8 +76,29 @@ fn resolve_commit_id(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, Revse
     Err(RevsetError::NoSuchRevision(symbol.to_owned()))
 }
 
+fn resolve_non_obsolete_change_id(
+    repo: RepoRef,
+    change_id_prefix: &str,
+) -> Result<Vec<CommitId>, RevsetError> {
+    if let Some(hex_prefix) = HexPrefix::new(change_id_prefix.to_owned()) {
+        let evolution = repo.evolution();
+        match evolution.resolve_change_id_prefix(&hex_prefix) {
+            PrefixResolution::NoMatch => {
+                Err(RevsetError::NoSuchRevision(change_id_prefix.to_owned()))
+            }
+            PrefixResolution::AmbiguousMatch => Err(RevsetError::AmbiguousChangeIdPrefix(
+                change_id_prefix.to_owned(),
+            )),
+            PrefixResolution::SingleMatch(change_id) => {
+                Ok(evolution.non_obsoletes(&change_id).into_iter().collect())
+            }
+        }
+    } else {
+        Err(RevsetError::NoSuchRevision(change_id_prefix.to_owned()))
+    }
+}
+
 pub fn resolve_symbol(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, RevsetError> {
-    // TODO: Support change ids.
     if symbol == "@" {
         Ok(vec![repo.view().checkout().clone()])
     } else if symbol == "root" {
@@ -93,6 +116,12 @@ pub fn resolve_symbol(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, Revs
             return commit_id_result;
         }
 
+        // Try to resolve as a change id (the non-obsolete commits in the change).
+        let change_id_result = resolve_non_obsolete_change_id(repo, symbol);
+        if !matches!(change_id_result, Err(RevsetError::NoSuchRevision(_))) {
+            return change_id_result;
+        }
+
         Err(RevsetError::NoSuchRevision(symbol.to_owned()))
     }
 }

lib/tests/test_revset.rs

@@ -16,8 +16,8 @@ use jujutsu_lib::commit_builder::CommitBuilder;
 use jujutsu_lib::repo::RepoRef;
 use jujutsu_lib::revset::{parse, resolve_symbol, RevsetError};
 use jujutsu_lib::store::{CommitId, MillisSinceEpoch, Signature, Timestamp};
-use jujutsu_lib::testutils;
 use jujutsu_lib::testutils::CommitGraphBuilder;
+use jujutsu_lib::{git, testutils};
 use test_case::test_case;
 
 #[test_case(false ; "local store")]
@@ -119,6 +119,119 @@ fn test_resolve_symbol_commit_id() {
     tx.discard();
 }
 
+#[test]
+fn test_resolve_symbol_change_id() {
+    let settings = testutils::user_settings();
+    // Test only with git so we can get predictable change ids
+    let (_temp_dir, repo) = testutils::init_repo(&settings, true);
+
+    let git_repo = repo.store().git_repo().unwrap();
+    // Add some commits that will end up having change ids with common prefixes
+    let empty_tree_id = git_repo.treebuilder(None).unwrap().write().unwrap();
+    let git_author = git2::Signature::new(
+        "git author",
+        "git.author@example.com",
+        &git2::Time::new(1000, 60),
+    )
+    .unwrap();
+    let git_committer = git2::Signature::new(
+        "git committer",
+        "git.committer@example.com",
+        &git2::Time::new(2000, -480),
+    )
+    .unwrap();
+    let git_tree = git_repo.find_tree(empty_tree_id).unwrap();
+    let mut git_commit_ids = vec![];
+    for i in &[133, 664, 840] {
+        let git_commit_id = git_repo
+            .commit(
+                Some(&format!("refs/heads/branch{}", i)),
+                &git_author,
+                &git_committer,
+                &format!("test {}", i),
+                &git_tree,
+                &[],
+            )
+            .unwrap();
+        git_commit_ids.push(git_commit_id);
+    }
+
+    let mut tx = repo.start_transaction("test");
+    git::import_refs(tx.mut_repo(), &git_repo).unwrap();
+    let repo = tx.commit();
+
+    // Test the test setup
+    assert_eq!(
+        hex::encode(git_commit_ids[0].as_bytes()),
+        // "04e12a5467bba790efb88a9870894ec208b16bf1" reversed
+        "8fd68d104372910e19511df709e5dde62a548720"
+    );
+    assert_eq!(
+        hex::encode(git_commit_ids[1].as_bytes()),
+        // "040b3ba3a51d8edbc4c5855cbd09de71d4c29cca" reversed
+        "5339432b8e7b90bd3aa1a323db71b8a5c5dcd020"
+    );
+    assert_eq!(
+        hex::encode(git_commit_ids[2].as_bytes()),
+        // "04e1c7082e4e34f3f371d8a1a46770b861b9b547" reversed
+        "e2ad9d861d0ee625851b8ecfcf2c727410e38720"
+    );
+
+    // Test lookup by full change id
+    let repo_ref = repo.as_repo_ref();
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e12a5467bba790efb88a9870894ec2"),
+        Ok(vec![CommitId::from_hex(
+            "8fd68d104372910e19511df709e5dde62a548720"
+        )])
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "040b3ba3a51d8edbc4c5855cbd09de71"),
+        Ok(vec![CommitId::from_hex(
+            "5339432b8e7b90bd3aa1a323db71b8a5c5dcd020"
+        )])
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e1c7082e4e34f3f371d8a1a46770b8"),
+        Ok(vec![CommitId::from_hex(
+            "e2ad9d861d0ee625851b8ecfcf2c727410e38720"
+        )])
+    );
+
+    // Test change id prefix
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e12"),
+        Ok(vec![CommitId::from_hex(
+            "8fd68d104372910e19511df709e5dde62a548720"
+        )])
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e1c"),
+        Ok(vec![CommitId::from_hex(
+            "e2ad9d861d0ee625851b8ecfcf2c727410e38720"
+        )])
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e1"),
+        Err(RevsetError::AmbiguousChangeIdPrefix("04e1".to_string()))
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, ""),
+        // Commit id is checked first, so this is considered an ambiguous commit id
+        Err(RevsetError::AmbiguousCommitIdPrefix("".to_string()))
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "04e13"),
+        Err(RevsetError::NoSuchRevision("04e13".to_string()))
+    );
+
+    // Test non-hex string
+    assert_eq!(
+        resolve_symbol(repo_ref, "foo"),
+        Err(RevsetError::NoSuchRevision("foo".to_string()))
+    );
+}
+
 #[test_case(false ; "local store")]
 #[test_case(true ; "git store")]
 fn test_resolve_symbol_checkout(use_git: bool) {