Le 13/11/2017 à 11:30, Junio C Hamano a écrit :
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Nicolas Morey-Chaisemartin <NMoreyChaisemartin@xxxxxxx> 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
