feat(move): implement move --reparent like amend --reparent #1631

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!

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

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.

git-branchless-lib/src/core/rewrite/plan.rs

git-branchless/tests/test_move.rs

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 🤣

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.

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

git-branchless-lib/src/core/rewrite/plan.rs

+    /// 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 "ontoparent_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!

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

git-branchless-lib/src/core/rewrite/plan.rs

+        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

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

git-branchless/tests/test_move.rs

+    // 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.

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

git-branchless/tests/test_move.rs

+    // 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.

claytonrcarter · 2026-01-09T22:18:03Z

git-branchless-lib/src/core/rewrite/plan.rs

+        if dest_oids.is_empty() {
+            eyre::bail!("Destination OIDs must not be empty for move_subtree");
+        }


Thank you for making this change even though it was preexisting! 😄

refactor(reparent): extract amend --reparent logic

b1e6810

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

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

git-branchless-lib/src/core/rewrite/plan.rs Outdated Show resolved Hide resolved

git-branchless/tests/test_move.rs Outdated Show resolved Hide resolved

bryango force-pushed the feature branch from 8d5232b to 31ff5d6 Compare December 2, 2025 03:54

bryango added 7 commits December 3, 2025 16:29

refactor(move): make --reparent option general

0230f10

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

432fd2e

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

5d06c5c

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

7b9bc96

feat(restack): implement --reparent

51dcbed

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

feat(sync): add --reparent option

d3fa892

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.

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

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

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

61ff4c0

claytonrcarter requested changes Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(move): implement move --reparent like amend --reparent #1631

feat(move): implement move --reparent like amend --reparent #1631

Uh oh!

bryango commented Nov 30, 2025 •

edited

Loading

Uh oh!

claytonrcarter left a comment

Uh oh!

Uh oh!

Uh oh!

bryango commented Dec 1, 2025

Uh oh!

bryango commented Dec 3, 2025 •

edited

Loading

Uh oh!

claytonrcarter left a comment

Uh oh!

claytonrcarter Jan 9, 2026

Uh oh!

claytonrcarter Jan 9, 2026

Uh oh!

claytonrcarter Jan 9, 2026

Uh oh!

claytonrcarter Jan 9, 2026

Uh oh!

claytonrcarter Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

feat(move): implement move --reparent like amend --reparent #1631

Are you sure you want to change the base?

feat(move): implement move --reparent like amend --reparent #1631

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

Choose a reason for hiding this comment

Uh oh!

claytonrcarter Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

claytonrcarter Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

claytonrcarter Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

claytonrcarter Jan 9, 2026

Choose a reason for hiding this comment

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