Skip to content

Commit

Permalink
revset: fix crash on "log --at-op 00000000 -r 'root()'"
Browse files Browse the repository at this point in the history
Spotted while refactoring IdPrefixContext.
  • Loading branch information
yuja committed Oct 1, 2024
1 parent 0a8d8da commit 480d8e1
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
13 changes: 12 additions & 1 deletion lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,18 @@ fn resolve_commit_ref(
Ok(wc_commits)
}
RevsetCommitRef::VisibleHeads => Ok(repo.view().heads().iter().cloned().collect_vec()),
RevsetCommitRef::Root => Ok(vec![repo.store().root_commit_id().clone()]),
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::Bookmarks(pattern) => {
let commit_ids = repo
.view()
Expand Down
17 changes: 17 additions & 0 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use jj_lib::op_store::RefTarget;
use jj_lib::op_store::RemoteRef;
use jj_lib::op_store::RemoteRefState;
use jj_lib::op_store::WorkspaceId;
use jj_lib::operation::Operation;
use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::repo_path::RepoPathUiConverter;
Expand Down Expand Up @@ -942,6 +943,14 @@ fn test_evaluate_expression_root_and_checkout() {
let test_workspace = TestWorkspace::init(&settings);
let repo = &test_workspace.repo;

let root_operation = {
let op_store = repo.op_store();
let id = op_store.root_operation_id();
let data = op_store.read_operation(id).unwrap();
Operation::new(op_store.clone(), id.clone(), data)
};
let root_repo = repo.reload_at(&root_operation).unwrap();

let mut tx = repo.start_transaction(&settings);
let mut_repo = tx.repo_mut();

Expand All @@ -954,6 +963,14 @@ fn test_evaluate_expression_root_and_checkout() {
vec![root_commit.id().clone()]
);

// but not in the root operation. It might be okay to pretend that the root
// commit exists in the root operation, but queries like "root()" shouldn't
// panic in any case.
assert_matches!(
resolve_symbol(root_repo.as_ref(), "root()"),
Err(RevsetResolutionError::NoSuchRevision { .. })
);

// Can find the current working-copy commit
mut_repo
.set_wc_commit(WorkspaceId::default(), commit1.id().clone())
Expand Down

0 comments on commit 480d8e1

Please sign in to comment.