sageserpent-open / kineticMerge

Merge a heavily refactored codebase and stay sane.
MIT License
9 stars 1 forks source link

Support `--no-commit` and `--no-ff`. #14

Closed sageserpent-open closed 9 months ago

sageserpent-open commented 9 months ago

Folk who are used to git-merge expect to be able to hold a successful merge in a merged but uncomitted state, with the merge changes in the index; this allows the merge to be inspected, tested and further remedial work to done as part of a single merge commit. This is done with the --no-commit flag.

In the same vein, fast-forward merges can also be blocked to prevent a precious branch from being advanced trivially to the head of an untested branch: typically this is done using a combination of --no-ff and --no-commit to force the merge to have to generate an empty merge commit with two parents, but not actually do the commit, thus giving an opportunity to test the putative merge.

The job here is to support these flags in Kinetic Merge.

sageserpent-open commented 9 months ago

Main.mergeTheirBranch has been cutover to support these two flags, and the existing tests have been bolstered to cover them.

What is needed now is to parse the flags passed in to Main.main.

The scopt library looks like a candidate for this...

sageserpent-open commented 9 months ago

There is a subtle case here - a trivial merge attempted with both --no-ff and --no-commit. That should yield a no-changes commit with two parents when it is finally committed.

Actually, that depends; experimenting with this shows that Kinetic Merge will successfully merge their branch if it is ahead of ours, staging the incoming changes and setting MERGE_HEAD. This can be committed and yields a merge commit with two parents.

However, Kinetic Merge will also successfully merge their branch if it is behind ours; the index isn't updated (as we have all the changes already in our branch) and MERGE_HEAD is set. This can be committed, but it yields an empty commit with just one parent, namely the previous head commit of our branch.

This doesn't seem consistent, but experimenting with git merge --no-ff --no-commit also shows an asymmetry: if their branch is ahead, again the incoming changes are staged and MERGE_HEAD is set, leading to a nice merge commit later on with two parents. On the other hand, if their branch is behind, the command invocation reports that our branch is up to date with the other one and does nothing at all. The porcelain command is protecting the odd behaviour exposed by Kinetic Merge at the plumbing level!

So the change of plan is to emulate what git merge does in both cases...

sageserpent-open commented 9 months ago

This asymmetry is also present with just git merge --no-ff. If our branch is ahead of theirs, no commit will be made, so Kinetic Merge should emulate that too.

sageserpent-open commented 9 months ago

The tests exercise the code path for a trivial no-fast-forward and no-commit merge that actually brings in changes.

The confirm a new merge commit is produced.

Despite that, trying to do this for real using the command line always produces a bogus commit with just one parent!

Investigating....

sageserpent-open commented 9 months ago

It turns out that MERGE_HEAD is necessary, but not sufficient to allow git commit to finish off a merge with a two-parent merge commit.

Experimenting with git merge --no-ff --no-commit shows that the MERGE_MODE file is also required - decanting a copy of this file from a 'genuine' delayed Git merge into a delayed Kinetic Merge results in the desired merge commit being created.

sageserpent-open commented 9 months ago

In fact, it seems the role of MERGE_MODE is to propagate the --no-ff flag through to the delayed git commit invocation. I suspect the latter can optimise away a trivial merge using --no-commit so that the staged merge changes are recognised as redundant, so yes, the orginally requested commit is kept, but it is empty and simply grafted on to the our branch after it is fast-forwarded to their branch.

Let's extend the the trivial merge test to confirm this...

sageserpent-open commented 9 months ago

Done, evidence enclosed for the --no-ff plus --no-commit trivial merge. This is after having manually run git commit to conclude the merge. Note the merge commit itself does not contribute any changes, as it represents a fast-forward. Screenshot 2023-10-06 at 09 18 51