On-branch topic description support?

On-branch topic description support?

[Junio C Hamano]

I've been playing with this idea from time to time, but recently I
started seeing a related discussion on the b4 front, so I thought I
would throw it out and how people would think.

We made "git merge" not to silently commit the result, but open the
editor to encourage the integrator to describe what the topic is
about.  We also made "git format-patch" prepare [PATCH 0/n] aka
"cover letter" so that the author of the patch series can express
what the overall topic is about.  What the author should say in the
cover letter very much overlaps what the integrator wants to have in
the log message of the commit that merges the topic to the
integration branch.

But there are two (and half) links from format-patch to that merge
commit that are missing.  

 - You cannot prepare the cover letter material while working on the
   topic---instead, you have to write one by editing the output from
   "format-patch --cover-letter";

 - "git am" at the receiving end discards the cover letter when
   queuing the e-mailed patches to a topic.

 - "git merge" cannot take advantage of the cover letter that was
   discarded when the topic was queued.

So, here is how I would imagine a slightly better world may work.

 * When you are almost finished with the initial draft of your
   topic, you'd write a cover letter, and record it as the log
   message of an empty commit at the tip of the topic.  As you go on
   polishing the topic with "rebase -i", the empty commit would be
   kept (if we currently lose an empty commit by default, we may
   need to teach "rebase -i" to special case an empty commit at the
   tip of the range to make this convenient to use), and you would
   keep it up to date as you update the topic.

 * "git format-patch" would notice that the topic has such an empty
   commit at the tip, and use the log message from it to
   pre-populate the cover letter.

 * "git am" would learn an option to save the cover letter [0/n] and
   create such an empty commit at the tip of the branch.

 * "git merge" would learn an option to recognize that the branch
   being merged has such an empty commit at the tip, and instead
   merge the parent of the tip of the branch into the integration
   branch, while using the log message of the discarded tip commit
   in the log message of the merge itself.

Yes, there is "git config branch.mytopic.description" that helps
when pre-populating the cover letter, but that only helps at the
origin, and it is not shared between your personal repositories.
If you have the draft of the cover letter as part of the branch,
you can push/fetch them around just like all the "real" commits
you are working on.

Regardless of where the cover letter comes from, the changes to "am"
and "merge" dreamed above in this message would be useful, and that
is the primary reason why I am envisioning that right at the origin
having the topic description as an empty commit at the tip would be
the most convenient.  It would match the shape of the history at the
author side and at the side who runs "git am".

Re: On-branch topic description support?

[Shaoxuan Yuan]

On 7/21/2022 7:40 AM, Junio C Hamano wrote:
 > I've been playing with this idea from time to time, but recently I
 > started seeing a related discussion on the b4 front, so I thought I
 > would throw it out and how people would think.
 >
 > We made "git merge" not to silently commit the result, but open the
 > editor to encourage the integrator to describe what the topic is
 > about.  We also made "git format-patch" prepare [PATCH 0/n] aka
 > "cover letter" so that the author of the patch series can express
 > what the overall topic is about.  What the author should say in the
 > cover letter very much overlaps what the integrator wants to have in
 > the log message of the commit that merges the topic to the
 > integration branch.
 >
 > But there are two (and half) links from format-patch to that merge
 > commit that are missing.
 >
 >  - You cannot prepare the cover letter material while working on the
 >    topic---instead, you have to write one by editing the output from
 >    "format-patch --cover-letter";

This bothered me for a long time. Across every "rebase -i", I could see
a chance to modify the cover letter, but only right before sending the
patch I got the chance to compose a summary of what I've done in the
series, and that often makes me forget about things that should be in
the cover letter. And I have to keep a draft somewhere to record the loose
thoughts along the way.

 >  - "git am" at the receiving end discards the cover letter when
 >    queuing the e-mailed patches to a topic.
 >
 >  - "git merge" cannot take advantage of the cover letter that was
 >    discarded when the topic was queued.
 >
 > So, here is how I would imagine a slightly better world may work.
 >
 >  * When you are almost finished with the initial draft of your
 >    topic, you'd write a cover letter, and record it as the log
 >    message of an empty commit at the tip of the topic.  As you go on
 >    polishing the topic with "rebase -i", the empty commit would be
 >    kept (if we currently lose an empty commit by default, we may
 >    need to teach "rebase -i" to special case an empty commit at the
 >    tip of the range to make this convenient to use), and you would
 >    keep it up to date as you update the topic.
 >
 >  * "git format-patch" would notice that the topic has such an empty
 >    commit at the tip, and use the log message from it to
 >    pre-populate the cover letter.

It sounds pretty neat to have this on-the-go topic recorder :-)

--
Thanks,
Shaoxuan

Re: On-branch topic description support?

[Elijah Newren]

