revsets: move code for resolving symbol to commit to new module

lib/src/lib.rs

@@ -39,6 +39,7 @@ pub mod operation;
 pub mod protos;
 pub mod repo;
 pub mod repo_path;
+pub mod revset;
 pub mod rewrite;
 pub mod settings;
 pub mod simple_op_store;

lib/src/revset.rs

@@ -0,0 +1,68 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use thiserror::Error;
+
+use crate::commit::Commit;
+use crate::index::{HexPrefix, PrefixResolution};
+use crate::repo::RepoRef;
+use crate::store::{CommitId, StoreError};
+
+#[derive(Debug, Error, PartialEq, Eq)]
+pub enum RevsetError {
+    #[error("Revision \"{0}\" doesn't exist")]
+    NoSuchRevision(String),
+    #[error("Commit id prefix \"{0}\" is ambiguous")]
+    AmbiguousCommitIdPrefix(String),
+    #[error("Unexpected error from store: {0}")]
+    StoreError(#[from] StoreError),
+}
+
+// TODO: Decide if we should allow a single symbol to resolve to multiple
+// revisions. For example, we may want to resolve a change id to all the
+// matching commits. Depending on how we decide to handle divergent git refs and
+// similar, we may also want those to resolve to multiple commits.
+pub fn resolve_symbol(repo: RepoRef, symbol: &str) -> Result<Commit, RevsetError> {
+    // TODO: Support git refs and change ids.
+    if symbol == "@" {
+        Ok(repo.store().get_commit(repo.view().checkout())?)
+    } else if symbol == "root" {
+        Ok(repo.store().root_commit())
+    } else {
+        // Try to resolve as a commit id. First check if it's a full commit id.
+        if let Ok(binary_commit_id) = hex::decode(symbol) {
+            let commit_id = CommitId(binary_commit_id);
+            match repo.store().get_commit(&commit_id) {
+                Ok(commit) => return Ok(commit),
+                Err(StoreError::NotFound) => {} // fall through
+                Err(err) => return Err(RevsetError::StoreError(err)),
+            }
+        }
+        let commit_id = resolve_commit_id_prefix(repo, symbol)?;
+        Ok(repo.store().get_commit(&commit_id)?)
+    }
+}
+
+fn resolve_commit_id_prefix(repo: RepoRef, symbol: &str) -> Result<CommitId, RevsetError> {
+    match repo
+        .index()
+        .resolve_prefix(&HexPrefix::new(symbol.to_owned()))
+    {
+        PrefixResolution::NoMatch => Err(RevsetError::NoSuchRevision(symbol.to_owned())),
+        PrefixResolution::AmbiguousMatch => {
+            Err(RevsetError::AmbiguousCommitIdPrefix(symbol.to_owned()))
+        }
+        PrefixResolution::SingleMatch(id) => Ok(id),
+    }
+}

lib/tests/test_revset.rs

@@ -0,0 +1,135 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use jujube_lib::commit_builder::CommitBuilder;
+use jujube_lib::revset::{resolve_symbol, RevsetError};
+use jujube_lib::store::{MillisSinceEpoch, Signature, Timestamp};
+use jujube_lib::testutils;
+use test_case::test_case;
+
+#[test_case(false ; "local store")]
+#[test_case(true ; "git store")]
+fn test_resolve_symbol_root(use_git: bool) {
+    let settings = testutils::user_settings();
+    let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
+
+    assert_eq!(
+        resolve_symbol(repo.as_repo_ref(), "root").unwrap(),
+        repo.store().root_commit()
+    );
+}
+
+#[test]
+fn test_resolve_symbol_commit_id() {
+    let settings = testutils::user_settings();
+    // Test only with git so we can get predictable commit ids
+    let (_temp_dir, repo) = testutils::init_repo(&settings, true);
+
+    let mut tx = repo.start_transaction("test");
+    let mut_repo = tx.mut_repo();
+    let signature = Signature {
+        name: "test".to_string(),
+        email: "test".to_string(),
+        timestamp: Timestamp {
+            timestamp: MillisSinceEpoch(0),
+            tz_offset: 0,
+        },
+    };
+
+    let mut commits = vec![];
+    for i in &[1, 167, 895] {
+        let commit = CommitBuilder::for_new_commit(
+            &settings,
+            repo.store(),
+            repo.store().empty_tree_id().clone(),
+        )
+        .set_description(format!("test {}", i))
+        .set_author(signature.clone())
+        .set_committer(signature.clone())
+        .write_to_repo(mut_repo);
+        commits.push(commit);
+    }
+
+    // Test the test setup
+    assert_eq!(
+        commits[0].id().hex(),
+        "0454de3cae04c46cda37ba2e8873b4c17ff51dcb"
+    );
+    assert_eq!(
+        commits[1].id().hex(),
+        "045f56cd1b17e8abde86771e2705395dcde6a957"
+    );
+    assert_eq!(
+        commits[2].id().hex(),
+        "0468f7da8de2ce442f512aacf83411d26cd2e0cf"
+    );
+
+    // Test lookup by full commit id
+    let repo_ref = mut_repo.as_repo_ref();
+    assert_eq!(
+        resolve_symbol(repo_ref, "0454de3cae04c46cda37ba2e8873b4c17ff51dcb").unwrap(),
+        commits[0]
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "045f56cd1b17e8abde86771e2705395dcde6a957").unwrap(),
+        commits[1]
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "0468f7da8de2ce442f512aacf83411d26cd2e0cf").unwrap(),
+        commits[2]
+    );
+
+    // Test commit id prefix
+    assert_eq!(resolve_symbol(repo_ref, "046").unwrap(), commits[2]);
+    assert_eq!(
+        resolve_symbol(repo_ref, "04"),
+        Err(RevsetError::AmbiguousCommitIdPrefix("04".to_string()))
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, ""),
+        Err(RevsetError::AmbiguousCommitIdPrefix("".to_string()))
+    );
+    assert_eq!(
+        resolve_symbol(repo_ref, "040"),
+        Err(RevsetError::NoSuchRevision("040".to_string()))
+    );
+
+    tx.discard();
+}
+
+#[test_case(false ; "local store")]
+#[test_case(true ; "git store")]
+fn test_resolve_symbol_checkout(use_git: bool) {
+    let settings = testutils::user_settings();
+    let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
+
+    let mut tx = repo.start_transaction("test");
+    let mut_repo = tx.mut_repo();
+
+    let commit1 = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo);
+    let commit2 = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo);
+
+    mut_repo.set_checkout(commit1.id().clone());
+    assert_eq!(
+        resolve_symbol(mut_repo.as_repo_ref(), "@").unwrap(),
+        commit1
+    );
+    mut_repo.set_checkout(commit2.id().clone());
+    assert_eq!(
+        resolve_symbol(mut_repo.as_repo_ref(), "@").unwrap(),
+        commit2
+    );
+
+    tx.discard();
+}

