feat(move): implement move --reparent like amend --reparent by bryango · Pull Request #1631 · arxanas/git-branchless

bryango · 2025-11-30T14:30:23Z

Closes #1537. This is done in the following commits:

refactor out the amend --reparent logic into a .reparent_subtree method for the RebasePlanBuilder
move the --reparent flag into MoveOptions which may be generalized for move and possibly restack, split etc ~~in the future (currently not implemented and would be a no-op)~~ (now implemented, see below)
actually implement --reparent for the move command with a few builder.reparent_subtree calls. Currently --reparent conflicts with --fixup since ~~I am not yet sure how it should be implemented (help wanted)~~ the behavior for --fixup --parent may be ambiguous or confusing for users
add tests for move --reparent and showcase its intended behavior.
refactor: use eyre::bail! instead of assert!
implement restack --reparent
implement split --reparent (for --stack or --detach)
implement sync --reparent

I would like to test this more in real life and see if it's actually working, but I think it would be good to post it here early to gather some feedback: is this feature desired? Would this be the correct way to implement it? I have very limited knowledge on the codebase so all feedbacks are much welcomed. Thank you!

claytonrcarter

Thank you for this. The code looks simple enough, and solid. And the test seems thorough enough for the questions that I came in with.

Currently --reparent conflicts with --fixup since I am not yet sure how it should be implemented

I suspect that it would be fine to make --reparent and --fixup conflict, since it's not at all clear to me what the intended result should be. (ie if you are reparenting the source commit, then you're replacing the destination commit with the source ... I think?; or would you be doing the fixup and then reparenting the original child commits of the target onto the newly squashed commit. (effectively reverting the squashed changes))

would like to test this more in real life and see if it's actually working

The tests seem to confirm the behavior, so I think this would be OK to merge (pending the review comments), but it would also be interesting to see how it works for you in real life.

bryango · 2025-12-01T07:07:19Z