On Wed, Jul 20, 2022 at 5:08 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> I've been playing with this idea from time to time, but recently I
> started seeing a related discussion on the b4 front, so I thought I
> would throw it out and how people would think.
>
> We made "git merge" not to silently commit the result, but open the
> editor to encourage the integrator to describe what the topic is
> about.  We also made "git format-patch" prepare [PATCH 0/n] aka
> "cover letter" so that the author of the patch series can express
> what the overall topic is about.  What the author should say in the
> cover letter very much overlaps what the integrator wants to have in
> the log message of the commit that merges the topic to the
> integration branch.
>
> But there are two (and half) links from format-patch to that merge
> commit that are missing.
>
>  - You cannot prepare the cover letter material while working on the
>    topic---instead, you have to write one by editing the output from
>    "format-patch --cover-letter";
>
>  - "git am" at the receiving end discards the cover letter when
>    queuing the e-mailed patches to a topic.
>
>  - "git merge" cannot take advantage of the cover letter that was
>    discarded when the topic was queued.
>
> So, here is how I would imagine a slightly better world may work.
>
>  * When you are almost finished with the initial draft of your
>    topic, you'd write a cover letter, and record it as the log
>    message of an empty commit at the tip of the topic.  As you go on
>    polishing the topic with "rebase -i", the empty commit would be
>    kept (if we currently lose an empty commit by default, we may
>    need to teach "rebase -i" to special case an empty commit at the
>    tip of the range to make this convenient to use), and you would
>    keep it up to date as you update the topic.

No need for rebase changes here.  By default, it preserves commits
which start empty, and only drops commits which *become* empty.

(If one wants to drop commits that start empty, you can pass
`--no-keep-empty`.  If you want to keep commits that become empty, you
can use either `--empty=keep` or `--empty=ask`.)

>  * "git format-patch" would notice that the topic has such an empty
>    commit at the tip, and use the log message from it to
>    pre-populate the cover letter.
>
>  * "git am" would learn an option to save the cover letter [0/n] and
>    create such an empty commit at the tip of the branch.
>
>  * "git merge" would learn an option to recognize that the branch
>    being merged has such an empty commit at the tip, and instead
>    merge the parent of the tip of the branch into the integration
>    branch, while using the log message of the discarded tip commit
>    in the log message of the merge itself.
>
> Yes, there is "git config branch.mytopic.description" that helps
> when pre-populating the cover letter, but that only helps at the
> origin, and it is not shared between your personal repositories.
> If you have the draft of the cover letter as part of the branch,
> you can push/fetch them around just like all the "real" commits
> you are working on.
>
> Regardless of where the cover letter comes from, the changes to "am"
> and "merge" dreamed above in this message would be useful, and that
> is the primary reason why I am envisioning that right at the origin
> having the topic description as an empty commit at the tip would be
> the most convenient.  It would match the shape of the history at the
> author side and at the side who runs "git am".

Seems mostly reasonable, but I'm curious about one thing:

