Re: [RFC] cover-at-tip

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2017, 10:30 UTC
Message-ID: <xmqqbmk68o9d.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqh8ty8q5x.fsf@gitster.mtv.corp.google.com>

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

Show 7 quoted lines

> Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> writes:
>
>> I agree this is a "am" job. Was just wondering if reusing some of
>> the code from apply (and move it so it makes more sense) wouldnd't
>> make more sense than rewriting a patch detection function.
>
> Yes, I understood that and have already given an answer, no?

This was a bit too terse to be useful, so let me try again.

I think the ideal endgame would be to allow people to come up with a topic branch of this shape (illustrated is a three-patch series on top of 'origin'):

    ---o---o (origin)
            \
             1---2---3

and then add an empty commit C whose log message is used to store "cover letter material", i.e.

    ---o---o (origin)
            \
             1---2---3---C (topic)

And then you should be able to

 (1) merge such branch yourself, coming up with a history like this,
     where merge M uses material from C in the merge log message

    ---o---o---x---x---M
            \         /
             1---2---3

 (2) "git format-patch origin..topic" that would create the cover
     letter using material found in C in addition to the usual
     stuff (like shortlog) generated by "format-patch --cover",
     followed by these three patches.

 (3) "git format-patch M" should be able to (a) realize that M
     merges a side branch that is a three-commit series (i.e.
     M^1..M^2), and (b) notice that log message of M has
     human-readable description.  Then it grabs the merge log
     message of M and do the same as (2).

 (4) "git am" the result from (2) or (3) should recreate the
     original history i.e. what we started with with C.

    ---o---o (origin)
            \
             1---2---3---C (topic)

Now, I _think_ what the machinery needs a lot more is to be able to detect C is an empty commit (when doing (2)), and then you have quite a lattitude in designing what exactly such an automated cover letter looks like, so that the receiving end (4) can recognize it more easily and (more importantly) more robustly than "the message does not have any patch in it". Not all random messages that do not have a patch in it are cover letters, and that is why I do not think touching any code in the apply layer in an attempt to "reuse" anything is a bad idea. It will risk butchering the code without any real gain, because what we really need to know is *not* absence of patch, but presence of cover letter material.

The simplest would probably be to notice that the subject of one has 0/N on it, while other messages were labeled with 1/N..(N-1)/N; that would be a lot stronger clue that 0/N has a cover than "it does not have any patch in it".

It may be that we would not just want to identify which message is cover and which message is not, but which part of the cover letter message should go back to the log message of the capping empty commit (and moved to the merge log message). Just like we invented the conventions like scissors, three-dashes, etc., you might want to come up with a way to do so in your format-patch enhancement used to do the (2) and (3) above. Then it will be the matter of teaching that convention to "am" used in (4).