From 4698dfe7e6fd74c2bf037e0e04200a16e462e224 Mon Sep 17 00:00:00 2001 From: Anton Bulakh Date: Sat, 17 Feb 2024 21:49:10 +0200 Subject: [PATCH] cli: Add an option to record consecutive snapshots as a single op --- CHANGELOG.md | 17 ++++++---- cli/src/cli_util.rs | 29 +++++++++++++++-- cli/src/config-schema.json | 5 +++ cli/src/operation_templater.rs | 2 ++ cli/tests/test_concurrent_operations.rs | 12 +++---- cli/tests/test_util_command.rs | 2 +- cli/tests/test_working_copy.rs | 42 +++++++++++++++++++++++++ cli/tests/test_workspaces.rs | 20 ++++++------ lib/src/settings.rs | 6 ++++ lib/src/transaction.rs | 29 +++++++++++++++-- 10 files changed, 135 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 458a5b38ad..0954a0169d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added completions for [Nushell](https://nushell.sh) to `jj util completion` +* Set config `snapshot.squash-consecutive-snapshots = true` to make consecutive + snapshots result in a single operation being recorded, effectively + automatically abandoning the intermediate snapshot operations, which helps + GC, especially if you run `jj log` in a loop for example. + ### Fixed bugs * On Windows, symlinks in the repo are now materialized as regular files in the @@ -78,7 +83,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 copy commit on top of a single specified revision, i.e. with one parent. `merge` creates a new working copy commit on top of *at least* two specified revisions, i.e. with two or more parents. - + The only difference between these commands and `jj new`, which *also* creates a new working copy commit, is that `new` can create a working copy commit on top of any arbitrary number of revisions, so it can handle both the previous @@ -230,7 +235,7 @@ Thanks to the people who made this release happen! * `jj branch set` no longer creates a new branch. Use `jj branch create` instead. - + * `jj init --git` in an existing Git repository now errors and exits rather than creating a second Git store. @@ -394,8 +399,8 @@ Thanks to the people who made this release happen! ### New features -* The `ancestors()` revset function now takes an optional `depth` argument - to limit the depth of the ancestor set. For example, use `jj log -r +* The `ancestors()` revset function now takes an optional `depth` argument + to limit the depth of the ancestor set. For example, use `jj log -r 'ancestors(@, 5)` to view the last 5 commits. * Support for the Watchman filesystem monitor is now bundled by default. Set @@ -560,13 +565,13 @@ Thanks to the people who made this release happen! respectively. * `jj log` timestamp format now accepts `.utc()` to convert a timestamp to UTC. - + * templates now support additional string methods `.starts_with(x)`, `.ends_with(x)` `.remove_prefix(x)`, `.remove_suffix(x)`, and `.substr(start, end)`. * `jj next` and `jj prev` are added, these allow you to traverse the history in a linear style. For people coming from Sapling and `git-branchles` - see [#2126](https://github.com/martinvonz/jj/issues/2126) for + see [#2126](https://github.com/martinvonz/jj/issues/2126) for further pending improvements. * `jj diff --stat` has been implemented. It shows a histogram of the changes, diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 3348f8ce61..24774e6166 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1413,10 +1413,18 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin let mut tx = start_repo_transaction(&self.user_repo.repo, &self.settings, &self.string_args); let mut_repo = tx.mut_repo(); - let commit = mut_repo + + let mut commit_builder = mut_repo .rewrite_commit(&self.settings, &wc_commit) - .set_tree_id(new_tree_id) - .write()?; + .set_tree_id(new_tree_id); + + let squash_snapshots = self.settings.squash_consecutive_snapshots(); + if squash_snapshots { + commit_builder = + commit_builder.set_predecessors(wc_commit.predecessor_ids().to_owned()) + } + + let commit = commit_builder.write()?; mut_repo.set_wc_commit(workspace_id, commit.id().clone())?; // Rebase descendants @@ -1433,6 +1441,21 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin print_failed_git_export(ui, &failed_branches)?; } + const TAG: &str = "snapshots"; + + let mut snapshots = 1; + + if squash_snapshots { + if let [parent_op] = tx.parent_ops() { + if let Some(tag) = parent_op.store_operation().metadata.tags.get(TAG) { + snapshots = tag.parse().unwrap_or(1) + 1; + tx.replace_parent()?; + } + } + } + + tx.set_tag(TAG.into(), snapshots.to_string()); + self.user_repo = ReadonlyUserRepo::new(tx.commit("snapshot working copy")); } locked_ws.finish(self.user_repo.repo.op_id().clone())?; diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index c3f0285fa6..7057c9cb84 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -357,6 +357,11 @@ ], "description": "New files with a size in bytes above this threshold are not snapshotted, unless the threshold is 0", "default": "1MiB" + }, + "squash-consecutive-snapshots": { + "type": "boolean", + "description": "Should snapshot operation happening after another snapshot operation replace it, effectively abandoning the first snapshot and releasing it's view for garbage collection", + "default": false } } }, diff --git a/cli/src/operation_templater.rs b/cli/src/operation_templater.rs index 0b1384f155..45cc547c65 100644 --- a/cli/src/operation_templater.rs +++ b/cli/src/operation_templater.rs @@ -138,6 +138,8 @@ fn build_operation_keyword( metadata .tags .iter() + .filter(|(k, v)| *k != "snapshots" || *v != "1") + .sorted_unstable_by_key(|(k, _)| k.to_owned()) .map(|(key, value)| format!("{key}: {value}")) .join("\n") })), diff --git a/cli/tests/test_concurrent_operations.rs b/cli/tests/test_concurrent_operations.rs index 31e4147900..55dcd08438 100644 --- a/cli/tests/test_concurrent_operations.rs +++ b/cli/tests/test_concurrent_operations.rs @@ -69,10 +69,10 @@ fn test_concurrent_operations_auto_rebase() { test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "initial"]); let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]); insta::assert_snapshot!(stdout, @r###" - @ d5b4f16ef469 test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 + @ faefa12d4ced 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 123ed18e4c4c0d77428df41112bc02ffc83fb935 │ args: jj describe -m initial - ◉ e632e64d7fa1 test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 + ◉ 75021f3fc19d test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 │ snapshot working copy │ args: jj describe -m initial ◉ 6ac4339ad699 test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 @@ -184,16 +184,16 @@ fn test_concurrent_snapshot_wc_reloadable() { let template = r#"id ++ "\n" ++ description ++ "\n" ++ tags"#; let op_log_stdout = test_env.jj_cmd_success(&repo_path, &["op", "log", "-T", template]); insta::assert_snapshot!(op_log_stdout, @r###" - @ 1578600dd63556a22abef7cf6e7054a7e07468187ba31f79d0aa6a197b17004b7cd3e19d2fab1e6a00f2520b48d41969dbbb562c60d4c4af9436224f7f14ab83 + @ bc38b5d20b2843d546bf5d40d8e1897be131b2ede73b4de031d3b2c6ac394e454a24f52196687accc8d27eb99aaadd8e1d63a8d30a2cb22efae0e3232492bc86 │ commit 323b414dd255b51375d7f4392b7b2641ffe4289f │ args: jj commit -m 'new child1' - ◉ 90bb10893e980b606939a1f45f2aadf7de1eef65589ac5cd70e20dc20dfd0073c989b5ba0de70ce79a52d27aab5f5699eba66649b531530be5d13bc12c6bd926 + ◉ af69a63bdc3c88f12e0b1fbc673bce01eb46087b8c6629efd57cb9e68d72973d928f738f2e51504959dc5f531f68a15321f6c9560bca9862f5e850a872ce0c30 │ snapshot working copy │ args: jj commit -m 'new child1' - ◉ 6104865e95226d46d8c6f5bf43ab025e67f88da6e27f8d8cc598c6d058e333126380c4cb25ea49c841480efee82ce2c602d87b4d3f53b85b4e704af5e83cbdc9 + ◉ 2af019579b0b19d9a147df4779e587b92061fb5cd18fe9ba6d8a33cbeac205ced27c7f411df78278626993725ace7738da9352addb6699898e252c75f57f6f93 │ commit 3d918700494a9895696e955b85fa05eb0d314cc6 │ args: jj commit -m initial - ◉ 76137fc212ef44c53db04be2010ba0419db1fe30e31289bed7d1d0410bee7c3c93d8fd5f6d1b03d93801a2517c436cc1bc4cc512c740e2d88979e771a6fb3730 + ◉ c07152a7412fac7a7b5570176a1f8799e878c9ec75c7f69633943e6d59a555b7b778804c5b930ce3528e787c5e221c3874b470abb1ff97a2663a4df21f9e09d2 │ snapshot working copy │ args: jj commit -m initial ◉ 6ac4339ad6999058dd1806653ec37fc0091c1cc17419c750fddc5e8c1a6a77829e6dd70b3408403fb2c0b9839cf6bfd1c270f980674f7f89d4d78dc54082a8ef diff --git a/cli/tests/test_util_command.rs b/cli/tests/test_util_command.rs index 87f8e9a8ba..085c8d0286 100644 --- a/cli/tests/test_util_command.rs +++ b/cli/tests/test_util_command.rs @@ -89,7 +89,7 @@ fn test_gc_operation_log() { // Now this doesn't work. let stderr = test_env.jj_cmd_failure(&repo_path, &["debug", "operation", &op_to_remove]); insta::assert_snapshot!(stderr, @r###" - Error: No operation ID matching "f9400b5274c6f1cfa23afbc1956349897a6975116135a59ab19d941119cc9fad93d9668b8c7d913fbd68c543dcba40a8d44135a53996a9e8ea92d4b78ef52cb6" + Error: No operation ID matching "f642b8a41a8c8da5252704ea63464f01a08ee5d478b6bff26d113b9a53d5e6b632177554eb04a80578447259e0bb4f90a794465991db172290bbc54e75d0d370" "###); } diff --git a/cli/tests/test_working_copy.rs b/cli/tests/test_working_copy.rs index e54437b28b..ec5e3d8cfd 100644 --- a/cli/tests/test_working_copy.rs +++ b/cli/tests/test_working_copy.rs @@ -30,3 +30,45 @@ fn test_snapshot_large_file() { want this file to be snapshotted. Otherwise add it to your `.gitignore` file. "###); } + +#[test] +fn test_consecutive_snapshots() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.add_config(r#"snapshot.squash-consecutive-snapshots = true"#); + + test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "sample text"]); + // initial WC is a predecessor of a described WC now + + std::fs::write(repo_path.join("a"), "").unwrap(); + test_env.jj_cmd_success(&repo_path, &["files"]); + + std::fs::write(repo_path.join("b"), "").unwrap(); + test_env.jj_cmd_success(&repo_path, &["files"]); + + let stdout = test_env.jj_cmd_success(&repo_path, &["operation", "log"]); + insta::assert_snapshot!(stdout, @r###" + @ 70756af40ab9 test-username@host.example.com 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 + │ snapshot working copy + │ args: jj files + │ snapshots: 2 + ◉ 4f5b67be313f 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 'sample text' + ◉ 6ac4339ad699 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' + ◉ 1b0049c19762 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() + "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["obslog"]); + insta::assert_snapshot!(stdout, @r###" + @ qpvuntsm test.user@example.com 2001-02-03 04:05:10.000 +07:00 bf6e46e6 + │ sample text + ◉ qpvuntsm hidden test.user@example.com 2001-02-03 04:05:07.000 +07:00 230dd059 + (empty) (no description set) + "###); +} diff --git a/cli/tests/test_workspaces.rs b/cli/tests/test_workspaces.rs index 008eddbbad..9d032e5030 100644 --- a/cli/tests/test_workspaces.rs +++ b/cli/tests/test_workspaces.rs @@ -297,14 +297,14 @@ fn test_workspaces_conflicting_edits() { "###); let stderr = test_env.jj_cmd_failure(&secondary_path, &["st"]); insta::assert_snapshot!(stderr, @r###" - Error: The working copy is stale (not updated since operation d93fe4c5a6d1). + Error: The working copy is stale (not updated since operation 279268a28063). Hint: Run `jj workspace update-stale` to update it. See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-working-copy for more information. "###); // Same error on second run, and from another command let stderr = test_env.jj_cmd_failure(&secondary_path, &["log"]); insta::assert_snapshot!(stderr, @r###" - Error: The working copy is stale (not updated since operation d93fe4c5a6d1). + Error: The working copy is stale (not updated since operation 279268a28063). Hint: Run `jj workspace update-stale` to update it. See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-working-copy for more information. "###); @@ -384,7 +384,7 @@ fn test_workspaces_updated_by_other() { "###); let stderr = test_env.jj_cmd_failure(&secondary_path, &["st"]); insta::assert_snapshot!(stderr, @r###" - Error: The working copy is stale (not updated since operation d93fe4c5a6d1). + Error: The working copy is stale (not updated since operation 279268a28063). Hint: Run `jj workspace update-stale` to update it. See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-working-copy for more information. "###); @@ -436,11 +436,11 @@ fn test_workspaces_current_op_discarded_by_other() { ], ); insta::assert_snapshot!(stdout, @r###" - @ 09350a8134 abandon commit acb4b92517b20aa4ee2f3dc58d7c2373754d0b29a3df310dbabda5813f13c3730d28d6a1b6dd37f3b0c8c5c9adaead5dab242ffe7ecc2e5a6a534fe4c6639f89 - ◉ d8304661a2 Create initial working-copy commit in workspace secondary - ◉ 4ae0e83b98 add workspace 'secondary' - ◉ 00cc5ecf50 new empty commit - ◉ e0be7087ba snapshot working copy + @ 4cbdebdfc0 abandon commit acb4b92517b20aa4ee2f3dc58d7c2373754d0b29a3df310dbabda5813f13c3730d28d6a1b6dd37f3b0c8c5c9adaead5dab242ffe7ecc2e5a6a534fe4c6639f89 + ◉ b4b07f3859 Create initial working-copy commit in workspace secondary + ◉ 68faf85152 add workspace 'secondary' + ◉ ab73dcd486 new empty commit + ◉ d47599bc7f snapshot working copy ◉ d0bd64e0b3 add workspace 'default' ◉ 06a74719e6 initialize repo ◉ 0000000000 @@ -466,7 +466,7 @@ fn test_workspaces_current_op_discarded_by_other() { let (stdout, stderr) = test_env.jj_cmd_ok(&secondary_path, &["workspace", "update-stale"]); insta::assert_snapshot!(stderr, @r###" - Failed to read working copy's current operation; attempting recovery. Error message from read attempt: Object d8304661a23b0d8b9ecc517a465869d7c8b6563b460f029541fbe5246cc02718145c5ad8f7fed26863b46a68e79d609b878a7ea5a239baf530858e86e81e72d1 of type operation not found + Failed to read working copy's current operation; attempting recovery. Error message from read attempt: Object b4b07f38593831f5a539a17138b9673936dd936f83bf22349c88127b5633cb3d22f94903518a7b673addafe31cc30b3518c30b01e6387417e68594df507cf4b5 of type operation not found Created and checked out recovery commit 9d040f9a433c "###); insta::assert_snapshot!(stdout, @""); @@ -658,7 +658,7 @@ fn test_workspaces_forget_multi_transaction() { // the op log should have multiple workspaces forgotten in a single tx let stdout = test_env.jj_cmd_success(&main_path, &["op", "log", "--limit", "1"]); insta::assert_snapshot!(stdout, @r###" - @ ea093f0a1a06 test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00 + @ 119435f7fd1d test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00 │ forget workspaces second, third │ args: jj workspace forget second third "###); diff --git a/lib/src/settings.rs b/lib/src/settings.rs index be51e237c8..bb8c1f2c41 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -261,6 +261,12 @@ impl UserSettings { } } + pub fn squash_consecutive_snapshots(&self) -> bool { + self.config + .get_bool("snapshot.squash-consecutive-snapshots") + .unwrap_or(false) + } + // separate from sign_settings as those two are needed in pretty different // places pub fn signing_backend(&self) -> Option { diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 11c1b34c7a..6599f2f9fb 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -20,7 +20,7 @@ use itertools::Itertools as _; use crate::backend::Timestamp; use crate::index::ReadonlyIndex; -use crate::op_store::OperationMetadata; +use crate::op_store::{OperationId, OperationMetadata}; use crate::operation::Operation; use crate::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader, RepoLoaderError}; use crate::settings::UserSettings; @@ -32,6 +32,7 @@ pub struct Transaction { parent_ops: Vec, op_metadata: OperationMetadata, end_time: Option, + old_ops: Vec, } impl Transaction { @@ -39,11 +40,13 @@ impl Transaction { let parent_ops = vec![mut_repo.base_repo().operation().clone()]; let op_metadata = create_op_metadata(user_settings, "".to_string()); let end_time = user_settings.operation_timestamp(); + let old_ops = parent_ops.iter().map(|op| op.id().clone()).collect(); Transaction { mut_repo, parent_ops, op_metadata, end_time, + old_ops, } } @@ -51,6 +54,10 @@ impl Transaction { self.mut_repo.base_repo() } + pub fn parent_ops(&self) -> &[Operation] { + &self.parent_ops + } + pub fn set_tag(&mut self, key: String, value: String) { self.op_metadata.tags.insert(key, value); } @@ -74,12 +81,25 @@ impl Transaction { let repo_loader = self.base_repo().loader(); let base_repo = repo_loader.load_at(&ancestor_op)?; let other_repo = repo_loader.load_at(&other_op)?; + self.old_ops.push(other_op.id().clone()); self.parent_ops.push(other_op); let merged_repo = self.mut_repo(); merged_repo.merge(&base_repo, &other_repo); Ok(()) } + /// Replaces the parent ops with grandparent ops, effectively rewriting the + /// parent with self. + /// + /// Panics if self does not have exactly one parent. + pub fn replace_parent(&mut self) -> Result<(), RepoLoaderError> { + let [parent]: [Operation; 1] = std::mem::take(&mut self.parent_ops) + .try_into() + .expect("Merge or root operations cannot replace their parent"); + self.parent_ops = parent.parents().collect::>()?; + Ok(()) + } + /// Writes the transaction to the operation store and publishes it. pub fn commit(self, description: impl Into) -> Arc { self.write(description).publish() @@ -117,7 +137,7 @@ impl Transaction { .index_store() .write_index(mut_index, operation.id()) .unwrap(); - UnpublishedOperation::new(base_repo.loader(), operation, view, index) + UnpublishedOperation::new(base_repo.loader(), operation, view, index, self.old_ops) } } @@ -148,6 +168,7 @@ pub struct UnpublishedOperation { repo_loader: RepoLoader, data: Option, closed: bool, + old_ops: Vec, } impl UnpublishedOperation { @@ -156,6 +177,7 @@ impl UnpublishedOperation { operation: Operation, view: View, index: Box, + old_ops: Vec, ) -> Self { let data = Some(NewRepoData { operation, @@ -166,6 +188,7 @@ impl UnpublishedOperation { repo_loader, data, closed: false, + old_ops, } } @@ -179,7 +202,7 @@ impl UnpublishedOperation { let _lock = self.repo_loader.op_heads_store().lock(); self.repo_loader .op_heads_store() - .update_op_heads(data.operation.parent_ids(), data.operation.id()); + .update_op_heads(&self.old_ops, data.operation.id()); } let repo = self .repo_loader