The cover letter material often includes stuff that would not make
sense for the merge message (e.g. "Rebased this version on master
because of conflicts with ...", "Changes since v3", "I'm a little
unsure of the logic in Patch 5", "Patch 3 might be controversial; take
a look", etc.)  Would there be some kind of syntax for specifying the
part of the cover letter meant to be used in the merge commit message,
or would we just start out with it pre-populating the commit message
and expect the integrator to cull out the irrelevant parts?

Re: On-branch topic description support?

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> The cover letter material often includes stuff that would not make
> sense for the merge message (e.g. "Rebased this version on master
> because of conflicts with ...", "Changes since v3", "I'm a little
> unsure of the logic in Patch 5", "Patch 3 might be controversial; take
> a look", etc.)  Would there be some kind of syntax for specifying the
> part of the cover letter meant to be used in the merge commit message,
> or would we just start out with it pre-populating the commit message
> and expect the integrator to cull out the irrelevant parts?

Yeah, I wanted to flesh out a bit more details on how this may work
before writing more about it, but what I envisioned would go
something along this line:

 * The authoring side would keep track of the description of the
   topic as a whole plus the description of inter-iteration changes,
   in the empty tip commit.  There may be some convention with a
   minimum mark-up (similar to how an innocuous three-dash line
   serves the "end of the message" marker in a patch e-mail) to
   separate the latter from the former.

 * "format-patch --cover-letter" would take the above, and add
   range-diff etc. that is easier to compute mechanically;

 * "am" may take the above, and keep the whole thing;

 * "merge" would keep the "main topic description" part and strip
   out everything after it, including the inter-iteration comparison
   and the range-diff, most likely by commenting the latter out when
   presenting it to the integrator.  So a lazy integrator can simply
   exit the editor to lose everything other than the topic
   description, and a more careful ones may refer to the supporting
   material to enhance/extend the topic description.

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Wed, Jul 20, 2022 at 10:25:19PM -0700, Elijah Newren wrote:
> Seems mostly reasonable, but I'm curious about one thing:
> 
> The cover letter material often includes stuff that would not make
> sense for the merge message (e.g. "Rebased this version on master
> because of conflicts with ...", "Changes since v3", "I'm a little
> unsure of the logic in Patch 5", "Patch 3 might be controversial; take
> a look", etc.)  Would there be some kind of syntax for specifying the
> part of the cover letter meant to be used in the merge commit message,
> or would we just start out with it pre-populating the commit message
> and expect the integrator to cull out the irrelevant parts?

Maybe reuse the scissors line, but throw away stuff after the scissors? E.g.:

    Subject: [PATCH v2 0/3] Implement foo

    This implements foo that is needed to...
    ...
    ...
    ...

    Signed-off-by: De Veloper <dev@example.com>
    Link: https://example.com/foo

    -- >8 --

    Note: I'm not sure about....

    ---

    Changes in v2:
     - Rewrote using libfoo
     - ...

    ---
    Diffstat, range-diff, etc.

    ---
    base-commit: abcde
    change-id: abcde-foobarbaz

-K

Re: On-branch topic description support?

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 20 2022, Junio C Hamano wrote:

>  * When you are almost finished with the initial draft of your
>    topic, you'd write a cover letter, and record it as the log
>    message of an empty commit at the tip of the topic.

Isn't thu DAG topology of creating a merge that encompases the given
range a more clear marker for this sort of thing?

	# Create a topic branch
	git checkout -b topic -t origin/master
	for f in A B C; do echo $f >$f && git add $f && git commit -m$f; done

	# I forget if there a way to make the porcelain do this, not this or --ff...
	git merge @{u}
        # ... that's better!
	git reset --hard $(echo My CL here | git commit-tree -p @{u} -p HEAD HEAD:)

To steal and amend a diagram from git-merge(1), we now have turned this:


              A---B---C topic
             / 
        X---Y
             \
              master

Into this:

              A---B---C---M topic
             / \         /
        X---Y   ---------
             \
              master

I vaguely recall having actually used this for something in the past.

I tried now with "git rebase --rebase-merges -i", and it supports it
properly, i.e. I could re-arrange it so that it's:

              A---C---B---M topic
             / \         /
        X---Y   ---------
             \
              master

And it still gave me a merge commit whose parents were my branch tip,
and my @{upstream}.

The advantage of doing it this way is that it's obvious from the
topology what the lower bound of this "summary so far merge" is, with an
empty commit on top it:

 A) isn't distinguishable from other uses of empty commits, but this
     merge structure is rather unusual, so it sets it apart.

 B) You don't know what the lower bound is if your @{upstream} ref
    moves.

    I could see it being useful to e.g. offer to update the CL if
    there's conflicts, or if upstream has integrated part of my commits.

Re: On-branch topic description support?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>  * "git merge" would learn an option to recognize that the branch
>    being merged has such an empty commit at the tip, and instead
>    merge the parent of the tip of the branch into the integration
>    branch, while using the log message of the discarded tip commit
>    in the log message of the merge itself.

An issue that needs to be worked out at the Porcelain level is that
this layout will make "git branch --[no-]merged master", which is an
effective way to list what's in and what's left out of the 'master'
integration branch, more or less useless.  A topic branch with the
cover letter commit at the tip may want to be merged to 'next' and
then after cooking for a while merged down to 'master', and each
time it gets merged, the merge has to leave the cover letter commit
alone.  In spirit, such a merge commit merges the topic "fully", but
at the topological level, the topic is not (and will never be)
merged fully, and "git branch --merged next" will not show.

I guess once the topic hits the oldest integration track that it
meant to go, we could "pop" the cover letter commit out of the
topic, and at that point it would appear to have been merged to
'master' as well as to 'next'.  But it would be nice if we did not
have to discard the cover letter commit only to please "git branch".
IOW, it would be helpful to teach "git branch --[no-]merged" to
compute the "right thing" in such a layout.

RE: On-branch topic description support?

[rsbecker]

On July 21, 2022 11:06 AM, Junio C Hamano wrote:
>Junio C Hamano <gitster@pobox.com> writes:
>
>>  * "git merge" would learn an option to recognize that the branch
>>    being merged has such an empty commit at the tip, and instead
>>    merge the parent of the tip of the branch into the integration
>>    branch, while using the log message of the discarded tip commit
>>    in the log message of the merge itself.
>
>An issue that needs to be worked out at the Porcelain level is that this
layout will
>make "git branch --[no-]merged master", which is an effective way to list
what's in
>and what's left out of the 'master'
>integration branch, more or less useless.  A topic branch with the cover
letter
>commit at the tip may want to be merged to 'next' and then after cooking
for a
>while merged down to 'master', and each time it gets merged, the merge has
to
>leave the cover letter commit alone.  In spirit, such a merge commit merges
the
>topic "fully", but at the topological level, the topic is not (and will
never be)
>merged fully, and "git branch --merged next" will not show.
>
>I guess once the topic hits the oldest integration track that it meant to
go, we
>could "pop" the cover letter commit out of the topic, and at that point it
would
>appear to have been merged to 'master' as well as to 'next'.  But it would
be nice if
>we did not have to discard the cover letter commit only to please "git
branch".
>IOW, it would be helpful to teach "git branch --[no-]merged" to compute the
>"right thing" in such a layout.

I might be missing something here, but could not a description follow
similar semantics to how git describe works on tags? Anchor the description
on the commit where the branch was spawned, then determine the appropriate
description based on things like --first-parent. Again, I'm probably in the
weeds.
--Randall

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Thu, Jul 21, 2022 at 11:29:20AM -0400, rsbecker@nexbridge.com wrote:
> I might be missing something here, but could not a description follow
> similar semantics to how git describe works on tags? Anchor the description
> on the commit where the branch was spawned, then determine the appropriate
> description based on things like --first-parent. Again, I'm probably in the
> weeds.

I think the trouble is that "commit where the branch was spawned" is not
reliable due to:

- rebasing the work on a different branch
- reordering commits (including the first commit in the series)

This is the main reason why I actually want to stick the empty commit with the
cover letter not at the tip of the branch, but at the start of the series, so
as to clearly demarcate where the series of commits we're interested in
starts (at least while the author is working on them -- when it sent off
upstream that commit can get removed/moved).

-K

RE: On-branch topic description support?

[rsbecker]

On July 21, 2022 11:40 AM, Konstantin Ryabitsev wrote:
>On Thu, Jul 21, 2022 at 11:29:20AM -0400, rsbecker@nexbridge.com wrote:
>> I might be missing something here, but could not a description follow
>> similar semantics to how git describe works on tags? Anchor the
>> description on the commit where the branch was spawned, then determine
>> the appropriate description based on things like --first-parent.
>> Again, I'm probably in the weeds.
>
>I think the trouble is that "commit where the branch was spawned" is not reliable
>due to:
>
>- rebasing the work on a different branch
>- reordering commits (including the first commit in the series)
>
>This is the main reason why I actually want to stick the empty commit with the
>cover letter not at the tip of the branch, but at the start of the series, so as to
>clearly demarcate where the series of commits we're interested in starts (at least
>while the author is working on them -- when it sent off upstream that commit can
>get removed/moved).

