Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: merge op heads and snapshot working copy by "op log" by default #4165

Merged
merged 5 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 31 additions & 13 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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=<head_id> 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<Operation, CommandError> {
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(),
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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=<operation ID>` to see what
/// the repo looked like at an earlier operation. For example `jj
/// --at-op=<operation ID> 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=<operation ID>` to see what the repo looked like at
/// an earlier operation. For example `jj --at-op=<operation ID> 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.
Expand All @@ -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<String>,
/// Enable debug logging
#[arg(long, global = true)]
pub debug: bool,
Expand Down
26 changes: 24 additions & 2 deletions cli/src/command_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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};
Expand Down Expand Up @@ -291,7 +292,12 @@ impl From<OpHeadResolutionError> for CommandError {
impl From<OpsetEvaluationError> 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(),
}
Expand Down Expand Up @@ -583,6 +589,22 @@ fn fileset_parse_error_hint(err: &FilesetParseError) -> Option<String> {
}
}

fn opset_resolution_error_hint(err: &OpsetResolutionError) -> Option<String> {
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<String> {
// Only for the bottom error, which is usually the root cause
let bottom_err = iter::successors(Some(err), |e| e.origin()).last().unwrap();
Expand Down
5 changes: 2 additions & 3 deletions cli/src/commands/debug/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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())
Expand Down
3 changes: 1 addition & 2 deletions cli/src/commands/debug/reindex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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::<DefaultIndexStore>() {
default_index_store.reinit().map_err(internal_error)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/git/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/git/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 2 additions & 3 deletions cli/src/commands/operation/abandon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, &current_head_op, op_str);
let (abandon_root_op, abandon_head_op) =
if let Some((root_op_str, head_op_str)) = args.operation.split_once("..") {
Expand Down
46 changes: 19 additions & 27 deletions cli/src/commands/operation/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
yuja marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(&current_op)).take(limit);
if !args.no_graph {
let mut graph = get_graphlog(command.settings(), formatter.raw());
for op in iter {
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
));
Expand Down
8 changes: 5 additions & 3 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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=<operation ID>` to see what the repo looked like at an earlier operation. For example `jj --at-op=<operation ID> 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=<operation ID>` to see what the repo looked like at an earlier operation. For example `jj --at-op=<operation ID> 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>` — When to colorize output (always, never, debug, auto)
* `--quiet` — Silence non-primary command output
Expand Down Expand Up @@ -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:**
Expand Down
16 changes: 10 additions & 6 deletions cli/tests/test_concurrent_operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected] 2001-02-03 04:05:09.000 +07:00 - 2001-02-03 04:05:09.000 +07:00
@ 48f4a48f3f70 [email protected] 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 [email protected] 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 [email protected] 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00
│ add workspace 'default'
○ 9a7d829846af [email protected] 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ fn test_help() {
-R, --repository <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 <AT_OPERATION> Operation to load the repo at [default: @] [aliases: at-op]
--at-operation <AT_OPERATION> Operation to load the repo at [aliases: at-op]
--debug Enable debug logging
--color <WHEN> When to colorize output (always, never, debug, auto)
--quiet Silence non-primary command output
Expand Down
Loading