Thank you so much for taking a look! I agree with all the review comment and will try to address them in the coming week (because of day job 😢

About the comments from the original issue #1537 (comment): yes indeed --insert --reparent feels a bit awkward and I'd like to see if it is actually useful in real life.

One thing that I like about --reparent is the guarantee that all contents (blobs?) associated with the descendants remain invariant before and after the move.

In the (not so practical) --insert --reparent example suggested there, starting from A <- B <- C and perform move --exact C --dest A --insert --reparent would actually lead to:

A <- C' (=B+C) <- B(without C) <- C(=B+C)

Note that the final C commit persists after the "move" (it's not actually moved haha) and has identical contents as before the move. This can be seen from the test here:

git-branchless/git-branchless/tests/test_move.rs

Lines 6409 to 6441 in 515faad

    
               // test --reparent with --insert 
        
               { 
        
                   let (stdout, _stderr) = git.branchless( 
        
                       "move", 
        
                       &[ 
        
                           "--source", 
        
                           &test3_oid.to_string(), 
        
                           "--dest", 
        
                           "master", 
        
                           "--reparent", 
        
                           "--insert", 
        
                       ], 
        
                   )?; 
        
                   insta::assert_snapshot!(stdout, @r" 
        
                   Attempting rebase in-memory... 
        
                   [1/4] Committed as: 3f0558d create test3_will_also_contain_test2_when_reparented.txt 
        
                   [2/4] Committed as: e1e0b99 create test2_will_also_contain_test1_when_reparented.txt 
        
                   [3/4] Committed as: 4b5cd3e create test3_will_also_contain_test2_when_reparented.txt 
        
                   [4/4] Committed as: fee6ba0 create test1.txt 
        
                   branchless: processing 4 rewritten commits 
        
                   branchless: running command: <git-executable> checkout e1e0b9952583334793f781ef25a6ce8d861cf85f 
        
                   O f777ecc (master) create initial.txt 
        
                   | 
        
                   o 3f0558d create test3_will_also_contain_test2_when_reparented.txt 
        
                   |\ 
        
                   | o fee6ba0 create test1.txt 
        
                   | 
        
                   @ e1e0b99 create test2_will_also_contain_test1_when_reparented.txt 
        
                   | 
        
                   o 4b5cd3e create test3_will_also_contain_test2_when_reparented.txt 
        
                   In-memory rebase succeeded. 
        
                   "); 
        
               }

The idea is that one can fearlessly manipulate the history without worrying about the final contents (C in this case) being changed. I think this is something powerful about --reparent. After rearranging histories some descendants may become empty and automatically drop out from the history, but during the whole process the final contents never changes, which is a nice property to have.
--insert --reparent above is indeed a bit of an edge case. I think the most powerful use case of --reparent is actually as an alternative for --merge (that's also why the option is placed next to --merge haha). Consider:

A <- B(diff: v1.0 -> v1.1) = trunk (immutable remote)
   \
    <- B'(diff: v1.0 -> v1.2) <- C = dev

move -b dev -d trunk would introduce conflicts that must be resolved manually, but
move -b dev -d trunk --reparent would lead to the graph:

A <- B(diff: v1.0 -> v1.1) <- B'(diff: v1.1 -> v1.2) <- C = dev

where B' has identical contents as before but becomes a "hard"-rebase on top of B, and the diff associated with B' now auto-magically becomes the intended v1.1 -> v1.2. So move --reparent can be a less confusing replacement for some "auto accept all incoming changes" version of git rebase / merge. This behavior can probably be emulated by some combinations of git reset / checkout -- with some confusing flags but I haven't been able to figure out how to do this safely and cleanly in vanilla git 🤣

bryango · 2025-12-03T08:35:01Z

I have implemented the consistent behavior for --reparent for all commands that accept MoveOptions. This includes move, restack, split, sync. Tests are added which verify the intended behavior. Note that for git-branchless test fix the behavior was already similar to --reparent, namely the flag is always implicitly assumed.

claytonrcarter

Thank you again @bryango for working on this.

I would like us to back out the changes to sync, split and restack for now. Let's move them into a different PR for now. I think that I would prefer to add --reparent only to move for now, and then see what feedback (if any) comes in the next release before extending it into more commands.

My concern is basically the same as I mentioned before: the behavior may be surprising, especially in the commands like sync and restack that are supposed to be more automatic and/or "hands off". And since both of those commands are just special cases of move, there is still a way for users to have this behavior if they wish.

As for split, I'm left wondering if a different name for the flag may be appropriate. For example, if --discard --reparent will remove the file from one commit but leave it in the child, it may be that --commute-down may be a more descriptive name. ?? 🤷

Then again, I tend to think of commits in terms of diffs, which isn't accurate. I'm open to compromise on this though. I don't want to dampen your enthusiasm!

claytonrcarter · 2026-01-09T21:38:42Z

+    /// Generate a sequence of rebase steps that cause the subtree at `source_oid`
+    /// to be "reparented" by `dest_oids`, namely, keeping all contents
+    /// of descendant commits exactly the same.
+    pub fn reparent_subtree(
+        &mut self,
+        source_oid: NonZeroOid,
+        parent_oids: Vec<NonZeroOid>,
+        repo: &Repo,
+    ) -> eyre::Result<()> {


[please clarify] Please correct me if I'm wrong, but I don't think that this comment nor the fn name match the actual behavior:

typo: I think that "by dest_oids" probably should be "onto parent_oids".

please confirm: does calling self.replace_commit() also trigger a rebase of the original commit's descendants onto the replacement commit? Or does it just do a replacement w/o rebase, leaving the descendants remaining on the original commit? ? The comment for replace_commit() doesn't mention anything about subtrees, which leads me to believe that this fn is also not actually triggering a rebase, but I don't know.

if replace_commit() does trigger a rebase, then we just need to address that first typo; the it doesn't rebase, then we should rewrite the comment and fn name to match

edit: I think I've confirmed that replace_commit does not also trigger a rebase of descendants, but I would love your 2nd opinion!

Yeah I believe you are correct (but it's a bit subtle, see below): I have renamed it to reparent_commit so that it is consistent with replace_commit and fixed the relevant wordings. During the rebase planning phase the descendants are not explicitly rebased. I have updated the function in a new commit f88edfd.

Note however that in the end the descendants of replaced commits are automatically rebased when executing the rebase plan, as documented in the original comments (and confirmed by the test cases). So in a sense the whole subtree is eventually rebased (but this is not explicit in the RebasePlan).

claytonrcarter · 2026-01-09T21:41:12Z

+        let descendant_commit = repo.find_commit_or_fail(source_oid)?;
+        let descendant_message = descendant_commit.get_message_raw();
+        let descendant_message = descendant_message.to_str().with_context(|| {
+            eyre::eyre!(
+                "Could not decode commit message for descendant commit: {:?}",
+                descendant_commit
+            )
+        })?;
+        let reparented_descendant_oid = repo.create_commit(
+            None,
+            &descendant_commit.get_author(),
+            &descendant_commit.get_committer(),
+            descendant_message,
+            &descendant_commit.get_tree()?,
+            parents.iter().collect(),
+        )?;
+        self.replace_commit(source_oid, reparented_descendant_oid)?;


[nitpick (subjective)] I think that these descendant_* names made sense in the original context, but probably don't make sense now. Changing them to source_* would line up better w/ the input params: eg descendant_commit → source_commit, reparented_descendant_oid → reparented_source_oid, etc

Good idea! Implemented in the new commit f88edfd.

claytonrcarter · 2026-01-09T22:11:02Z

+    // test --reparent with --insert
+    {
+        let (stdout, _stderr) = git.branchless(
+            "move",
+            &[
+                "--source",
+                &test3_oid.to_string(),
+                "--dest",
+                "master",
+                "--reparent",
+                "--insert",
+            ],
+        )?;
+        insta::assert_snapshot!(stdout, @r"
+        Attempting rebase in-memory...
+        [1/4] Committed as: 3f0558d create test3_will_also_contain_test2_when_reparented.txt
+        [2/4] Committed as: e1e0b99 create test2_will_also_contain_test1_when_reparented.txt
+        [3/4] Committed as: 4b5cd3e create test3_will_also_contain_test2_when_reparented.txt
+        [4/4] Committed as: fee6ba0 create test1.txt
+        branchless: processing 4 rewritten commits
+        branchless: running command: <git-executable> checkout e1e0b9952583334793f781ef25a6ce8d861cf85f
+        O f777ecc (master) create initial.txt
+        |
+        o 3f0558d create test3_will_also_contain_test2_when_reparented.txt
+        |\
+        | o fee6ba0 create test1.txt
+        |
+        @ e1e0b99 create test2_will_also_contain_test1_when_reparented.txt
+        |
+        o 4b5cd3e create test3_will_also_contain_test2_when_reparented.txt
+        In-memory rebase succeeded.
+        ");
+    }


[please change] This seems like a bug. If we move test3, I would expect it to be moved completely, not copied. I suspect that there is a conflict/oversight happening in the rebase planner that isn't indicating that the original test3 has been replaced w/ the new one.

Yes this indeed looks like a correctness bug that I would like to fix before the release, but I haven't figured out why it is happening (and it happens only when --reparent is combined with --insert).

I have figured it out: this is a bug of the test, as explained in #1631 (comment). The --source should not be test3_oid but the updated test3-branch. Fixed in a0c9459.

claytonrcarter · 2026-01-09T22:17:13Z

+    // test --reparent with --exact
+    {
+        let (stdout, _stderr) = git.branchless(
+            "move",
+            &["--exact", "e1e0b99", "--dest", "master", "--reparent"],
+        )?;
+        insta::assert_snapshot!(stdout, @r"
+        Attempting rebase in-memory...
+        [1/2] Skipped now-empty commit: 5cdb6f1 create test3_will_also_contain_test2_when_reparented.txt
+        [2/2] Committed as: 40ca381 create test2_will_also_contain_test1_when_reparented.txt
+        branchless: processing 2 rewritten commits
+        branchless: running command: <git-executable> checkout 3f0558d435e63ebdfd1e81f5dbd3ddfaca387864
+        O f777ecc (master) create initial.txt
+        |\
+        | o 40ca381 create test2_will_also_contain_test1_when_reparented.txt
+        |
+        @ 3f0558d create test3_will_also_contain_test2_when_reparented.txt
+        |
+        o fee6ba0 create test1.txt
+        In-memory rebase succeeded.
+        ");
+    }


[comment (no action needed)] When the "copied test3" bug is fixed, this test may go away, it seems.

[suggestion (non-blocking)] I think it would be easier to follow these tests if we used branch names for each of the commits. Then we could refer to (eg) test-2 instead of e1e0b99.

I think it would be easier to follow these tests if we used branch names for each of the commits. Then we could refer to (eg) test-2 instead of e1e0b99.

This is a very good suggestion that helped resolved the duplicated test3 bug! Implemented in a0c9459.

claytonrcarter · 2026-02-26T16:19:24Z

I would like us to back out the changes to sync, split and restack for now. Let's move them into a different PR for now. I think that I would prefer to add --reparent only to move for now, and then see what feedback (if any) comes in the next release before extending it into more commands.

@bryango Please disregard this ☝️ comment of mine. The flags are opt-in only, so there should be no risk of unwanted misuse, and I'm OK to merge this PR for all affected commands. Once it's released, we'll find out if my concerns about confusion and awkward flag names are founded or not! 😄

I think that the other comments still need to be addressed before merge, though.

I may try to make a release soon, do you think you'll have a chance to work on this again soon?

bryango · 2026-02-27T04:05:24Z

I would like us to back out the changes to sync, split and restack for now. Let's move them into a different PR for now. I think that I would prefer to add --reparent only to move for now, and then see what feedback (if any) comes in the next release before extending it into more commands.

@bryango Please disregard this ☝️ comment of mine. The flags are opt-in only, so there should be no risk of unwanted misuse, and I'm OK to merge this PR for all affected commands. Once it's released, we'll find out if my concerns about confusion and awkward flag names are founded or not! 😄

I think that the other comments still need to be addressed before merge, though.

I may try to make a release soon, do you think you'll have a chance to work on this again soon?

Thank you for the reviews! I will try to work on this again but a) I am slow, and b) I would like to fix all the correctness issues before it is out, so if this couldn't make into the release, I would try to target the following one. Thanks!

bryango · 2026-02-27T15:33:31Z

+            "move",
+            &[
+                "--source",
+                &test3_oid.to_string(),


The test3_oid here is incorrect; it is a hidden (moved) revision, not the up-to-date test3 commit. This is fixed in a0c9459 by attaching a test3-branch to the commit.

bryango · 2026-02-28T04:13:55Z

@claytonrcarter I have (hopefully) addressed all the issues in the new commits. In particular,

there is a merge from master to resolve conflicts (I avoided rebasing so that the commit history is more tractable, but feel free to rebase before merging), and
there is a final commit to fix some test snapshots that I missed.

... into dedicated function, to be reused in the future.

Move the `reparent` option from being specific to the `amend` command to a general option within `MoveOptions`. This allows its use across various move-related commands like `restack`, `split`, and `sync`.

Implement the new `reparent` option for the `move` command. This ensures moved subtrees are reparented to their new destination. The `fixup` option now conflicts with `reparent`.

This causes abandoned commits to be reparented to their new destination, mimicking the behavior of `amend --reparent`.

When splitting a commit with `--discard` or `--detach`, the `--reparent` option ensures that the changes from the discarded or detached portion are squashed to the child commit, just like `amend --reparent`. Reparenting is not applied to `InsertAfter` or `InsertBefore` modes as it would simply be a no-op in these cases (the descendants are not changed in the first place).

Allows `sync` to reparent commits onto the main branch, effectively "undoing" intermediate main branch commits that would otherwise be between the original parent and the new parent.

Clarify reparent_commit operates on a single source commit, while its descendants are rebased automatically. Also rename internal variables to be more descriptive.

Previously the --source [rev] supplied to the --reparent --insert test case was actually incorrect and caused a confusing test output: the old [rev] was a hidden (moved) test3_oid, not the updated one. The test result then appears to have two duplicated test3 commits. The correct [rev]s should instead be the updated commit, which are now tracked with branches named as `testN-branch`s. This makes the test correct and maintainable.

claytonrcarter · 2026-05-07T14:26:11Z

Sorry for the delay on this. I will try to get to it soon. In the meantime, I've rebased it onto current master.

bryango mentioned this pull request Nov 30, 2025

Support git branchless move --reparent #1537

Open

bryango force-pushed the feature branch from 13918a9 to 515faad Compare November 30, 2025 17:13

claytonrcarter requested changes Dec 1, 2025

View reviewed changes

Comment thread git-branchless-lib/src/core/rewrite/plan.rs Outdated

Comment thread git-branchless/tests/test_move.rs Outdated

bryango force-pushed the feature branch 2 times, most recently from b801d73 to d3fa892 Compare December 3, 2025 08:30

bryango requested a review from claytonrcarter December 3, 2025 08:35

claytonrcarter requested changes Jan 9, 2026

View reviewed changes

bryango commented Feb 27, 2026

View reviewed changes

bryango force-pushed the feature branch from cbebd00 to 0b0a5fc Compare February 28, 2026 03:37

bryango requested a review from claytonrcarter February 28, 2026 04:14

bryango added 12 commits May 7, 2026 10:21

refactor(reparent): extract amend --reparent logic

da36da3

... into dedicated function, to be reused in the future.

refactor(move): make --reparent option general

9608122

Move the `reparent` option from being specific to the `amend` command to a general option within `MoveOptions`. This allows its use across various move-related commands like `restack`, `split`, and `sync`.

feat(move): implement move --reparent

ec28288

Implement the new `reparent` option for the `move` command. This ensures moved subtrees are reparented to their new destination. The `fixup` option now conflicts with `reparent`.

feat(move): add test for move --reparent

a3a0167

refactor(plan): use eyre::bail! instead of assert!

988a5cf

feat(restack): implement --reparent

065e743

This causes abandoned commits to be reparented to their new destination, mimicking the behavior of `amend --reparent`.

feat(sync): add --reparent option

4bd046c

Allows `sync` to reparent commits onto the main branch, effectively "undoing" intermediate main branch commits that would otherwise be between the original parent and the new parent.

feat(fix): clarify --merge option ignored when fixing commits

288a182

refactor(plan.rs): reparent_subtree -> _commit

c2c2024

Clarify reparent_commit operates on a single source commit, while its descendants are rebased automatically. Also rename internal variables to be more descriptive.

fix(test_*_reparent): update insta snapshots

819062c

claytonrcarter force-pushed the feature branch from 0b0a5fc to 819062c Compare May 7, 2026 14:24

Uh oh!

Conversation

bryango commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claytonrcarter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bryango commented Dec 1, 2025

Uh oh!

bryango commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claytonrcarter left a comment

Choose a reason for hiding this comment

Uh oh!

claytonrcarter Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claytonrcarter commented Feb 26, 2026

Uh oh!

bryango commented Feb 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bryango commented Feb 28, 2026

Uh oh!

claytonrcarter commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bryango commented Nov 30, 2025 •

edited

Loading

bryango commented Dec 3, 2025 •

edited

Loading

claytonrcarter Jan 9, 2026 •

edited

Loading