That process may be more suitable outside the git team where merge --squash is commonplace and the whole series goes into next, main, maint as a single commit as the empty commit will simply vanish. Although, with that said, for the git team's committer process, having a starting point commit in the history also has some obvious advantages, for example, starting and ending points in history for git bisect when things go terribly wrong.
--R

Re: On-branch topic description support?

[Philip Oakley]

On 21/07/2022 06:25, Elijah Newren wrote:
> On Wed, Jul 20, 2022 at 5:08 PM Junio C Hamano <gitster@pobox.com> wrote:
>> I've been playing with this idea from time to time, but recently I
>> started seeing a related discussion on the b4 front, so I thought I
>> would throw it out and how people would think.
>>
>> We made "git merge" not to silently commit the result, but open the
>> editor to encourage the integrator to describe what the topic is
>> about.  We also made "git format-patch" prepare [PATCH 0/n] aka
>> "cover letter" so that the author of the patch series can express
>> what the overall topic is about.  What the author should say in the
>> cover letter very much overlaps what the integrator wants to have in
>> the log message of the commit that merges the topic to the
>> integration branch.
>>
>> But there are two (and half) links from format-patch to that merge
>> commit that are missing.
>>
>>  - You cannot prepare the cover letter material while working on the
>>    topic---instead, you have to write one by editing the output from
>>    "format-patch --cover-letter";
>>
>>  - "git am" at the receiving end discards the cover letter when
>>    queuing the e-mailed patches to a topic.
>>
>>  - "git merge" cannot take advantage of the cover letter that was
>>    discarded when the topic was queued.
>>
>> So, here is how I would imagine a slightly better world may work.
>>
>>  * When you are almost finished with the initial draft of your
>>    topic, you'd write a cover letter, and record it as the log
>>    message of an empty commit at the tip of the topic.  

If one has a plan in place for a series in preparation, I'd want to
prepare the outline of the cover letter first, and then later let it
float to the tip of the series after a rebase, and incorporate (meld)
additional notelets recorded as additional empty commits.

On thought is to marry this with the `rebase --autosquash` mechanism
that will rearrange commits (when enabled) so that we could have a
"branchtip! " subject line, similar to the `fixup! / squash!` subject
lines to assist in that rearrangement  of the series for transmission.