src/commands.rs

@@ -34,19 +34,20 @@ use jujube_lib::dag_walk::{topo_order_reverse, walk_ancestors};
 use jujube_lib::evolution::{evolve, EvolveListener};
 use jujube_lib::files::DiffLine;
 use jujube_lib::git::GitFetchError;
-use jujube_lib::index::{HexPrefix, PrefixResolution};
+use jujube_lib::index::HexPrefix;
 use jujube_lib::op_store::{OpStore, OpStoreError, OperationId};
 use jujube_lib::operation::Operation;
 use jujube_lib::repo::{MutableRepo, ReadonlyRepo, RepoLoadError, RepoLoader};
 use jujube_lib::repo_path::RepoPath;
+use jujube_lib::revset::RevsetError;
 use jujube_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit};
 use jujube_lib::settings::UserSettings;
-use jujube_lib::store::{CommitId, StoreError, Timestamp, TreeValue};
+use jujube_lib::store::{CommitId, Timestamp, TreeValue};
 use jujube_lib::store_wrapper::StoreWrapper;
 use jujube_lib::tree::Tree;
 use jujube_lib::trees::Diff;
 use jujube_lib::working_copy::{CheckoutStats, WorkingCopy};
-use jujube_lib::{conflicts, files, git};
+use jujube_lib::{conflicts, files, git, revset};
 use pest::Parser;
 
 use self::chrono::{FixedOffset, TimeZone, Utc};
