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

[bryango]

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

1. refactor out the amend --reparent logic into a .reparent_subtree method for the RebasePlanBuilder
2. 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)
3. 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
4. add tests for move --reparent and showcase its intended behavior.
5. refactor: use eyre::bail! instead of assert!
6. implement restack --reparent
7. implement split --reparent (for --stack or --detach)
8. 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.

[claytonrcarter]

Let's not use an assert!() here, instead returning an Err if parent_oids.is_empty().

[bryango]

This is copied from .move_subtree, my bad. I have changed both occurrences to eyre:bail!.

[claytonrcarter]

[suggestion (maybe change)] Maybe consider using git.run(&["show", "--pretty=format:", "--stat", "HEAD"])?; (as is done in test_split). That makes the output much less verbose and may be easier to maintain.

[bryango]

Yes this looks much better! Implemented.

[bryango]

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]

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.