I'm not yet sure how the "branchtip" method would work with Pull
Requests and the idea of actually having a push/pull exchangeable branch
description - there's been a very recent discussion on the Git-user list
[1] which started from a slightly different premise, but maybe has some
of the same issues as persistent Change-ID's [2].
>> As you go on
>>    polishing the topic with "rebase -i", the empty commit would be
>>    kept (if we currently lose an empty commit by default, we may
>>    need to teach "rebase -i" to special case an empty commit at the
>>    tip of the range to make this convenient to use), and you would
>>    keep it up to date as you update the topic.
> No need for rebase changes here.  By default, it preserves commits
> which start empty, and only drops commits which *become* empty.
>
> (If one wants to drop commits that start empty, you can pass
> `--no-keep-empty`.  If you want to keep commits that become empty, you
> can use either `--empty=keep` or `--empty=ask`.)
>
>>  * "git format-patch" would notice that the topic has such an empty
>>    commit at the tip, and use the log message from it to
>>    pre-populate the cover letter.
>>
>>  * "git am" would learn an option to save the cover letter [0/n] and
>>    create such an empty commit at the tip of the branch.
>>
>>  * "git merge" would learn an option to recognize that the branch
>>    being merged has such an empty commit at the tip, and instead
>>    merge the parent of the tip of the branch into the integration
>>    branch, while using the log message of the discarded tip commit
>>    in the log message of the merge itself.
>>
>> Yes, there is "git config branch.mytopic.description" that helps
>> when pre-populating the cover letter, but that only helps at the
>> origin, and it is not shared between your personal repositories.
>> If you have the draft of the cover letter as part of the branch,
>> you can push/fetch them around just like all the "real" commits
>> you are working on.
>>
>> Regardless of where the cover letter comes from, the changes to "am"
>> and "merge" dreamed above in this message would be useful, and that
>> is the primary reason why I am envisioning that right at the origin
>> having the topic description as an empty commit at the tip would be
>> the most convenient.  It would match the shape of the history at the
>> author side and at the side who runs "git am".
> Seems mostly reasonable, but I'm curious about one thing:
>
> The cover letter material often includes stuff that would not make
> sense for the merge message (e.g. "Rebased this version on master
> because of conflicts with ...", "Changes since v3", "I'm a little
> unsure of the logic in Patch 5", "Patch 3 might be controversial; take
> a look", etc.)  Would there be some kind of syntax for specifying the
> part of the cover letter meant to be used in the merge commit message,
> or would we just start out with it pre-populating the commit message
> and expect the integrator to cull out the irrelevant parts?
--
Philip

[1] https://groups.google.com/g/git-users/c/5RftRV1c5Zw/m/cpem3KL1AAAJ
Dan Rosen 18/07/2022
[2]
https://lore.kernel.org/git/bdbe9b7c1123f70c0b4325d778af1df8fea2bb1b.camel@that.guru/

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Thu, Jul 21, 2022 at 04:53:18PM +0200, Ævar Arnfjörð Bjarmason wrote:
> I tried now with "git rebase --rebase-merges -i", and it supports it
> properly, i.e. I could re-arrange it so that it's:
> 
>               A---C---B---M topic
>              / \         /
>         X---Y   ---------
>              \
>               master

This is clever, but it has a hard restriction that nothing happens to A:

- you can't move it so it's B---A---C---M
- you can't squash A+B

Unless all the tools are taught to properly modify the merge commit.

Or am I not reading this right?

-K

Re: On-branch topic description support?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> To steal and amend a diagram from git-merge(1), we now have turned this:
>
>
>               A---B---C topic
>              / 
>         X---Y
>              \
>               master
>
> Into this:
>
>               A---B---C---M topic
>              / \         /
>         X---Y   ---------
>              \
>               master

If you mean your topic have 3 commits, then I think you want the
first parent of M to be Y, not A, but I agree the above arrangement
can also represent what I was discussing.

The thing is, it is unclear if such an artificial merge is an easy
thing to maintain by individual contributors.  "rebase -i" with some
extra options people are not familiar with may be able to reproduce
the topology without losing information after you accumulate more
commits on top of M (e.g. you build D and E on top of M and now you
want to make it A-B-C-D-E merged into Y with M that records the tree
of E; you'd amend M's message to talk also about D and E), but
"it is possible" is different from "people are comfortable doing".

If M did not have a backpointer to the other parent, and instead
were an empty commit, then you recreated what I wrote in the message
you are responding to.

Re: On-branch topic description support?

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

> If one has a plan in place for a series in preparation, I'd want to
> prepare the outline of the cover letter first, and then later let it
> float to the tip of the series after a rebase, and incorporate (meld)
> additional notelets recorded as additional empty commits.
>
> On thought is to marry this with the `rebase --autosquash` mechanism
> that will rearrange commits (when enabled) so that we could have a
> "branchtip! " subject line, similar to the `fixup! / squash!` subject
> lines to assist in that rearrangement  of the series for transmission.

Yeah, people have different preferences, and "rebase -i" is probably
a key ingredient to make it work.  As long as the convention to have
topic description at a fixed known place on the topic branch, be it
at the top or at the bottom, is followed in the end result, other
tools like format-patch, am and merge can be written to follow and
take advantage that known structure/topology of the topic branch.

> I'm not yet sure how the "branchtip" method would work with Pull
> Requests and the idea of actually having a push/pull exchangeable branch
> description

I think what I described for "merge" would work well for pull
requests (not the traditional e-mail kind of "please pull" we see on
the kernel mailing list, but the GitHub kind).  Merge the topic
proper, excluding the tip "empty" commit that is there only to hold
the description of the topic, into the history, and use the log
message from the empty commit the procedure discards as the log
message for the resulting merge.

Re: On-branch topic description support?

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Thu, Jul 21, 2022 at 04:53:18PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> I tried now with "git rebase --rebase-merges -i", and it supports it
>> properly, i.e. I could re-arrange it so that it's:
>> 
>>               A---C---B---M topic
>>              / \         /
>>         X---Y   ---------
>>              \
>>               master
>
> This is clever, but it has a hard restriction that nothing happens to A:
>
> - you can't move it so it's B---A---C---M
> - you can't squash A+B
>
> Unless all the tools are taught to properly modify the merge commit.
>
> Or am I not reading this right?

I think the drawing is about a two-commit topic that has B and C

Or the drawing is wrong and M's other parent should have been Y.
IOW, "git merge-base M^1 M^2" is the bottom of the topic (and our
ranges always exclude the bottom end, X..Y does not include X but
does include Y).  "git rev-list M^1...M^2" are the commits on the
topic.

Then you can shuffle A--B--C in whatever way you like with "rebase
-i" or "filter-branch", using "git merge-base M^1 M^2" as the base.

Re: On-branch topic description support?

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 21 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> To steal and amend a diagram from git-merge(1), we now have turned this:
>>
>>
>>               A---B---C topic
>>              / 
>>         X---Y
>>              \
>>               master
>>
>> Into this:
>>
>>               A---B---C---M topic
>>              / \         /
>>         X---Y   ---------
>>              \
>>               master
>
> If you mean your topic have 3 commits, then I think you want the
> first parent of M to be Y, not A, but I agree the above arrangement
> can also represent what I was discussing.

Yes, sorry. The diagram is incorrect, but not the commands I posted,
i.e. one of the parents is "master".

> The thing is, it is unclear if such an artificial merge is an easy
> thing to maintain by individual contributors.  "rebase -i" with some
> extra options people are not familiar with may be able to reproduce
> the topology without losing information after you accumulate more
> commits on top of M (e.g. you build D and E on top of M and now you
> want to make it A-B-C-D-E merged into Y with M that records the tree
> of E; you'd amend M's message to talk also about D and E), but
> "it is possible" is different from "people are comfortable doing".

Yes, it's magical, but so is an empty commit having a new "CL"
meaning. I'd think both would require relevant tooling to be aware of
them.

But this is worse in that "git rebase" will get rid of it by default.

> If M did not have a backpointer to the other parent, and instead
> were an empty commit, then you recreated what I wrote in the message
> you are responding to.

Yes, that's the difference between the two.

Re: On-branch topic description support?

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 21 2022, Junio C Hamano wrote:

> Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
>
>> On Thu, Jul 21, 2022 at 04:53:18PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>> I tried now with "git rebase --rebase-merges -i", and it supports it
>>> properly, i.e. I could re-arrange it so that it's:
>>> 
>>>               A---C---B---M topic
>>>              / \         /
>>>         X---Y   ---------
>>>              \
>>>               master
>>
>> This is clever, but it has a hard restriction that nothing happens to A:
>>
>> - you can't move it so it's B---A---C---M
>> - you can't squash A+B
>>
>> Unless all the tools are taught to properly modify the merge commit.
>>
>> Or am I not reading this right?
>
> I think the drawing is about a two-commit topic that has B and C
>
> Or the drawing is wrong and M's other parent should have been Y.
> IOW, "git merge-base M^1 M^2" is the bottom of the topic (and our
> ranges always exclude the bottom end, X..Y does not include X but
> does include Y).  "git rev-list M^1...M^2" are the commits on the
> topic.
>
> Then you can shuffle A--B--C in whatever way you like with "rebase
> -i" or "filter-branch", using "git merge-base M^1 M^2" as the base.

Yes, those last two should be:

              A---B---C---M topic
             /           /
        X---Y------------
             \
              master

And:

              A---C---B---M topic
             /          /
        X---Y-----------
             \
              master

I.e. you could re-arrange it so that it's:

              C---B---A---M topic
             /          /
        X---Y-----------
             \
              master

Or whatever, sorry about that mix-up.

Re: On-branch topic description support?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> But this is worse in that "git rebase" will get rid of it by default.

FWIW, I think I like this much better than Konstantin's "there is an
empty commit at the bottom", for exactly the same reason why I like
the original "empty commit at the tip", i.e. simply because we can
strip away the "extra" commit that holds the topic description
without having to change all the "real" commits.

In fact, I thought one of the newer "b4" subcommands that is used to
accept a patch series with a cover letter creates exactly this sort
of topology, when told to apply the topic to the tip of the
integration branch?  I do prefer to see a tool that it can use the
same "format" of data for both input and output, and I think your
"there is a redundant merge commit at the tip, with topic
description" topology can be that input format, which would be
recreated at the receiving end if the receiver happens to have the
same base when applying it and the final integration branch was also
at the same base.

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Thu, Jul 21, 2022 at 11:02:26AM -0700, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > But this is worse in that "git rebase" will get rid of it by default.
> 
> FWIW, I think I like this much better than Konstantin's "there is an
> empty commit at the bottom", for exactly the same reason why I like
> the original "empty commit at the tip", i.e. simply because we can
> strip away the "extra" commit that holds the topic description
> without having to change all the "real" commits.

I'm happy to consider alternatives if I can have a reliable way of tracking
"the series we're working on starts at this commit". I know that this is
antithetical to git's design, but I also can't think of anything else that
reliably survives rebases.

> In fact, I thought one of the newer "b4" subcommands that is used to
> accept a patch series with a cover letter creates exactly this sort
> of topology, when told to apply the topic to the tip of the
> integration branch? 

Yes, you're thinking "b4 shazam -M", which does a "merge --no-ff" with the
cover letter as the template merge commit. I guess it does create this exact
structure:

             C---B---A---M
             /          /
        X---Y-----------

I'll play with tip merge commits and see how well my "survives rebases" tests
go.

-K

Re: On-branch topic description support?

[Ævar Arnfjörð Bjarmason]

On Thu, Jul 21 2022, Konstantin Ryabitsev wrote:

> On Thu, Jul 21, 2022 at 11:02:26AM -0700, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> 
>> > But this is worse in that "git rebase" will get rid of it by default.
>> 
>> FWIW, I think I like this much better than Konstantin's "there is an
>> empty commit at the bottom", for exactly the same reason why I like
>> the original "empty commit at the tip", i.e. simply because we can
>> strip away the "extra" commit that holds the topic description
>> without having to change all the "real" commits.
>
> I'm happy to consider alternatives if I can have a reliable way of tracking
> "the series we're working on starts at this commit". I know that this is
> antithetical to git's design, but I also can't think of anything else that
> reliably survives rebases.

Aside from any discussion of how such a "cover letter" thingy gets
represented in the DAG, I wonder if rebase should be less aggressive
about wiping away merge commits in general.

It's really handy to e.g. fix something where you've got repeated merges
of the "master" into your branch.

But if you've got some novel merge structure that's existing purely
within your branch perhaps we should at least show some advice() before
proceeding, or suggest --rebase-merges...

I haven't thought through all the edge cases with that, just food for
thought.

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Thu, Jul 21, 2022 at 02:26:45PM -0400, Konstantin Ryabitsev wrote:
> I'll play with tip merge commits and see how well my "survives rebases" tests
> go.

Hmm... well, it works great as long as you remember to always use
--rebase-merges. The moment you forget to pass -r to "git rebase", your cover
letter commit disappears, and I'm not sure this is going to work for what I am
trying to do (which is to make the process of submitting patch series
simpler). I know it's possible to configure git so that "git pull --rebase"
preserves merges, but there doesn't appear to be a way to force "git rebase
-i" to do the same without the -r flag. Also, "rebase -ir" looks really
different when there is a tip merge commit, which will probably also be
confusing to newbies just starting out with rebase workflows.

So, I'm not sure that at this time this is objectively "better" than keeping
the cover letter in an empty commit at the start of the series. :-/

-K

Re: On-branch topic description support?

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> Hmm... well, it works great as long as you remember to always use
> --rebase-merges. The moment you forget to pass -r to "git rebase", your cover
> letter commit disappears, and I'm not sure this is going to work for what I am
> trying to do (which is to make the process of submitting patch series
> simpler). I know it's possible to configure git so that "git pull --rebase"
> preserves merges, but there doesn't appear to be a way to force "git rebase
> -i" to do the same without the -r flag. Also, "rebase -ir" looks really
> different when there is a tip merge commit, which will probably also be
> confusing to newbies just starting out with rebase workflows.
>
> So, I'm not sure that at this time this is objectively "better" than keeping
> the cover letter in an empty commit at the start of the series. :-/

Now you are discussing this on the git mailing list, you do not
necessarily have to take the existing behaviour of Git as given.  

For example, I do not think it is unreasonable to teach "git rebase
[-i]" to special case a certain merge commit in the rebased range
without any extra option, as long as the criteria to pick such a
special "merge" is specific and narrow enough (a two-parent merge M
whose tree matches that of one parent's tree (say M^1^{tree}), the
other parent (say M^2) is an immediate ancestor of the bottommost
commit of the range being rebased, or something like that).  And the
way you "special case" it does not have to be tied to the way the
"-r" option handles it, either.  

A possible design could go like this:

 * we recognize such a special merge commit;

 * we rebase the rest of the range, pretending as if that merge
   commit did not exist and instead its children are all direct
   child of one of its parent (say M^1), using the options given (so
   "-r" would affect how other merges in the rebased range is
   handled).

 * after everything is done, we create a new empty merge commit at
   the top, merging the bottom of the range and the tip of the range
   as its parents, using the log message from original M.  This can
   be done totally outside of the regular "rebase" machinery.

Such a change to existing behaviour is well within the scope of
"On-branch topic description support", I would think.

Re: On-branch topic description support?

[Konstantin Ryabitsev]

On Thu, Jul 21, 2022 at 01:01:16PM -0700, Junio C Hamano wrote:
> Now you are discussing this on the git mailing list, you do not
> necessarily have to take the existing behaviour of Git as given.  

Indeed, I was mostly observing about how it works right now -- please don't
take this as complaining. :)

