Re: [RFC] cover-at-tip

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de>]

Le 13/11/2017 à 11:30, Junio C Hamano a écrit :
> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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.

Thanks ;)

>
> 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)

That what I got from the archive referenced in the leftover bits.
I'm currently focusing on (2) and (4).
(3) might come reasonably "easy" after (2) but I don't know enough about the internal API yet so I focused on the simplest ;)

>
> Now, I _think_ what the machinery needs a lot more is to be able to
> detect C is an empty commit (when doing (2)),

Unless I'm mistaken, this should be covered by the RFC for format-patch:

+			if (commit->parents && !commit->parents->next &&
+			    !oidcmp(&commit->tree->object.oid,
+				    &commit->parents->item->tree->object.oid)) {
+				cover_at_tip_commit = commit;

As I said, I'm focusing only for (2) now, so we check there is only one parent and that the commit did not change the tree hash. (meaning an empty commit right ?)

>  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.

Agreed.

> 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".

Good idea, and should be easy enough to put in place.

>
> 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).
>

I like that too. It could also allow using (4) on series with a manual cover letter without pulling the shortlog/diffstat stuff into the new topic commit.

Nicolas