diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 8ba78e29f8f..6bcd381963a 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -41,9 +41,10 @@ use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile}; use jj_lib::hex_util::to_reverse_hex; use jj_lib::id_prefix::IdPrefixContext; use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher}; +use jj_lib::merge::MergeBuilder; use jj_lib::merged_tree::MergedTree; use jj_lib::object_id::ObjectId; -use jj_lib::op_store::{OpStoreError, OperationId, WorkspaceId}; +use jj_lib::op_store::{OpStoreError, OperationId, RefTarget, WorkspaceId}; use jj_lib::op_walk::OpsetEvaluationError; use jj_lib::operation::Operation; use jj_lib::repo::{ @@ -389,6 +390,18 @@ impl ReadonlyUserRepo { } } +/// A branch that should be advanced to satisfy the "advance-branches" feature. +/// This is a helper for `WorkspaceCommandTransaction`. It provides a type-safe +/// way to separate the work of checking whether a branch can be advanced and +/// actually advancing it. Advancing the branch never fails, but can't be done +/// until the new `CommitId` is available. Splitting the work in this way also +/// allows us to identify eligible branches without actually moving them. +pub struct AdvanceableBranch { + name: String, + old_commit_id: CommitId, + old_target: RefTarget, +} + /// Provides utilities for writing a command that works on a [`Workspace`] /// (which most commands do). pub struct WorkspaceCommandHelper { @@ -1362,6 +1375,96 @@ Then run `jj squash` to move the resolution into the conflicted commit."#, Ok(()) } + + // Retrieves pattern literals from the user's advance-branches config and parses + // them. + fn advance_branches_patterns( + &self, + setting_key: &str, + ) -> Result, CommandError> { + let setting = format!("experimental-advance-branches.{setting_key}"); + let err_msg = format!("Error parsing pattern for {setting}"); + match self.settings.config().get_array(&setting) { + Ok(patterns) => patterns + .into_iter() + .map(|p| { + p.into_string() + .map_err(|e| config_error_with_message(&err_msg, e)) + .and_then(|s| { + StringPattern::parse(&s) + .map_err(|e| config_error_with_message(&err_msg, e)) + }) + }) + .collect(), + Err(_) => Ok(Vec::new()), + } + } + + // Returns a set of patterns that can be used to identify local branches + // which have the advance-branches setting enabled. + fn advance_branches_enabled_branches(&self) -> Result, CommandError> { + self.advance_branches_patterns("enabled-branches") + } + + // Returns a set of patterns that can be used to identify local branches + // which have the advance-branches setting disabled. + fn advance_branches_disabled_branches(&self) -> Result, CommandError> { + self.advance_branches_patterns("disabled-branches") + } + + /// Identifies branches which are eligible to be moved automatically during + /// `jj commit` and `jj new`. Whether a branch is eligible is determined by + /// its target and the user and repo config for "advance-branches". + /// + /// Returns a Vec of branches in `repo` that point to any of the `from` + /// commits and that are eligible to advance. The `from` commits are + /// typically the parents of the target commit of `jj commit` or `jj new`. + /// + /// Branches are not moved until + /// `WorkspaceCommandTransaction::advance_branches()` is called with the + /// `AdvanceableBranch`s returned by this function. + /// + /// Returns an empty `std::Vec` if no branches are eligible to advance. + pub fn get_advanceable_branches<'a>( + &self, + from: impl IntoIterator, + ) -> Result, CommandError> { + let enabled_branches = self.advance_branches_enabled_branches()?; + let disabled_branches = self.advance_branches_disabled_branches()?; + let allow_branch = |branch: &str| { + for disabled in disabled_branches.iter() { + if disabled.matches(branch) { + return false; + } + } + for enabled in enabled_branches.iter() { + if enabled.matches(branch) { + return true; + } + } + false + }; + + // Return early if we know that there's no work to do. + if enabled_branches.is_empty() { + return Ok(Vec::new()); + } + + let mut advanceable_branches = Vec::new(); + for from_commit in from { + for (name, target) in self.repo().view().local_branches_for_commit(from_commit) { + if allow_branch(name) { + advanceable_branches.push(AdvanceableBranch { + name: name.to_owned(), + old_commit_id: from_commit.clone(), + old_target: target.clone(), + }); + } + } + } + + Ok(advanceable_branches) + } } /// A [`Transaction`] tied to a particular workspace. @@ -1448,6 +1551,34 @@ impl WorkspaceCommandTransaction<'_> { pub fn into_inner(self) -> Transaction { self.tx } + + /// Moves each branch in `branches` from an old commit it's associated with + /// (configured by `get_advanceable_branches`) to the `move_to` commit. If + /// the branch is conflicted before the update, it will remain conflicted + /// after the update, but the conflict will involve the `move_to` commit + /// instead of the old commit. + pub fn advance_branches(&mut self, branches: Vec, move_to: &CommitId) { + for branch in branches { + // We are going to remove the old commit and add the new commit. The + // removed commit must be listed first in order for the `MergeBuilder` + // to recognize that it's being removed. + let remove_add = [Some(branch.old_commit_id), Some(move_to.clone())]; + let new_target = RefTarget::from_merge( + MergeBuilder::from_iter( + branch + .old_target + .as_merge() + .iter() + .chain(&remove_add) + .cloned(), + ) + .build() + .simplify(), + ); + self.mut_repo() + .set_local_branch_target(&branch.name, new_target); + } + } } fn find_workspace_dir(cwd: &Path) -> &Path { diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index 2ad459046db..573afc80d8c 100644 --- a/cli/src/commands/commit.rs +++ b/cli/src/commands/commit.rs @@ -54,6 +54,7 @@ pub(crate) fn cmd_commit( .get_wc_commit_id() .ok_or_else(|| user_error("This command requires a working copy"))?; let commit = workspace_command.repo().store().get_commit(commit_id)?; + let advanceable_branches = workspace_command.get_advanceable_branches(commit.parent_ids())?; let matcher = workspace_command.matcher_from_values(&args.paths)?; let diff_selector = workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?; @@ -119,6 +120,11 @@ new working-copy commit. commit.tree_id().clone(), ) .write()?; + + if !advanceable_branches.is_empty() { + tx.advance_branches(advanceable_branches, new_commit.id()); + } + for workspace_id in workspace_ids { tx.mut_repo().edit(workspace_id, &new_wc_commit).unwrap(); } diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index a6bca7fffc0..3267b1b7a56 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -385,6 +385,26 @@ } } }, + "experimental-advance-branches": { + "type": "object", + "description": "Settings controlling the 'advance-branches' feature which moves branches forward when new commits are created.", + "properties": { + "enabled-branches": { + "type": "array", + "description": "Patterns used to identify branches which may be advanced.", + "items": { + "type": "string" + } + }, + "disabled-branches": { + "type": "array", + "description": "Patterns used to identify branches which are not advanced. Takes precedence over 'enabled-branches'.", + "items": { + "type": "string" + } + } + } + }, "signing": { "type": "object", "description": "Settings for verifying and creating cryptographic commit signatures", diff --git a/cli/tests/runner.rs b/cli/tests/runner.rs index dcd71c016a5..20801685b26 100644 --- a/cli/tests/runner.rs +++ b/cli/tests/runner.rs @@ -9,6 +9,7 @@ fn test_no_forgotten_test_files() { } mod test_abandon_command; +mod test_advance_branches; mod test_alias; mod test_branch_command; mod test_builtin_aliases; diff --git a/cli/tests/test_advance_branches.rs b/cli/tests/test_advance_branches.rs new file mode 100644 index 00000000000..384b1de3a04 --- /dev/null +++ b/cli/tests/test_advance_branches.rs @@ -0,0 +1,236 @@ +use std::path::Path; + +use itertools::Itertools; + +use crate::common::TestEnvironment; + +fn get_log_output_with_branches(test_env: &TestEnvironment, cwd: &Path) -> String { + let template = r#"commit_id.short() ++ " br:{" ++ local_branches ++ "} dsc: " ++ description"#; + test_env.jj_cmd_success(cwd, &["log", "-T", template]) +} + +fn enable_advance_branches_for_patterns(test_env: &TestEnvironment, cwd: &Path, patterns: &[&str]) { + #[rustfmt::skip] + let pattern_string: String = patterns.iter().map(|x| format!("\"{}\"", x)).join(","); + test_env.jj_cmd_success( + cwd, + &[ + "config", + "set", + "--repo", + "experimental-advance-branches.enabled-branches", + &format!("[{}]", pattern_string), + ], + ); +} + +fn set_advance_branches(test_env: &TestEnvironment, cwd: &Path, value: bool) { + if value { + enable_advance_branches_for_patterns(test_env, cwd, &["glob:*"]); + } else { + enable_advance_branches_for_patterns(test_env, cwd, &[""]); + } +} + +// Check that enabling and disabling advance-branches works as expected. +#[test] +fn test_advance_branches_enabled() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + // First, test with advance-branches enabled. Start by creating a branch on the + // root commit. + set_advance_branches(&test_env, &workspace_path, true); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@-", "test_branch"], + ); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 230dd059e1b0 br:{} dsc: + ◉ 000000000000 br:{test_branch} dsc: + "###); + + // Run jj commit, which will advance the branch pointing to @-. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 24bb7f9da598 br:{} dsc: + ◉ 95f2456c4bbd br:{test_branch} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + + // Now disable advance branches and commit again. The branch shouldn't move. + set_advance_branches(&test_env, &workspace_path, false); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=second"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ b29edd893970 br:{} dsc: + ◉ ebf7d96fb6ad br:{} dsc: second + ◉ 95f2456c4bbd br:{test_branch} dsc: first + ◉ 000000000000 br:{} dsc: + "###); +} + +// Check that only a branch pointing to @- advances. Branches pointing to @ are +// not advanced. +#[test] +fn test_advance_branches_at_minus() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + set_advance_branches(&test_env, &workspace_path, true); + test_env.jj_cmd_ok(&workspace_path, &["branch", "create", "test_branch"]); + + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 230dd059e1b0 br:{test_branch} dsc: + ◉ 000000000000 br:{} dsc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 24bb7f9da598 br:{} dsc: + ◉ 95f2456c4bbd br:{test_branch} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + + // Create a second branch pointing to @. On the next commit, only the first + // branch, which points to @-, will advance. + test_env.jj_cmd_ok(&workspace_path, &["branch", "create", "test_branch2"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=second"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ b29edd893970 br:{} dsc: + ◉ ebf7d96fb6ad br:{test_branch test_branch2} dsc: second + ◉ 95f2456c4bbd br:{} dsc: first + ◉ 000000000000 br:{} dsc: + "###); +} + +// Test that per-branch overrides invert the behavior of +// experimental-advance-branches.enabled. +#[test] +fn test_advance_branches_overrides() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + // advance-branches is disabled by default. + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@-", "test_branch"], + ); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 230dd059e1b0 br:{} dsc: + ◉ 000000000000 br:{test_branch} dsc: + "###); + + // Commit will not advance the branch since advance-branches is disabled. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 7e3a6f5e0f15 br:{} dsc: + ◉ 307e33f70413 br:{} dsc: first + ◉ 000000000000 br:{test_branch} dsc: + "###); + + // Now enable advance branches for "test_branch", move the branch, and commit + // again. + test_env.add_config( + r#"[experimental-advance-branches] + enabled-branches = ["test_branch"] + "#, + ); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "set", "test_branch", "-r", "@-"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 7e3a6f5e0f15 br:{} dsc: + ◉ 307e33f70413 br:{test_branch} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=second"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 8c1bd3e7de60 br:{} dsc: + ◉ 468d1ab20fb3 br:{test_branch} dsc: second + ◉ 307e33f70413 br:{} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + + // Now disable advance branches for "test_branch" and "second_branch", which + // we will use later. Disabling always takes precedence over enabling. + test_env.add_config( + r#"[experimental-advance-branches] + enabled-branches = ["test_branch", "second_branch"] + disabled-branches = ["test_branch"] + "#, + ); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=third"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 5888a83948dd br:{} dsc: + ◉ 50e9c28e6d85 br:{} dsc: third + ◉ 468d1ab20fb3 br:{test_branch} dsc: second + ◉ 307e33f70413 br:{} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + + // If we create a new branch at @- and move test_branch there as well. When + // we commit, only "second_branch" will advance since "test_branch" is disabled. + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "second_branch", "-r", "@-"], + ); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "set", "test_branch", "-r", "@-"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 5888a83948dd br:{} dsc: + ◉ 50e9c28e6d85 br:{second_branch test_branch} dsc: third + ◉ 468d1ab20fb3 br:{} dsc: second + ◉ 307e33f70413 br:{} dsc: first + ◉ 000000000000 br:{} dsc: + "###); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=fourth"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 666d42aedca7 br:{} dsc: + ◉ f23aa63eeb99 br:{second_branch} dsc: fourth + ◉ 50e9c28e6d85 br:{test_branch} dsc: third + ◉ 468d1ab20fb3 br:{} dsc: second + ◉ 307e33f70413 br:{} dsc: first + ◉ 000000000000 br:{} dsc: + "###); +} + +// If multiple eligible branches point to @-, all of them will be advanced. +#[test] +fn test_advance_branches_multiple_branches() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let workspace_path = test_env.env_root().join("repo"); + + set_advance_branches(&test_env, &workspace_path, true); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@-", "first_branch"], + ); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@-", "second_branch"], + ); + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ 230dd059e1b0 br:{} dsc: + ◉ 000000000000 br:{first_branch second_branch} dsc: + "###); + + // Both branches are eligible and both will advance. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ f307e5d9f90b br:{} dsc: + ◉ 0fca5c9228e6 br:{first_branch second_branch} dsc: first + ◉ 000000000000 br:{} dsc: + "###); +} diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 1eb572469bd..b6a2cdc785e 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -288,6 +288,12 @@ impl UserSettings { _ => symbol.unwrap_or_else(|_| fallback.to_owned()), } } + + pub fn advance_branches(&self) -> bool { + self.config + .get_bool("experimental-advance-branches.enabled") + .unwrap_or(false) + } } /// This Rng uses interior mutability to allow generating random values using an diff --git a/lib/src/view.rs b/lib/src/view.rs index 4a1b3af1bb7..1125bd0b2b3 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -123,6 +123,16 @@ impl View { .map(|(name, target)| (name.as_ref(), target)) } + /// Iterates local branches `(name, target)` in lexicographical order where + /// the target adds `commit_id`. + pub fn local_branches_for_commit<'a: 'b, 'b>( + &'a self, + commit_id: &'b CommitId, + ) -> impl Iterator + 'b { + self.local_branches() + .filter(|(_, target)| target.added_ids().contains(commit_id)) + } + /// Iterates local branch `(name, target)`s matching the given pattern. /// Entries are sorted by `name`. pub fn local_branches_matching<'a: 'b, 'b>(