martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
9.04k stars 314 forks source link

Panic in `jj op show` on "reconcile divergent operations" #4465

Open scott2000 opened 1 month ago

scott2000 commented 1 month ago

Description

I was messing around with divergent operations to see how jj handles abandoning a commit while also trying to edit it concurrently, and I found this crash in jj op show.

Steps to Reproduce the Problem

jj git init temp && cd temp
jj workspace add ../temp2
jj new 'root()' -m 'A'
jj new 'root()'
jj abandon 'description(A)' &
jj edit 'description(A)'
wait
cd ../temp2
jj log
jj op show @

Interestingly, jj op diff --op @ works and does not panic.

Expected Behavior

No panic.

Actual Behavior

thread 'main' panicked at lib/src/default_index/revset_engine.rs:972:55:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: core::option::unwrap_failed
   4: jj_lib::default_index::revset_engine::EvaluationContext::revset_for_commit_ids::{{closure}}
   5: core::iter::adapters::map::map_fold::{{closure}}
   6: <core::slice::iter::Iter<T> as core::iter::traits::iterator::Iterator>::fold
   7: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
   8: core::iter::traits::iterator::Iterator::for_each
   9: alloc::vec::Vec<T,A>::extend_trusted
  10: <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
  11: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
  12: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
  13: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
  14: core::iter::traits::iterator::Iterator::collect
  15: itertools::Itertools::collect_vec
  16: jj_lib::default_index::revset_engine::EvaluationContext::revset_for_commit_ids
  17: jj_lib::default_index::revset_engine::EvaluationContext::evaluate
  18: jj_lib::default_index::revset_engine::EvaluationContext::evaluate
  19: jj_lib::default_index::revset_engine::evaluate
  20: jj_lib::default_index::composite::CompositeIndex::evaluate_revset
  21: <jj_lib::default_index::readonly::DefaultReadonlyIndex as jj_lib::index::Index>::evaluate_revset
  22: jj_lib::revset::ResolvedExpression::evaluate
  23: jj_lib::revset::RevsetExpression::evaluate_programmatic
  24: jj_lib::revset::walk_revs
  25: jj_cli::commands::operation::diff::compute_operation_commits_diff
  26: jj_cli::commands::operation::diff::show_op_diff
  27: jj_cli::commands::operation::show::cmd_op_show
  28: jj_cli::commands::operation::cmd_operation
  29: jj_cli::commands::run_command
  30: core::ops::function::FnOnce::call_once
  31: core::ops::function::FnOnce::call_once{{vtable.shim}}
  32: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
  33: jj_cli::cli_util::CliRunner::run_internal
  34: jj_cli::cli_util::CliRunner::run
  35: jj::main
  36: core::ops::function::FnOnce::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Specifications

yuja commented 1 month ago

Perhaps, the problem is that merge_operations() can create new commit (such as replacement for an abandoned wc commit.) You can see op diff --op @ returns random diffs (where auto-merge diff should be empty.)