Re: [PATCH v3 04/15] merge-tree: implement real merges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Feb 3, 2022 at 2:42 AM Johannes Altmanninger <aclopte@xxxxxxxxx> wrote:
>
> On Wed, Feb 02, 2022 at 04:18:39PM -0800, Elijah Newren wrote:
> > On Wed, Feb 2, 2022 at 2:01 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > >
> > > Elijah Newren <newren@xxxxxxxxx> writes:
> > >
> > > > 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux