From 543036c753fd8f9af3a95bbe7ef8070765f7989e Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 11 Jan 2024 12:31:54 +0900 Subject: [PATCH] cli: run "op log" without loading repo or merging concurrent ops When debugging behavior of badly-GCed repos, I find it's annoying that "op log" fails because the index can't be loaded. Since "op log" doesn't need a repo, I think it's better to display the exact op-heads state without merging. --- cli/src/commands/operation.rs | 43 ++++++++++++++++++------- cli/tests/test_concurrent_operations.rs | 15 +++++++++ lib/src/op_walk.rs | 2 +- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/cli/src/commands/operation.rs b/cli/src/commands/operation.rs index 11a3da63b1..2bc47b902d 100644 --- a/cli/src/commands/operation.rs +++ b/cli/src/commands/operation.rs @@ -132,26 +132,45 @@ fn cmd_op_log( command: &CommandHelper, args: &OperationLogArgs, ) -> Result<(), CommandError> { - let workspace_command = command.workspace_helper(ui)?; - let repo = workspace_command.repo(); - let head_op = repo.operation().clone(); - let head_op_id = head_op.id().clone(); + // 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(), + )? + } + } 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, + }; let template_string = match &args.template { Some(value) => value.to_owned(), None => command.settings().config().get_string("templates.op_log")?, }; - let template = operation_templater::parse( - Some(repo.op_id()), - &template_string, - workspace_command.template_aliases_map(), - )?; + let template_aliases = command.load_template_aliases(ui)?; + let template = operation_templater::parse(current_op_id, &template_string, &template_aliases)?; let with_content_format = LogContentFormat::new(ui, command.settings())?; ui.request_pager(); let mut formatter = ui.stdout_formatter(); let formatter = formatter.as_mut(); - let iter = op_walk::walk_ancestors(&[head_op]).take(args.limit.unwrap_or(usize::MAX)); + let iter = op_walk::walk_ancestors(&head_ops).take(args.limit.unwrap_or(usize::MAX)); if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); let default_node_symbol = graph.default_node_symbol().to_owned(); @@ -161,7 +180,7 @@ fn cmd_op_log( for id in op.parent_ids() { edges.push(Edge::direct(id.clone())); } - let is_head_op = op.id() == &head_op_id; + let is_current_op = Some(op.id()) == current_op_id; let mut buffer = vec![]; with_content_format.write_graph_text( ui.new_formatter(&mut buffer).as_mut(), @@ -173,7 +192,7 @@ fn cmd_op_log( if !buffer.ends_with(b"\n") { buffer.push(b'\n'); } - let node_symbol = if is_head_op { + let node_symbol = if is_current_op { "@" } else { &default_node_symbol diff --git a/cli/tests/test_concurrent_operations.rs b/cli/tests/test_concurrent_operations.rs index 2e5a7a307e..33a3efa280 100644 --- a/cli/tests/test_concurrent_operations.rs +++ b/cli/tests/test_concurrent_operations.rs @@ -32,6 +32,21 @@ 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"]); + insta::assert_snapshot!(stdout, @r###" + ◉ 94fce7319d45 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 @- + │ ◉ 15e35d1e9190 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' + ◉ 19b8089fc78b 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' + ◉ f1c462c494be 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 + "###); + // We should be informed about the concurrent modification let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-T", "description"]); insta::assert_snapshot!(stdout, @r###" diff --git a/lib/src/op_walk.rs b/lib/src/op_walk.rs index bfc0783630..b10454e13c 100644 --- a/lib/src/op_walk.rs +++ b/lib/src/op_walk.rs @@ -158,7 +158,7 @@ fn resolve_single_op_from_store( /// Loads the current head operations. The returned operations may contain /// redundant ones which are ancestors of the other heads. -fn get_current_head_ops( +pub fn get_current_head_ops( op_store: &Arc, op_heads_store: &dyn OpHeadsStore, ) -> OpStoreResult> {