> For example, I do not think it is unreasonable to teach "git rebase
> [-i]" to special case a certain merge commit in the rebased range
> without any extra option, as long as the criteria to pick such a
> special "merge" is specific and narrow enough (a two-parent merge M
> whose tree matches that of one parent's tree (say M^1^{tree}), the
> other parent (say M^2) is an immediate ancestor of the bottommost
> commit of the range being rebased, or something like that).  And the
> way you "special case" it does not have to be tied to the way the
> "-r" option handles it, either.  
> 
> A possible design could go like this:
> 
>  * we recognize such a special merge commit;
> 
>  * we rebase the rest of the range, pretending as if that merge
>    commit did not exist and instead its children are all direct
>    child of one of its parent (say M^1), using the options given (so
>    "-r" would affect how other merges in the rebased range is
>    handled).
> 
>  * after everything is done, we create a new empty merge commit at
>    the top, merging the bottom of the range and the tip of the range
>    as its parents, using the log message from original M.  This can
>    be done totally outside of the regular "rebase" machinery.
> 
> Such a change to existing behaviour is well within the scope of
> "On-branch topic description support", I would think.

I agree, this would be great. If this is in place, then it would certainly
bring us closer to managing "proposed change" series completely in-git.

One thought that comes to mind -- perhaps it would be easier not to track the
special commit, but designate the branch as a special "proposed changes"
branch that could even "hide" the cover-letter merge commit (CLM) from most
operations. It's still a simple two-parent merge commit, so it can be
pushed/pulled with any existing remotes without requiring any changes on the
server side, but operations like adding a regular new commit to the tip of
the "proposed changes" branch would automatically move the merge commit so
it's back at the tip. This way "not sure what you're trying to do" situations
like this would be avoided:

  A--B--C--CLM--D--E
 /          /
