diff --git a/CHANGELOG.md b/CHANGELOG.md index e1444c566b..4d33e1b524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). preserved. If a checked-out named branch gets deleted locally or remotely, the corresponding commits will be abandoned. +* `jj --at-op=@` no longer merges concurrent operations if explicitly specified. + ### Deprecations * The original configuration syntax for `jj fix` is now deprecated in favor of diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index bc1f0e71f2..b70c59db18 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -300,8 +300,7 @@ impl CommandHelper { let workspace = self.load_workspace()?; let op_head = self.resolve_operation(ui, workspace.repo_loader())?; let repo = workspace.repo_loader().load_at(&op_head)?; - let loaded_at_head = self.global_args.at_operation == "@"; - WorkspaceCommandHelper::new(ui, self, workspace, repo, loaded_at_head) + WorkspaceCommandHelper::new(ui, self, workspace, repo, self.is_at_head_operation()) } pub fn get_working_copy_factory(&self) -> Result<&dyn WorkingCopyFactory, CommandError> { @@ -328,13 +327,32 @@ impl CommandHelper { .map_err(|err| map_workspace_load_error(err, self.global_args.repository.as_deref())) } + /// Returns true if the working copy to be loaded is writable, and therefore + /// should usually be snapshotted. + pub fn is_working_copy_writable(&self) -> bool { + self.is_at_head_operation() && !self.global_args.ignore_working_copy + } + + /// Returns true if the current operation is considered to be the head. + pub fn is_at_head_operation(&self) -> bool { + // TODO: should we accept --at-op= as the head op? or should we + // make --at-op=@ imply --ignore-workign-copy (i.e. not at the head.) + matches!(self.global_args.at_operation.as_deref(), None | Some("@")) + } + + /// Resolves the current operation from the command-line argument. + /// + /// If no `--at-operation` is specified, the head operations will be + /// loaded. If there are multiple heads, they'll be merged. #[instrument(skip_all)] pub fn resolve_operation( &self, ui: &mut Ui, repo_loader: &RepoLoader, ) -> Result { - if self.global_args.at_operation == "@" { + if let Some(op_str) = &self.global_args.at_operation { + Ok(op_walk::resolve_op_for_load(repo_loader, op_str)?) + } else { op_heads_store::resolve_op_heads( repo_loader.op_heads_store().as_ref(), repo_loader.op_store(), @@ -365,10 +383,6 @@ impl CommandHelper { .clone()) }, ) - } else { - let operation = - op_walk::resolve_op_for_load(repo_loader, &self.global_args.at_operation)?; - Ok(operation) } } @@ -2420,10 +2434,14 @@ pub struct GlobalArgs { /// Operation to load the repo at /// /// Operation to load the repo at. By default, Jujutsu loads the repo at the - /// most recent operation. You can use `--at-op=` to see what - /// the repo looked like at an earlier operation. For example `jj - /// --at-op= st` will show you what `jj st` would have - /// shown you when the given operation had just finished. + /// most recent operation, or at the merge of the concurrent operations if + /// any. + /// + /// You can use `--at-op=` to see what the repo looked like at + /// an earlier operation. For example `jj --at-op= st` will + /// show you what `jj st` would have shown you when the given operation had + /// just finished. `--at-op=@` is pretty much the same as the default except + /// that concurrent operations will never be merged. /// /// Use `jj op log` to find the operation ID you want. Any unambiguous /// prefix of the operation ID is enough. @@ -2435,8 +2453,8 @@ pub struct GlobalArgs { /// earlier operation. Doing that is equivalent to having run concurrent /// commands starting at the earlier operation. There's rarely a reason to /// do that, but it is possible. - #[arg(long, visible_alias = "at-op", global = true, default_value = "@")] - pub at_operation: String, + #[arg(long, visible_alias = "at-op", global = true)] + pub at_operation: Option, /// Enable debug logging #[arg(long, global = true)] pub debug: bool, diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 6f95390aa4..841d71df07 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -24,7 +24,7 @@ use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemote use jj_lib::gitignore::GitIgnoreError; use jj_lib::op_heads_store::OpHeadResolutionError; use jj_lib::op_store::OpStoreError; -use jj_lib::op_walk::OpsetEvaluationError; +use jj_lib::op_walk::{OpsetEvaluationError, OpsetResolutionError}; use jj_lib::repo::{CheckOutCommitError, EditCommitError, RepoLoaderError, RewriteRootCommit}; use jj_lib::repo_path::{RepoPathBuf, UiPathParseError}; use jj_lib::revset::{ @@ -36,6 +36,7 @@ use jj_lib::working_copy::{ResetError, SnapshotError, WorkingCopyStateError}; use jj_lib::workspace::WorkspaceInitError; use thiserror::Error; +use crate::cli_util::short_operation_hash; use crate::diff_util::DiffRenderError; use crate::formatter::{FormatRecorder, Formatter}; use crate::merge_tools::{ConflictResolveError, DiffEditError, MergeToolConfigError}; @@ -291,7 +292,12 @@ impl From for CommandError { impl From for CommandError { fn from(err: OpsetEvaluationError) -> Self { match err { - OpsetEvaluationError::OpsetResolution(err) => user_error(err), + OpsetEvaluationError::OpsetResolution(err) => { + let hint = opset_resolution_error_hint(&err); + let mut cmd_err = user_error(err); + cmd_err.extend_hints(hint); + cmd_err + } OpsetEvaluationError::OpHeadResolution(err) => err.into(), OpsetEvaluationError::OpStore(err) => err.into(), } @@ -583,6 +589,22 @@ fn fileset_parse_error_hint(err: &FilesetParseError) -> Option { } } +fn opset_resolution_error_hint(err: &OpsetResolutionError) -> Option { + match err { + OpsetResolutionError::MultipleOperations { + expr: _, + candidates, + } => Some(format!( + "Try specifying one of the operations by ID: {}", + candidates.iter().map(short_operation_hash).join(", ") + )), + OpsetResolutionError::EmptyOperations(_) + | OpsetResolutionError::InvalidIdPrefix(_) + | OpsetResolutionError::NoSuchOperation(_) + | OpsetResolutionError::AmbiguousIdPrefix(_) => None, + } +} + fn revset_parse_error_hint(err: &RevsetParseError) -> Option { // Only for the bottom error, which is usually the root cause let bottom_err = iter::successors(Some(err), |e| e.origin()).last().unwrap(); diff --git a/cli/src/commands/debug/index.rs b/cli/src/commands/debug/index.rs index 2c9eb06a68..8793fceede 100644 --- a/cli/src/commands/debug/index.rs +++ b/cli/src/commands/debug/index.rs @@ -16,7 +16,6 @@ use std::fmt::Debug; use std::io::Write as _; use jj_lib::default_index::{AsCompositeIndex as _, DefaultReadonlyIndex}; -use jj_lib::op_walk; use crate::cli_util::CommandHelper; use crate::command_error::{internal_error, user_error, CommandError}; @@ -32,10 +31,10 @@ pub fn cmd_debug_index( _args: &DebugIndexArgs, ) -> Result<(), CommandError> { // Resolve the operation without loading the repo, so this command won't - // merge concurrent operations and update the index. + // update the index. let workspace = command.load_workspace()?; let repo_loader = workspace.repo_loader(); - let op = op_walk::resolve_op_for_load(repo_loader, &command.global_args().at_operation)?; + let op = command.resolve_operation(ui, repo_loader)?; let index_store = repo_loader.index_store(); let index = index_store .get_index_at_op(&op, repo_loader.store()) diff --git a/cli/src/commands/debug/reindex.rs b/cli/src/commands/debug/reindex.rs index 02ecc6f009..b8b2fa7f16 100644 --- a/cli/src/commands/debug/reindex.rs +++ b/cli/src/commands/debug/reindex.rs @@ -16,7 +16,6 @@ use std::fmt::Debug; use std::io::Write as _; use jj_lib::default_index::{AsCompositeIndex as _, DefaultIndexStore}; -use jj_lib::op_walk; use crate::cli_util::CommandHelper; use crate::command_error::{internal_error, user_error, CommandError}; @@ -35,7 +34,7 @@ pub fn cmd_debug_reindex( // be rebuilt while loading the repo. let workspace = command.load_workspace()?; let repo_loader = workspace.repo_loader(); - let op = op_walk::resolve_op_for_load(repo_loader, &command.global_args().at_operation)?; + let op = command.resolve_operation(ui, repo_loader)?; let index_store = repo_loader.index_store(); if let Some(default_index_store) = index_store.as_any().downcast_ref::() { default_index_store.reinit().map_err(internal_error)?; diff --git a/cli/src/commands/git/clone.rs b/cli/src/commands/git/clone.rs index b7cfdf8101..004985f17e 100644 --- a/cli/src/commands/git/clone.rs +++ b/cli/src/commands/git/clone.rs @@ -85,7 +85,7 @@ pub fn cmd_git_clone( command: &CommandHelper, args: &GitCloneArgs, ) -> Result<(), CommandError> { - if command.global_args().at_operation != "@" { + if command.global_args().at_operation.is_some() { return Err(cli_error("--at-op is not respected")); } let remote_name = "origin"; diff --git a/cli/src/commands/git/init.rs b/cli/src/commands/git/init.rs index 4b463c8dc2..f627625c53 100644 --- a/cli/src/commands/git/init.rs +++ b/cli/src/commands/git/init.rs @@ -76,7 +76,7 @@ pub fn cmd_git_init( if command.global_args().ignore_working_copy { return Err(cli_error("--ignore-working-copy is not respected")); } - if command.global_args().at_operation != "@" { + if command.global_args().at_operation.is_some() { return Err(cli_error("--at-op is not respected")); } let cwd = command.cwd(); diff --git a/cli/src/commands/init.rs b/cli/src/commands/init.rs index 71ff673579..d499f5c2cd 100644 --- a/cli/src/commands/init.rs +++ b/cli/src/commands/init.rs @@ -55,7 +55,7 @@ pub(crate) fn cmd_init( if command.global_args().ignore_working_copy { return Err(cli_error("--ignore-working-copy is not respected")); } - if command.global_args().at_operation != "@" { + if command.global_args().at_operation.is_some() { return Err(cli_error("--at-op is not respected")); } let cwd = command.cwd(); diff --git a/cli/src/commands/operation/abandon.rs b/cli/src/commands/operation/abandon.rs index d337b399fe..3ec22090df 100644 --- a/cli/src/commands/operation/abandon.rs +++ b/cli/src/commands/operation/abandon.rs @@ -53,11 +53,10 @@ pub fn cmd_op_abandon( let op_store = repo_loader.op_store(); // It doesn't make sense to create concurrent operations that will be merged // with the current head. - let head_op_str = &command.global_args().at_operation; - if head_op_str != "@" { + if command.global_args().at_operation.is_some() { return Err(cli_error("--at-op is not respected")); } - let current_head_op = op_walk::resolve_op_for_load(repo_loader, head_op_str)?; + let current_head_op = op_walk::resolve_op_for_load(repo_loader, "@")?; let resolve_op = |op_str| op_walk::resolve_op_at(op_store, ¤t_head_op, op_str); let (abandon_root_op, abandon_head_op) = if let Some((root_op_str, head_op_str)) = args.operation.split_once("..") { diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index d0bc505ff5..0ccfb5e34c 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::slice; + use jj_lib::op_walk; use crate::cli_util::{format_template, CommandHelper, LogContentFormat}; @@ -21,6 +23,10 @@ use crate::operation_templater::OperationTemplateLanguage; use crate::ui::Ui; /// Show the operation log +/// +/// Like other commands, `jj op log` snapshots the current working-copy changes +/// and merges concurrent operations. Use `--at-op=@ --ignore-working-copy` to +/// inspect the current state without mutation. #[derive(clap::Args, Clone, Debug)] pub struct OperationLogArgs { /// Limit number of operations to show @@ -49,40 +55,26 @@ pub fn cmd_op_log( command: &CommandHelper, args: &OperationLogArgs, ) -> Result<(), CommandError> { - // Don't load the repo so that the operation history can be inspected even - // with a corrupted repo state. For example, you can find the first bad - // operation id to be abandoned. - let workspace = command.load_workspace()?; - let repo_loader = workspace.repo_loader(); - let head_op_str = &command.global_args().at_operation; - let head_ops = if head_op_str == "@" { - // If multiple head ops can't be resolved without merging, let the - // current op be empty. Beware that resolve_op_for_load() will eliminate - // redundant heads whereas get_current_head_ops() won't. - let current_op = op_walk::resolve_op_for_load(repo_loader, head_op_str).ok(); - if let Some(op) = current_op { - vec![op] - } else { - op_walk::get_current_head_ops( - repo_loader.op_store(), - repo_loader.op_heads_store().as_ref(), - )? - } + let current_op = if command.is_working_copy_writable() { + let workspace_command = command.workspace_helper(ui)?; + workspace_command.repo().operation().clone() } else { - vec![op_walk::resolve_op_for_load(repo_loader, head_op_str)?] - }; - let current_op_id = match &*head_ops { - [op] => Some(op.id()), - _ => None, + // Don't load the repo so that the operation history can be inspected + // even with a corrupted repo state. For example, you can find the first + // bad operation id to be abandoned. + let workspace = command.load_workspace()?; + command.resolve_operation(ui, workspace.repo_loader())? }; + let op_store = current_op.op_store(); + let with_content_format = LogContentFormat::new(ui, command.settings())?; let template; let op_node_template; { let language = OperationTemplateLanguage::new( - repo_loader.op_store().root_operation_id(), - current_op_id, + op_store.root_operation_id(), + Some(current_op.id()), command.operation_template_extensions(), ); let text = match &args.template { @@ -117,7 +109,7 @@ pub fn cmd_op_log( )?; } let limit = args.limit.or(args.deprecated_limit).unwrap_or(usize::MAX); - let iter = op_walk::walk_ancestors(&head_ops).take(limit); + let iter = op_walk::walk_ancestors(slice::from_ref(¤t_op)).take(limit); if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); for op in iter { diff --git a/cli/src/commands/util.rs b/cli/src/commands/util.rs index 54feeb7f84..aa85f0c067 100644 --- a/cli/src/commands/util.rs +++ b/cli/src/commands/util.rs @@ -173,7 +173,7 @@ fn cmd_util_gc( command: &CommandHelper, args: &UtilGcArgs, ) -> Result<(), CommandError> { - if command.global_args().at_operation != "@" { + if !command.is_at_head_operation() { return Err(user_error( "Cannot garbage collect from a non-head operation", )); diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index b06a2f5e0f..20ceae6159 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -166,15 +166,15 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d This option only affects the check. It does not affect the `immutable_heads()` revset or the `immutable` template keyword. * `--at-operation ` — Operation to load the repo at - Operation to load the repo at. By default, Jujutsu loads the repo at the most recent operation. You can use `--at-op=` to see what the repo looked like at an earlier operation. For example `jj --at-op= st` will show you what `jj st` would have shown you when the given operation had just finished. + Operation to load the repo at. By default, Jujutsu loads the repo at the most recent operation, or at the merge of the concurrent operations if any. + + You can use `--at-op=` to see what the repo looked like at an earlier operation. For example `jj --at-op= st` will show you what `jj st` would have shown you when the given operation had just finished. `--at-op=@` is pretty much the same as the default except that concurrent operations will never be merged. Use `jj op log` to find the operation ID you want. Any unambiguous prefix of the operation ID is enough. When loading the repo at an earlier operation, the working copy will be ignored, as if `--ignore-working-copy` had been specified. It is possible to run mutating commands when loading the repo at an earlier operation. Doing that is equivalent to having run concurrent commands starting at the earlier operation. There's rarely a reason to do that, but it is possible. - - Default value: `@` * `--debug` — Enable debug logging * `--color ` — When to colorize output (always, never, debug, auto) * `--quiet` — Silence non-primary command output @@ -1335,6 +1335,8 @@ Compare changes to the repository between two operations Show the operation log +Like other commands, `jj op log` snapshots the current working-copy changes and merges concurrent operations. Use `--at-op=@ --ignore-working-copy` to inspect the current state without mutation. + **Usage:** `jj operation log [OPTIONS]` ###### **Options:** diff --git a/cli/tests/test_concurrent_operations.rs b/cli/tests/test_concurrent_operations.rs index 6bf8e96107..1c6e41be69 100644 --- a/cli/tests/test_concurrent_operations.rs +++ b/cli/tests/test_concurrent_operations.rs @@ -30,15 +30,19 @@ fn test_concurrent_operation_divergence() { &["describe", "-m", "message 2", "--at-op", "@-"], ); - // "op log" doesn't merge the concurrent operations - let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]); + // "--at-op=@" disables op heads merging, and prints head operation ids. + let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "log", "--at-op=@"]); + insta::assert_snapshot!(stderr, @r###" + Error: The "@" expression resolved to more than one operation + Hint: Try specifying one of the operations by ID: e31015019d90, 48f4a48f3f70 + "###); + + // "op log --at-op" should work without merging the head operations + let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log", "--at-op=48f4a48f3f70"]); insta::assert_snapshot!(stdout, @r###" - ○ 48f4a48f3f70 test-username@host.example.com 2001-02-03 04:05:09.000 +07:00 - 2001-02-03 04:05:09.000 +07:00 + @ 48f4a48f3f70 test-username@host.example.com 2001-02-03 04:05:09.000 +07:00 - 2001-02-03 04:05:09.000 +07:00 │ describe commit 230dd059e1b059aefc0da06a2e5a7dbf22362f22 │ args: jj describe -m 'message 2' --at-op @- - │ ○ e31015019d90 test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 - ├─╯ describe commit 230dd059e1b059aefc0da06a2e5a7dbf22362f22 - │ args: jj describe -m 'message 1' ○ b51416386f26 test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 │ add workspace 'default' ○ 9a7d829846af test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 00502df279..088c346cf7 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -607,7 +607,7 @@ fn test_help() { -R, --repository Path to repository to operate on --ignore-working-copy Don't snapshot the working copy, and don't update it --ignore-immutable Allow rewriting immutable commits - --at-operation Operation to load the repo at [default: @] [aliases: at-op] + --at-operation Operation to load the repo at [aliases: at-op] --debug Enable debug logging --color When to colorize output (always, never, debug, auto) --quiet Silence non-primary command output diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index 609d4db532..9d2d6e732c 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -17,7 +17,7 @@ use std::path::Path; use itertools::Itertools; use regex::Regex; -use crate::common::{get_stdout_string, TestEnvironment}; +use crate::common::{get_stdout_string, strip_last_line, TestEnvironment}; #[test] fn test_op_log() { @@ -90,6 +90,7 @@ fn test_op_log() { ); insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "@-"]), @r###" Error: The "@" expression resolved to more than one operation + Hint: Try specifying one of the operations by ID: 5f690688f7d7, cfb67eb2b65c "###); } @@ -507,6 +508,105 @@ fn test_op_abandon_without_updating_working_copy() { "###); } +#[test] +fn test_op_recover_from_bad_gc() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo", "--colocate"]); + let repo_path = test_env.env_root().join("repo"); + let git_object_path = |hex: &str| { + let (shard, file_name) = hex.split_at(2); + let mut file_path = repo_path.to_owned(); + file_path.extend([".git", "objects", shard, file_name]); + file_path + }; + + test_env.jj_cmd_ok(&repo_path, &["describe", "-m1"]); + test_env.jj_cmd_ok(&repo_path, &["describe", "-m2"]); // victim + test_env.jj_cmd_ok(&repo_path, &["abandon"]); // break predecessors chain + test_env.jj_cmd_ok(&repo_path, &["new", "-m3"]); + test_env.jj_cmd_ok(&repo_path, &["describe", "-m4"]); + + let stdout = test_env.jj_cmd_success( + &repo_path, + &["op", "log", "--no-graph", r#"-Tid.short() ++ "\n""#], + ); + let (head_op_id, _, _, bad_op_id) = stdout.lines().next_tuple().unwrap(); + insta::assert_snapshot!(head_op_id, @"43d51d9b0c0c"); + insta::assert_snapshot!(bad_op_id, @"e02cba71e65d"); + + // Corrupt the repo by removing hidden but reachable commit object. + let bad_commit_id = test_env.jj_cmd_success( + &repo_path, + &[ + "log", + "--at-op", + bad_op_id, + "--no-graph", + "-r@", + "-Tcommit_id", + ], + ); + insta::assert_snapshot!(bad_commit_id, @"ddf84fc5e0dd314092b3dfb13e09e37fa7d04ef9"); + std::fs::remove_file(git_object_path(&bad_commit_id)).unwrap(); + + // Do concurrent modification to make the situation even worse. At this + // point, the index can be loaded, so this command succeeds. + // TODO: test_env.jj_cmd_ok(&repo_path, &["--at-op=@-", "describe", "-m4.1"]); + // TODO: "op abandon" doesn't work if there are multiple op heads. + + let stderr = + test_env.jj_cmd_internal_error(&repo_path, &["--at-op", head_op_id, "debug", "reindex"]); + insta::assert_snapshot!(strip_last_line(&stderr), @r###" + Internal error: Failed to index commits at operation e02cba71e65dc9b58bb46e4a31500d94c31ce217f19459883fc85c2d1cfde7124f556565be4f8a3bcd059fe1faee317c3e3eacd3034d419083fd6b85aea396f4 + Caused by: + 1: Object ddf84fc5e0dd314092b3dfb13e09e37fa7d04ef9 of type commit not found + "###); + + // "op log" should still be usable. + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "log", "--ignore-working-copy"]); + insta::assert_snapshot!(stdout, @r###" + @ 43d51d9b0c0c test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00 + │ describe commit 37bb762e5dc08073ec4323bdffc023a0f0cc901e + │ args: jj describe -m4 + ○ 13035304f7d6 test-username@host.example.com 2001-02-03 04:05:11.000 +07:00 - 2001-02-03 04:05:11.000 +07:00 + │ new empty commit + │ args: jj new -m3 + ○ fcf34f8ae8ac test-username@host.example.com 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 + │ abandon commit ddf84fc5e0dd314092b3dfb13e09e37fa7d04ef9 + │ args: jj abandon + ○ e02cba71e65d test-username@host.example.com 2001-02-03 04:05:09.000 +07:00 - 2001-02-03 04:05:09.000 +07:00 + │ describe commit 8b64ddff700dc214dec05d915e85ac692233e6e3 + │ args: jj describe -m2 + ○ 0d7016e495d7 test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 + │ describe commit 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + │ args: jj describe -m1 + ○ b51416386f26 test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 + │ add workspace 'default' + ○ 9a7d829846af test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 + │ initialize repo + ○ 000000000000 root() + "###); + insta::assert_snapshot!(stderr, @""); + + // "op abandon" should work. + let (_stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["op", "abandon", &format!("..{bad_op_id}")]); + insta::assert_snapshot!(stderr, @r###" + Abandoned 4 operations and reparented 3 descendant operations. + "###); + + // The repo should no longer be corrupt. + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log"]); + insta::assert_snapshot!(stdout, @r###" + @ mzvwutvl test.user@example.com 2001-02-03 08:05:12 6d868f04 + │ (empty) 4 + ○ zsuskuln test.user@example.com 2001-02-03 08:05:10 HEAD@git f652c321 + │ (empty) (no description set) + ◆ zzzzzzzz root() 00000000 + "###); + insta::assert_snapshot!(stderr, @""); +} + #[test] fn test_op_diff() { let test_env = TestEnvironment::default(); diff --git a/lib/src/op_walk.rs b/lib/src/op_walk.rs index 14eee4fc62..b33e19101d 100644 --- a/lib/src/op_walk.rs +++ b/lib/src/op_walk.rs @@ -50,8 +50,13 @@ pub enum OpsetResolutionError { // TODO: Maybe empty/multiple operations should be allowed, and rejected by // caller as needed. /// Expression resolved to multiple operations. - #[error(r#"The "{0}" expression resolved to more than one operation"#)] - MultipleOperations(String), + #[error(r#"The "{expr}" expression resolved to more than one operation"#)] + MultipleOperations { + /// Source expression. + expr: String, + /// Matched operation ids. + candidates: Vec, + }, /// Expression resolved to no operations. #[error(r#"The "{0}" expression resolved to no operations"#)] EmptyOperations(String), @@ -74,8 +79,12 @@ pub fn resolve_op_for_load( let op_store = repo_loader.op_store(); let op_heads_store = repo_loader.op_heads_store().as_ref(); let get_current_op = || { - op_heads_store::resolve_op_heads(op_heads_store, op_store, |_| { - Err(OpsetResolutionError::MultipleOperations("@".to_owned()).into()) + op_heads_store::resolve_op_heads(op_heads_store, op_store, |op_heads| { + Err(OpsetResolutionError::MultipleOperations { + expr: "@".to_owned(), + candidates: op_heads.iter().map(|op| op.id().clone()).collect(), + } + .into()) }) }; let get_head_ops = || get_current_head_ops(op_store, op_heads_store); @@ -127,7 +136,10 @@ fn resolve_single_op( operation = match neighbor_ops.len() { 0 => Err(OpsetResolutionError::EmptyOperations(op_str.to_owned()))?, 1 => neighbor_ops.pop().unwrap(), - _ => Err(OpsetResolutionError::MultipleOperations(op_str.to_owned()))?, + _ => Err(OpsetResolutionError::MultipleOperations { + expr: op_str.to_owned(), + candidates: neighbor_ops.iter().map(|op| op.id().clone()).collect(), + })?, }; } Ok(operation) diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index d7bb81b745..be3040ab63 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -580,14 +580,14 @@ fn test_resolve_op_parents_children() { assert_matches!( op_walk::resolve_op_with_repo(&repo, &format!("{op5_id_hex}-")), Err(OpsetEvaluationError::OpsetResolution( - OpsetResolutionError::MultipleOperations(_) + OpsetResolutionError::MultipleOperations { .. } )) ); let op2_id_hex = operations[2].id().hex(); assert_matches!( op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}+")), Err(OpsetEvaluationError::OpsetResolution( - OpsetResolutionError::MultipleOperations(_) + OpsetResolutionError::MultipleOperations { .. } )) ); }