@@ -93,6 +94,12 @@ impl From<RepoLoadError> for CommandError {
     }
 }
 
+impl From<RevsetError> for CommandError {
+    fn from(err: RevsetError) -> Self {
+        CommandError::UserError(format!("{}", err))
+    }
+}
+
 fn get_repo(ui: &Ui, matches: &ArgMatches) -> Result<Arc<ReadonlyRepo>, CommandError> {
     let wc_path_str = matches.value_of("repository").unwrap();
     let wc_path = ui.cwd().join(wc_path_str);
@@ -105,19 +112,6 @@ fn get_repo(ui: &Ui, matches: &ArgMatches) -> Result<Arc<ReadonlyRepo>, CommandE
     }
 }
 
-fn resolve_commit_id_prefix(
-    repo: &ReadonlyRepo,
-    prefix: &HexPrefix,
-) -> Result<CommitId, CommandError> {
-    match repo.index().resolve_prefix(prefix) {
-        PrefixResolution::NoMatch => Err(CommandError::UserError(String::from("No such commit"))),
-        PrefixResolution::AmbiguousMatch => {
-            Err(CommandError::UserError(String::from("Ambiguous prefix")))
-        }
-        PrefixResolution::SingleMatch(id) => Ok(id),
-    }
-}
-
 fn resolve_revision_arg(
     ui: &Ui,
     repo: &mut ReadonlyRepo,
@@ -131,18 +125,19 @@ fn resolve_single_rev(
     repo: &mut ReadonlyRepo,
     revision_str: &str,
 ) -> Result<Commit, CommandError> {
+    // If we're looking up the working copy commit ("@"), make sure that it is up to
+    // date (the lib crate only looks at the checkout in the view).
     if revision_str == "@" {
         let owned_wc = repo.working_copy().clone();
-        let wc = owned_wc.lock().unwrap();
         // TODO: Avoid committing every time this function is called.
-        Ok(wc.commit(ui.settings(), repo))
+        owned_wc.lock().unwrap().commit(ui.settings(), repo);
-    } else if revision_str == "@^" {
+    }
+
+    if revision_str == "@^" {
         let commit = repo.store().get_commit(repo.view().checkout()).unwrap();
         assert!(commit.is_open());
         let parents = commit.parents();
         Ok(parents[0].clone())
-    } else if revision_str == "root" {
-        Ok(repo.store().root_commit())
     } else if revision_str.starts_with("desc(") && revision_str.ends_with(')') {
         let needle = revision_str[5..revision_str.len() - 1].to_string();
         let mut matches = vec![];
@@ -160,21 +155,7 @@ fn resolve_single_rev(
             .pop()
             .ok_or_else(|| CommandError::UserError(String::from("No matching commit")))
     } else {
-        if let Ok(binary_commit_id) = hex::decode(revision_str) {
+        Ok(revset::resolve_symbol(repo.as_repo_ref(), revision_str)?)
-            let commit_id = CommitId(binary_commit_id);
-            match repo.store().get_commit(&commit_id) {
-                Ok(commit) => return Ok(commit),
-                Err(StoreError::NotFound) => {} // fall through
-                Err(err) => {
-                    return Err(CommandError::InternalError(format!(
-                        "Failed to read commit: {}",
-                        err
-                    )))
-                }
-            }
-        }
-        let id = resolve_commit_id_prefix(repo, &HexPrefix::new(revision_str.to_owned()))?;
-        Ok(repo.store().get_commit(&id).unwrap())
     }
 }