Y-----------

-K

Re: On-branch topic description support?

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> One thought that comes to mind -- perhaps it would be easier not to track the
> special commit, but designate the branch as a special "proposed changes"
> branch that could even "hide" the cover-letter merge commit (CLM) from most
> operations. It's still a simple two-parent merge commit, so it can be
> pushed/pulled with any existing remotes without requiring any changes on the
> server side, but operations like adding a regular new commit to the tip of
> the "proposed changes" branch would automatically move the merge commit so
> it's back at the tip. This way "not sure what you're trying to do" situations
> like this would be avoided:
>
>   A--B--C--CLM--D--E
>  /          /
> Y-----------

Sorry, you lost me.  It is quite clear what the user is trying to do
in this case.  After preparing the initial cut of the topic that has
three commits, A, B, and C, and writing about them in CLM, there
were fix-ups and enhancements needed, which were implemented in D
and E.  The branch is waiting to be rebased into

   A--B--C--D--E--CLM'
  /               / 
 Y----------------

which gives you a chance to talk also about D and E in the updated
iteration of CLM.

Whether the "cover letter material in an empty commit at the tip" is
a single-parent commit or a needless merge, there will be fallouts,
such as "git branch --[no-]merged" becoming much less useful.  I am
sure there will be many others once we start seriously exploring
these approaches.

