Re: [PATCH v3 04/15] merge-tree: implement real merges
From: Elijah Newren <hidden>
Date: 2022-02-03 16:55:09
Possibly related (same subject, not in this thread)
- 2022-02-27 · Re: [PATCH v3 04/15] merge-tree: implement real merges · Johannes Altmanninger <hidden>
- 2022-02-27 · Re: [PATCH v3 04/15] merge-tree: implement real merges · Johannes Altmanninger <hidden>
- 2022-02-24 · Re: [PATCH v3 04/15] merge-tree: implement real merges · Junio C Hamano <hidden>
- 2022-02-24 · Re: [PATCH v3 04/15] merge-tree: implement real merges · Junio C Hamano <hidden>
- 2022-02-24 · Re: [PATCH v3 04/15] merge-tree: implement real merges · Elijah Newren <hidden>
Hi, On Thu, Feb 3, 2022 at 2:42 AM Johannes Altmanninger [off-list ref] wrote:
On Wed, Feb 02, 2022 at 04:18:39PM -0800, Elijah Newren wrote:quoted
On Wed, Feb 2, 2022 at 2:01 PM Junio C Hamano [off-list ref] wrote:quoted
Elijah Newren [off-list ref] writes:quoted
Yes, you are reading right. I think the cherry-pick/rebase replacement actually deserves a separate command from what merges should use; replaying a sequence of commits just has a number of UI differences and abilities that I think pull it in a different direction.I completely disagree. Each individual step in a sequence of replaying commits in order (or in reverse order) should be scriptable as a single merge-tree that takes "apply the change to go from A^ to A on X". Sequencing and placing UI around it is a job for the script that drives merge-tree.Adding such an ability to merge-tree would be trivial -- it basically involves just two things: (1) accepting one extra argument, and (2) calling merge_incore_nonrecursive() instead of merge_incore_recursive(). However, I think forking a subprocess for every merge of a series of commits is a completely unreasonable overhead, so even if we provide such an option to merge-tree, I still want a separate plumbing-ish tool that does non-worktree/non-index replaying of commits which is not written as a driver of merge-tree. That other tool should just call merge_incore_nonrecursive() directly. And such a tool, since it should handle an arbitrary number of commits, should certainly be able to handle just one commit. From that angle, it feels like adding another mode to merge-tree would just be a partial duplication of the other tool.I wonder how the UI of a tool that does non-worktree/non-index cherry-picks will look like. I'd expect it to produce the same output as merge-tree, except cherry-pick should probably output a commit OID, not a tree. Maybe we want a unified command that produces commits from any sequence of merge/cherry-pick/revert/reword steps. The obvious UI would use something like the rebase-todo list as input. For example: $ echo ' pick commit1 reword commit2 # edit commit message in $GIT_EDITOR merge commit3 -m "log message" ' | git create-commit commit0 <OID of final commit> we start from commit0 and apply steps one-by-one. Obviously, one unsolved problem is how to pass parameters like commit messages if no editor should be invoked (my sketch uses -m). If any of the steps fails when merging merge, then we get the tree with conflicts $ echo ' pick commit1 pick commit2 pick commit-that-does-not-apply ' | git create-commit commit0 <OID of commit after step 2> <OID of toplevel tree after failed merge> <Conflicted file info> <Informational messages> Replaying a series of commits might look like this: $ echo 'pick commit1 ^commit0' | git create-commit new-base I'm concluding that this is a difficult UI problem
I agree. I've got a lot of thoughts on it, and some work in progress towards it (https://github.com/newren/git/tree/replay -- _very_ hacky, not even close to alpha quality, lots of fixup commits, todo comments, random brain dump files added to the tree, based on a previous round of this patch series, not updated for weeks, etc., etc.)
and having a merge-tree command that accepts a "common ancestor" parameter could make it easier to experiment. Of course that depends on who is experimenting.
I think that would result in experiments and eventually full-blown scripts designed around forking subprocesses for every merge, and pushes us back into the world of having a scripted-rebase again. Yes, I know people can transliterate shell back to C; it seems to always be done as a half-way measure with the forking just being done from C or have other UI-warts guided by the shell design. In fact, *that* was the primary reason for me not providing a merge-tree option based on merge_incore_nonrecursive(), despite how trivial it'd be to provide it. If someone wanted a merge_incore_nonrecursive() mode for merge-tree for reasons other than attempting to build a rebase/cherry-pick replacement based on it, then I'd be much happier to provide it. If someone wants to experiment with what a plumbing-ish rebase/cherry-pick would look like, the _right_ way to do it would be making using of merge_incore_nonrecursive() directly. If they want example code, I already provided some a year and a half ago and got it merged into git.git in the form of t/helper/test-fast-rebase.c. My "replay" branch is based on that code, but (a) moves it from t/helper to a real builtin, (b) removes the hardcoded very strict input, (c) removes the line of code doing the index & working tree updates, and (d) modifies the output to be a more plumbing-ish style. We'll certainly have discussions on what that should look like. But a plumbing-ish replacement for merge was much simpler, and made sense to do first. I would prefer to concentrate on getting that hammered down first. Then I'll start discussions on a plumbing-ish rebase/cherry-pick. And if that doesn't fulfill all the needs that folks think they want out of merge-tree, then we can add a merge_incore_nonrecursive()-based mode to merge-tree. It's all coming, but having fought transliterations-of-scripts in merge-recursive.c, sequencer.c, stash.c, rebase.c, etc. for years I really, really don't want any more of that. Let's end that insanity.