diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index b8473c468c..d84c48a333 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -52,6 +52,7 @@ use crate::graph::GraphEdge; use crate::matchers::Matcher; use crate::matchers::Visit; use crate::merged_tree::resolve_file_values; +use crate::object_id::ObjectId as _; use crate::repo_path::RepoPath; use crate::revset::ResolvedExpression; use crate::revset::ResolvedPredicateExpression; @@ -759,7 +760,7 @@ impl<'index> EvaluationContext<'index> { let index = self.index; match expression { ResolvedExpression::Commits(commit_ids) => { - Ok(Box::new(self.revset_for_commit_ids(commit_ids))) + Ok(Box::new(self.revset_for_commit_ids(commit_ids)?)) } ResolvedExpression::Ancestors { heads, generation } => { let head_set = self.evaluate(heads)?; @@ -966,14 +967,28 @@ impl<'index> EvaluationContext<'index> { } } - fn revset_for_commit_ids(&self, commit_ids: &[CommitId]) -> EagerRevset { - let mut positions = commit_ids + fn revset_for_commit_ids( + &self, + commit_ids: &[CommitId], + ) -> Result { + let mut positions: Vec<_> = commit_ids .iter() - .map(|id| self.index.commit_id_to_pos(id).unwrap()) - .collect_vec(); + .map(|id| { + // Invalid commit IDs should be rejected by the revset frontend, + // but there are a few edge cases that break the precondition. + // For example, in jj <= 0.22, the root commit doesn't exist in + // the root operation. + self.index.commit_id_to_pos(id).ok_or_else(|| { + RevsetEvaluationError::Other(format!( + "Commit ID {} not found in index (index or view might be corrupted)", + id.hex() + )) + }) + }) + .try_collect()?; positions.sort_unstable_by_key(|&pos| Reverse(pos)); positions.dedup(); - EagerRevset { positions } + Ok(EagerRevset { positions }) } fn take_latest_revset(&self, candidate_set: &dyn InternalRevset, count: usize) -> EagerRevset { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 4f96ad16e1..76c3030c83 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1765,18 +1765,7 @@ fn resolve_commit_ref( Ok(wc_commits) } RevsetCommitRef::VisibleHeads => Ok(repo.view().heads().iter().cloned().collect_vec()), - RevsetCommitRef::Root => { - let commit_id = repo.store().root_commit_id(); - if repo.index().has_id(commit_id) { - Ok(vec![commit_id.clone()]) - } else { - // The root commit doesn't exist at the root operation. - Err(RevsetResolutionError::NoSuchRevision { - name: "root()".to_owned(), - candidates: vec![], - }) - } - } + RevsetCommitRef::Root => Ok(vec![repo.store().root_commit_id().clone()]), RevsetCommitRef::Bookmarks(pattern) => { let commit_ids = repo .view() diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 6ec652150a..540fbb1d16 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -971,6 +971,10 @@ fn test_evaluate_expression_root_and_checkout() { resolve_commit_ids_in_workspace(mut_repo, "@", &test_workspace.workspace, None), vec![commit1.id().clone()] ); + + // Shouldn't panic by unindexed commit ID + let expression = RevsetExpression::commit(commit1.id().clone()).resolve_programmatic(tx.repo()); + assert!(expression.evaluate(tx.base_repo().as_ref()).is_err()); } #[test]