Re: On-branch topic description support?

[Bagas Sanjaya]

On Wed, Jul 20, 2022 at 04:40:52PM -0700, Junio C Hamano wrote:
> I've been playing with this idea from time to time, but recently I
> started seeing a related discussion on the b4 front, so I thought I
> would throw it out and how people would think.
> 
> We made "git merge" not to silently commit the result, but open the
> editor to encourage the integrator to describe what the topic is
> about.  We also made "git format-patch" prepare [PATCH 0/n] aka
> "cover letter" so that the author of the patch series can express
> what the overall topic is about.  What the author should say in the
> cover letter very much overlaps what the integrator wants to have in
> the log message of the commit that merges the topic to the
> integration branch.
> 
> But there are two (and half) links from format-patch to that merge
> commit that are missing.  
> 
>  - You cannot prepare the cover letter material while working on the
>    topic---instead, you have to write one by editing the output from
>    "format-patch --cover-letter";
> 
>  - "git am" at the receiving end discards the cover letter when
>    queuing the e-mailed patches to a topic.
> 
>  - "git merge" cannot take advantage of the cover letter that was
>    discarded when the topic was queued.
> 
> So, here is how I would imagine a slightly better world may work.
> 
>  * When you are almost finished with the initial draft of your
>    topic, you'd write a cover letter, and record it as the log
>    message of an empty commit at the tip of the topic.  As you go on
>    polishing the topic with "rebase -i", the empty commit would be
>    kept (if we currently lose an empty commit by default, we may
>    need to teach "rebase -i" to special case an empty commit at the
>    tip of the range to make this convenient to use), and you would
>    keep it up to date as you update the topic.
> 
>  * "git format-patch" would notice that the topic has such an empty
>    commit at the tip, and use the log message from it to
>    pre-populate the cover letter.
> 
>  * "git am" would learn an option to save the cover letter [0/n] and
>    create such an empty commit at the tip of the branch.
> 
>  * "git merge" would learn an option to recognize that the branch
>    being merged has such an empty commit at the tip, and instead
>    merge the parent of the tip of the branch into the integration
>    branch, while using the log message of the discarded tip commit
>    in the log message of the merge itself.
> 
> Yes, there is "git config branch.mytopic.description" that helps
> when pre-populating the cover letter, but that only helps at the
> origin, and it is not shared between your personal repositories.
> If you have the draft of the cover letter as part of the branch,
> you can push/fetch them around just like all the "real" commits
> you are working on.
> 
> Regardless of where the cover letter comes from, the changes to "am"
> and "merge" dreamed above in this message would be useful, and that
> is the primary reason why I am envisioning that right at the origin
> having the topic description as an empty commit at the tip would be
> the most convenient.  It would match the shape of the history at the
> author side and at the side who runs "git am".

Thanks for the idea.

The alternative would be "commit groups", which are named and annotated
range of commits. The group description can contain cover letter
content. This way, integrators can still have a description of merged
commits without having an empty commit. Commit groups are particularly
useful for rebase-then-merge workflow, where topic branches are rebased
first before being merged into trunk.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara