From 006171d8555c88851b691a8d924d229bb26f44d0 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Fri, 16 Feb 2024 14:01:34 -0500 Subject: [PATCH 1/4] Implement advance-branches for jj commit ## Feature Description If enabled in the user or repository settings, the local branches pointing to the parents of the revision targeted by `jj commit` will be advanced to the newly created commit. Support for `jj new` will be added in a future change. This behavior can be enabled by default for all branches by setting the following in the config.toml: ``` [experimental-advance-branches] enabled-branches = ["glob:*"] ``` Specific branches can also be disabled: ``` [experimental-advance-branches] enabled-branches = ["glob:*"] disabled-branches = ["main"] ``` Branches that match a disabled pattern will not be advanced, even if they also match an enabled pattern. This implements feature request #2338. --- cli/src/cli_util.rs | 128 +++++++++++++++- cli/src/commands/commit.rs | 5 + cli/src/config-schema.json | 20 +++ cli/tests/runner.rs | 1 + cli/tests/test_advance_branches.rs | 236 +++++++++++++++++++++++++++++ lib/src/view.rs | 10 ++ 6 files changed, 399 insertions(+), 1 deletion(-) create mode 100644 cli/tests/test_advance_branches.rs diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 1c37c99fbb..0a5a2ffa86 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -43,7 +43,7 @@ use jj_lib::id_prefix::IdPrefixContext; use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher}; 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 +389,77 @@ 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 and +/// return config errors to the user early. +pub struct AdvanceableBranch { + name: String, + old_commit_id: CommitId, +} + +/// Helper for parsing and evaluating settings for the advance-branches feature. +/// Settings are configured in the jj config.toml as lists of [`StringPattern`]s +/// for enabled and disabled branches. Example: +/// ```toml +/// [experimental-advance-branches] +/// # Enable the feature for all branches except "main". +/// enabled-branches = ["glob:*"] +/// disabled-branches = ["main"] +/// ``` +struct AdvanceBranchesSettings { + enabled_branches: Vec, + disabled_branches: Vec, +} + +impl AdvanceBranchesSettings { + fn from_config(config: &config::Config) -> Result { + let get_setting = |setting_key| { + let setting = format!("experimental-advance-branches.{setting_key}"); + match config.get::>(&setting).optional()? { + Some(patterns) => patterns + .into_iter() + .map(|s| { + StringPattern::parse(&s).map_err(|e| { + config_error_with_message( + format!("Error parsing '{s}' for {setting}"), + e, + ) + }) + }) + .collect(), + None => Ok(Vec::new()), + } + }; + Ok(Self { + enabled_branches: get_setting("enabled-branches")?, + disabled_branches: get_setting("disabled-branches")?, + }) + } + + /// Returns true if the advance-branches feature is enabled for + /// `branch_name`. + fn branch_is_eligible(&self, branch_name: &str) -> bool { + if self + .disabled_branches + .iter() + .any(|d| d.matches(branch_name)) + { + return false; + } + self.enabled_branches.iter().any(|e| e.matches(branch_name)) + } + + /// Returns true if the config includes at least one "enabled-branches" + /// pattern. + fn feature_enabled(&self) -> bool { + !self.enabled_branches.is_empty() + } +} + /// Provides utilities for writing a command that works on a [`Workspace`] /// (which most commands do). pub struct WorkspaceCommandHelper { @@ -1362,6 +1433,44 @@ Then run `jj squash` to move the resolution into the conflicted commit."#, Ok(()) } + + /// 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 ab_settings = AdvanceBranchesSettings::from_config(self.settings.config())?; + if !ab_settings.feature_enabled() { + // Return early if we know that there's no work to do. + return Ok(Vec::new()); + } + + let mut advanceable_branches = Vec::new(); + for from_commit in from { + for (name, _) in self.repo().view().local_branches_for_commit(from_commit) { + if ab_settings.branch_is_eligible(name) { + advanceable_branches.push(AdvanceableBranch { + name: name.to_owned(), + old_commit_id: from_commit.clone(), + }); + } + } + } + + Ok(advanceable_branches) + } } /// A [`Transaction`] tied to a particular workspace. @@ -1448,6 +1557,23 @@ 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 { + // This removes the old commit ID from the branch's RefTarget and + // replaces it with the `move_to` ID. + self.mut_repo().merge_local_branch( + &branch.name, + &RefTarget::normal(branch.old_commit_id), + &RefTarget::normal(move_to.clone()), + ); + } + } } fn find_workspace_dir(cwd: &Path) -> &Path { diff --git a/cli/src/commands/commit.rs b/cli/src/commands/commit.rs index 2ad459046d..5e1d35750f 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,10 @@ new working-copy commit. commit.tree_id().clone(), ) .write()?; + + // Does nothing if there's no branches to advance. + 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 a6bca7fffc..3267b1b7a5 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 dcd71c016a..20801685b2 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 0000000000..384b1de3a0 --- /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/view.rs b/lib/src/view.rs index 4a1b3af1bb..1125bd0b2b 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>( From 4e83afd06b2d1adcc0fbc9669fa358545deb96e5 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Thu, 14 Mar 2024 17:15:18 -0400 Subject: [PATCH 2/4] Allow advance_branches tests to be parameterized In a future commit these tests will run with both `jj commit` and `jj new` since both will have the same semantics for advancing the branch pointer. Parameterizing the tests allows us to run both variants without duplicating the test bodies. Since the commit IDs are different when `jj describe` + `jj new` is used instead of `jj commit`, this commit also drops the commit IDs from the test snapshots. This is fine because the commit IDs are not important for these tests. --- cli/tests/test_advance_branches.rs | 222 +++++++++++++++++------------ 1 file changed, 129 insertions(+), 93 deletions(-) diff --git a/cli/tests/test_advance_branches.rs b/cli/tests/test_advance_branches.rs index 384b1de3a0..af110015ce 100644 --- a/cli/tests/test_advance_branches.rs +++ b/cli/tests/test_advance_branches.rs @@ -1,116 +1,133 @@ use std::path::Path; -use itertools::Itertools; +use test_case::test_case; 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"#; + // Don't include commit IDs since they will be different depending on + // whether the test runs with `jj commit` or `jj describe` + `jj new`. + let template = r#""branches{" ++ local_branches ++ "} desc: " ++ 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:*"]); +fn set_advance_branches(test_env: &TestEnvironment, enabled: bool) { + if enabled { + test_env.add_config( + r#"[experimental-advance-branches] + enabled-branches = ["glob:*"] + "#, + ); } else { - enable_advance_branches_for_patterns(test_env, cwd, &[""]); + test_env.add_config( + r#"[experimental-advance-branches] + enabled-branches = [""] + "#, + ); } } +// Runs a command in the specified test environment and workspace path that +// describes the current commit with `commit_message` and creates a new commit +// on top of it. +type CommitFn = fn(env: &TestEnvironment, workspace_path: &Path, commit_message: &str); + +// Implements CommitFn using the `jj commit` command. +fn commit_cmd(env: &TestEnvironment, workspace_path: &Path, commit_message: &str) { + env.jj_cmd_ok(workspace_path, &["commit", "-m", commit_message]); +} + // Check that enabling and disabling advance-branches works as expected. -#[test] -fn test_advance_branches_enabled() { +#[test_case(commit_cmd ; "commit")] +fn test_advance_branches_enabled(make_commit: CommitFn) { 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); + set_advance_branches(&test_env, true); test_env.jj_cmd_ok( &workspace_path, &["branch", "create", "-r", "@-", "test_branch"], ); // Check the initial state of the repo. + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" - @ 230dd059e1b0 br:{} dsc: - ◉ 000000000000 br:{test_branch} dsc: + @ branches{} desc: + ◉ branches{test_branch} desc: "###); + } // Run jj commit, which will advance the branch pointing to @-. - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + make_commit(&test_env, &workspace_path, "first"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{test_branch} desc: first + ◉ branches{} desc: "###); + } // 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"]); + set_advance_branches(&test_env, false); + make_commit(&test_env, &workspace_path, "second"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: "###); + } } // Check that only a branch pointing to @- advances. Branches pointing to @ are // not advanced. -#[test] -fn test_advance_branches_at_minus() { +#[test_case(commit_cmd ; "commit")] +fn test_advance_branches_at_minus(make_commit: CommitFn) { 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); + set_advance_branches(&test_env, true); test_env.jj_cmd_ok(&workspace_path, &["branch", "create", "test_branch"]); + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" - @ 230dd059e1b0 br:{test_branch} dsc: - ◉ 000000000000 br:{} dsc: + @ branches{test_branch} desc: + ◉ branches{} desc: "###); + } - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + make_commit(&test_env, &workspace_path, "first"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{test_branch} desc: first + ◉ branches{} desc: "###); + } // 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"]); + make_commit(&test_env, &workspace_path, "second"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{test_branch test_branch2} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: "###); + } } // Test that per-branch overrides invert the behavior of // experimental-advance-branches.enabled. -#[test] -fn test_advance_branches_overrides() { +#[test_case(commit_cmd ; "commit")] +fn test_advance_branches_overrides(make_commit: CommitFn) { 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"); @@ -122,18 +139,22 @@ fn test_advance_branches_overrides() { ); // Check the initial state of the repo. + insta::allow_duplicates! { insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" - @ 230dd059e1b0 br:{} dsc: - ◉ 000000000000 br:{test_branch} dsc: + @ branches{} desc: + ◉ branches{test_branch} desc: "###); + } // Commit will not advance the branch since advance-branches is disabled. - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + make_commit(&test_env, &workspace_path, "first"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{} desc: first + ◉ branches{test_branch} desc: "###); + } // Now enable advance branches for "test_branch", move the branch, and commit // again. @@ -146,18 +167,22 @@ fn test_advance_branches_overrides() { &workspace_path, &["branch", "set", "test_branch", "-r", "@-"], ); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{test_branch} desc: first + ◉ branches{} desc: "###); - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=second"]); + } + make_commit(&test_env, &workspace_path, "second"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{test_branch} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: "###); + } // Now disable advance branches for "test_branch" and "second_branch", which // we will use later. Disabling always takes precedence over enabling. @@ -167,14 +192,16 @@ fn test_advance_branches_overrides() { disabled-branches = ["test_branch"] "#, ); - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=third"]); + make_commit(&test_env, &workspace_path, "third"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{} desc: third + ◉ branches{test_branch} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: "###); + } // 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. @@ -186,32 +213,36 @@ fn test_advance_branches_overrides() { &workspace_path, &["branch", "set", "test_branch", "-r", "@-"], ); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{second_branch test_branch} desc: third + ◉ branches{} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: "###); - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=fourth"]); + } + make_commit(&test_env, &workspace_path, "fourth"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{second_branch} desc: fourth + ◉ branches{test_branch} desc: third + ◉ branches{} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: "###); + } } // If multiple eligible branches point to @-, all of them will be advanced. -#[test] -fn test_advance_branches_multiple_branches() { +#[test_case(commit_cmd ; "commit")] +fn test_advance_branches_multiple_branches(make_commit: CommitFn) { 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); + set_advance_branches(&test_env, true); test_env.jj_cmd_ok( &workspace_path, &["branch", "create", "-r", "@-", "first_branch"], @@ -220,17 +251,22 @@ fn test_advance_branches_multiple_branches() { &workspace_path, &["branch", "create", "-r", "@-", "second_branch"], ); + + insta::allow_duplicates! { // 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: + @ branches{} desc: + ◉ branches{first_branch second_branch} desc: "###); + } // Both branches are eligible and both will advance. - test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=first"]); + make_commit(&test_env, &workspace_path, "first"); + insta::allow_duplicates! { 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: + @ branches{} desc: + ◉ branches{first_branch second_branch} desc: first + ◉ branches{} desc: "###); + } } From 8a218d2d7c313632eac83d11199f30028169dc83 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Mon, 18 Mar 2024 23:35:10 -0400 Subject: [PATCH 3/4] Implement advance-branches for jj new --- cli/src/commands/new.rs | 28 ++++++- cli/tests/test_advance_branches.rs | 130 ++++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 4 deletions(-) diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 1b10eca862..99b5b9cf7a 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -23,7 +23,8 @@ use jj_lib::rewrite::{merge_commit_trees, rebase_commit}; use tracing::instrument; use crate::cli_util::{ - resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, RevisionArg, + resolve_multiple_nonempty_revsets_default_single, short_commit_hash, AdvanceableBranch, + CommandHelper, RevisionArg, WorkspaceCommandHelper, }; use crate::command_error::{user_error, CommandError}; use crate::description_util::join_message_paragraphs; @@ -101,6 +102,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", .into_iter() .collect_vec(); let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec(); + let advanceable_branches = get_advanceable_branches(args, &workspace_command, &target_commits)?; let mut tx = workspace_command.start_transaction(); let mut num_rebased; let new_commit; @@ -138,11 +140,11 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", .set_description(join_message_paragraphs(&args.message_paragraphs)) .write()?; num_rebased = target_ids.len(); - for child_commit in target_commits { + for child_commit in &target_commits { rebase_commit( command.settings(), tx.mut_repo(), - &child_commit, + child_commit, &[new_commit.clone()], )?; } @@ -199,6 +201,26 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", if num_rebased > 0 { writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; } + + // Does nothing if there's no branches to advance. + tx.advance_branches(advanceable_branches, target_commits[0].id()); + tx.finish(ui, "new empty commit")?; Ok(()) } + +// Branches are only advanced if `jj new` has a single target commit and the +// new commit is not being inserted before or after the target. +fn get_advanceable_branches( + args: &NewArgs, + ws: &WorkspaceCommandHelper, + target_commits: &[Commit], +) -> Result, CommandError> { + let should_advance_branches = + target_commits.len() == 1 && !args.insert_before && !args.insert_after; + if should_advance_branches { + ws.get_advanceable_branches(target_commits[0].parent_ids()) + } else { + Ok(Vec::new()) + } +} diff --git a/cli/tests/test_advance_branches.rs b/cli/tests/test_advance_branches.rs index af110015ce..f5b23cf12e 100644 --- a/cli/tests/test_advance_branches.rs +++ b/cli/tests/test_advance_branches.rs @@ -21,7 +21,7 @@ fn set_advance_branches(test_env: &TestEnvironment, enabled: bool) { } else { test_env.add_config( r#"[experimental-advance-branches] - enabled-branches = [""] + enabled-branches = [] "#, ); } @@ -37,8 +37,15 @@ fn commit_cmd(env: &TestEnvironment, workspace_path: &Path, commit_message: &str env.jj_cmd_ok(workspace_path, &["commit", "-m", commit_message]); } +// Implements CommitFn using the `jj describe` and `jj new`. +fn describe_new_cmd(env: &TestEnvironment, workspace_path: &Path, commit_message: &str) { + env.jj_cmd_ok(workspace_path, &["describe", "-m", commit_message]); + env.jj_cmd_ok(workspace_path, &["new"]); +} + // Check that enabling and disabling advance-branches works as expected. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_enabled(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -86,6 +93,7 @@ fn test_advance_branches_enabled(make_commit: CommitFn) { // Check that only a branch pointing to @- advances. Branches pointing to @ are // not advanced. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_at_minus(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -127,6 +135,7 @@ fn test_advance_branches_at_minus(make_commit: CommitFn) { // Test that per-branch overrides invert the behavior of // experimental-advance-branches.enabled. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_overrides(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -237,6 +246,7 @@ fn test_advance_branches_overrides(make_commit: CommitFn) { // If multiple eligible branches point to @-, all of them will be advanced. #[test_case(commit_cmd ; "commit")] +#[test_case(describe_new_cmd; "new")] fn test_advance_branches_multiple_branches(make_commit: CommitFn) { let test_env = TestEnvironment::default(); test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); @@ -270,3 +280,121 @@ fn test_advance_branches_multiple_branches(make_commit: CommitFn) { "###); } } + +// Call `jj new` on an interior commit and see that the branch pointing to its +// parent's parent is advanced. +#[test] +fn test_new_advance_branches_interior() { + 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, true); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: + "###); + + // Create a gap in the commits for us to insert our new commit with --before. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "third"]); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@---", "test_branch"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: third + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["new", "-r", "@--"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + │ ◉ branches{} desc: third + ├─╯ + ◉ branches{test_branch} desc: second + ◉ branches{} desc: first + ◉ branches{} desc: + "###); +} + +// If the `--before` flag is passed to `jj new`, branches are not advanced. +#[test] +fn test_new_advance_branches_before() { + 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, true); + + // Check the initial state of the repo. + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: + "###); + + // Create a gap in the commits for us to insert our new commit with --before. + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "second"]); + test_env.jj_cmd_ok(&workspace_path, &["commit", "-m", "third"]); + test_env.jj_cmd_ok( + &workspace_path, + &["branch", "create", "-r", "@---", "test_branch"], + ); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: third + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["new", "--before", "-r", "@-"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + ◉ branches{} desc: third + @ branches{} desc: + ◉ branches{} desc: second + ◉ branches{test_branch} desc: first + ◉ branches{} desc: + "###); +} + +// If the `--after` flag is passed to `jj new`, branches are not advanced. +#[test] +fn test_new_advance_branches_after() { + 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, 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###" + @ branches{} desc: + ◉ branches{test_branch} desc: + "###); + + test_env.jj_cmd_ok(&workspace_path, &["describe", "-m", "first"]); + test_env.jj_cmd_ok(&workspace_path, &["new", "--after"]); + insta::assert_snapshot!(get_log_output_with_branches(&test_env, &workspace_path), @r###" + @ branches{} desc: + ◉ branches{} desc: first + ◉ branches{test_branch} desc: + "###); +} From 2cd99190b1a1181425a5ce73137ea13eaae73810 Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Fri, 23 Feb 2024 14:52:31 -0500 Subject: [PATCH 4/4] Don't detatch Git HEAD when advance-branches is enabled for a branch When setting the working copy commit, if a single branch points to a parent of the working copy and advance-branches is enabled for that branch, set Git HEAD to the branch instead of detatching at the parent commit. --- cli/src/cli_util.rs | 25 ++++++++++++++++++++++--- lib/src/git.rs | 28 ++++++++++++++++++++++++++-- lib/tests/test_git.rs | 8 ++++---- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0a5a2ffa86..0c399368a3 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1266,11 +1266,30 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin .transpose()?; if self.working_copy_shared_with_git { let git_repo = self.git_backend().unwrap().open_git_repo()?; - if let Some(wc_commit) = &maybe_new_wc_commit { - git::reset_head(tx.mut_repo(), &git_repo, wc_commit)?; - } + // TODO(emesterhazy): Is it problematic that we're exporting these + // refs before resetting head? If the ref export fails, the head + // won't be reset. We could defer returning the error until after + // HEAD is reset, but we need to try to export the refs first so + // that we can set HEAD to an advanceable branch if one exists. let failed_branches = git::export_refs(tx.mut_repo())?; print_failed_git_export(ui, &failed_branches)?; + if let Some(wc_commit) = &maybe_new_wc_commit { + // If there's a single branch pointing to one of the working + // copy's parents and advance-branches is enabled for it, then + // set the Git HEAD to that branch instead of detaching at the + // commit. Ignore errors since it's too late to bail out without + // losing any work. + let parent_branch = match self.get_advanceable_branches(wc_commit.parent_ids()) { + Ok(branches) if branches.len() == 1 => Some(branches[0].name.clone()), + _ => None, + }; + git::reset_head( + tx.mut_repo(), + &git_repo, + wc_commit, + parent_branch.as_deref(), + )?; + } } self.user_repo = ReadonlyUserRepo::new(tx.commit(description)); self.report_repo_changes(ui, &old_repo)?; diff --git a/lib/src/git.rs b/lib/src/git.rs index ee3748c18b..eef30880ee 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -904,20 +904,43 @@ fn update_git_ref( } /// Sets `HEAD@git` to the parent of the given working-copy commit and resets -/// the Git index. +/// the Git index. If `try_branch` points to the parent of the given +/// working-copy commit, then the Git HEAD is set to the branch instead of being +/// detached. pub fn reset_head( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, wc_commit: &Commit, + try_branch: Option<&str>, ) -> Result<(), git2::Error> { let first_parent_id = &wc_commit.parent_ids()[0]; + // Try to look up the branch reference if `try_branch` is provided, but + // don't return an error and proceed to detach HEAD it the lookup fails. + // Setting HEAD to a branch instead of a commit provides a better Git + // interop experience for CLI users that enable the "advance-branches" + // feature. + let branch_ref = if let Some(branch) = try_branch { + git_repo.resolve_reference_from_short_name(branch).ok() + } else { + None + }; + if first_parent_id != mut_repo.store().root_commit_id() { let first_parent = RefTarget::normal(first_parent_id.clone()); let git_head = mut_repo.view().git_head(); let new_git_commit_id = Oid::from_bytes(first_parent_id.as_bytes()).unwrap(); let new_git_commit = git_repo.find_commit(new_git_commit_id)?; if git_head != &first_parent { - git_repo.set_head_detached(new_git_commit_id)?; + if let Some(branch_ref) = branch_ref { + let branch_commit = branch_ref.peel_to_commit()?.id(); + if branch_commit == new_git_commit_id { + // Set Git HEAD to the branch pointing to the parent Git + // commit instead of detaching. + git_repo.set_head_bytes(branch_ref.name_bytes())?; + } + } else { + git_repo.set_head_detached(new_git_commit_id)?; + } mut_repo.set_git_head_target(first_parent); } git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?; @@ -941,6 +964,7 @@ pub fn reset_head( git_repo.cleanup_state()?; mut_repo.set_git_head_target(RefTarget::absent()); } + Ok(()) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 5bdf2c0970..c6b33b3acc 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -1942,7 +1942,7 @@ fn test_reset_head_to_root() { .unwrap(); // Set Git HEAD to commit2's parent (i.e. commit1) - git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap(); + git::reset_head(tx.mut_repo(), &git_repo, &commit2, None).unwrap(); assert!(git_repo.head().is_ok()); assert_eq!( tx.mut_repo().git_head(), @@ -1950,7 +1950,7 @@ fn test_reset_head_to_root() { ); // Set Git HEAD back to root - git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap(); + git::reset_head(tx.mut_repo(), &git_repo, &commit1, None).unwrap(); assert!(git_repo.head().is_err()); assert!(tx.mut_repo().git_head().is_absent()); @@ -1958,7 +1958,7 @@ fn test_reset_head_to_root() { git_repo .reference("refs/jj/root", git_id(&commit1), false, "") .unwrap(); - git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap(); + git::reset_head(tx.mut_repo(), &git_repo, &commit2, None).unwrap(); assert!(git_repo.head().is_ok()); assert_eq!( tx.mut_repo().git_head(), @@ -1967,7 +1967,7 @@ fn test_reset_head_to_root() { assert!(git_repo.find_reference("refs/jj/root").is_ok()); // Set Git HEAD back to root - git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap(); + git::reset_head(tx.mut_repo(), &git_repo, &commit1, None).unwrap(); assert!(git_repo.head().is_err()); assert!(tx.mut_repo().git_head().is_absent()); // The placeholder ref should be deleted