Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:20

Linus Torvalds [off-list ref] writes:
For example, afaik, when merging multiple branches that had partially been 
merged already (ie they had overlapping new stuff), if I read the old perl 
code correctly, it would talk about the new stuff multiple times. This one 
doesn't.
I think this is not quite right, even though it only matters in
Octopus and not many people do Octopus anyway.  Suppose you are
merging lt/rev-list and fk/blame branches into master, starting
from this state:

    ! [master] GIT-VERSION-GEN: squelch unneed
     ! [lt/rev-list] setup_revisions(): handle
      ! [fk/blame] git-blame, take 2
    ---
     +  [lt/rev-list] setup_revisions(): handl
     +  [lt/rev-list^] git-log (internal): mor
     +  [lt/rev-list~2] git-log (internal): ad
      + [fk/blame] git-blame, take 2
      - [fk/blame^] Merge part of 'lt/rev-list
     ++ [lt/rev-list~3] Rip out merge-order an
     ++ [lt/rev-list~4] Tie it all together: "
     ++ [lt/rev-list~5] Introduce trivial new 
     ++ [lt/rev-list~6] git-rev-list libificat
     ++ [lt/rev-list~7] Splitting rev-list int
     ++ [lt/rev-list~8] rev-list split: minimu
     ++ [lt/rev-list~9] First cut at libifying
      + [fk/blame~2] Add git-blame, a tool for
    --- [lt/rev-list~10] Merge branch 'maint' 

And you had lt/rev-list branch first listed in FETCH_HEAD.  In
this particular example, lt/rev-list has only 3 commits on top
of common things, but if your max were 3 instead of 10, the
first round would actually show the tip 3 without showing any
common stuff, and then the next round to show fk/blame branch
would show only the remaining two, without ever showing the
common stuff, even though it _could_ say the latest of the
common stuff.
The things it doesn't do:
 - the old one had a limit of 20, the new one has a limit of 10 commits 
   reported
Good change I would say, except for the above.
 - the old one was tested, the new one is written by me.
 - the old one honored the "merge.summary" git config option. The new one 
   doesn't.
Easily rectifiable ;-).
 - the old one did some formatting of the branch message that I don't 
   follow because I'm not a perl user. The new one just takes the 
   explanatory message for the branch merging as-is.
FETCH_HEAD has explanatory message in more or less "canonical"
form.  It has noise word "branch", and the current repository is
typically " of .".  These are removed by the code, so that you would
not have to see:

	Merge branch 'jc/delta' of .

Instead you would see:

	Merge 'jc/delta' into 'next'.

The last part, " into 'next'", is also missing from your
version.  I can distinguish a merge into 'master' (which does
not have " into 'master'") and other branches easily that way,
and I find it handy.

Other things the Perl code does are purely for Octopus support:
things like coalescing multiple branches taken from the same
repositories.  You would get something like:

	Merge 'lt/rev-list' and 'fk/blame' into 'next'.

	* lt/rev-list:
	  commit 1
          commit 2

	* fk/blame:
	  commit 3
	  commit 4

instead of (your version):

	Merge branch 'lt/rev-list' of .
	   * commit 1
           * commit 2

	Merge branch 'fk/blame' of .
	   * commit 3
           * commit 4

Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6

From: Linus Torvalds <torvalds@osdl.org>
Date: 2016-06-15 22:42:20


On Thu, 2 Mar 2006, Junio C Hamano wrote:
And you had lt/rev-list branch first listed in FETCH_HEAD.  In
this particular example, lt/rev-list has only 3 commits on top
of common things, but if your max were 3 instead of 10, the
first round would actually show the tip 3 without showing any
common stuff, and then the next round to show fk/blame branch
would show only the remaining two, without ever showing the
common stuff, even though it _could_ say the latest of the
common stuff.
Yes. I considered it briefly, and it's fixable, but to fix it you'd 
have to actualyl walk the parent list yourself, rather than letting 
get_revision do it all for you.

And what my simple thing shows isn't really technically "wrong", since it 
has shown that there are commits missing from the output with the "..."

The question is just whether shared commits should be "balanced out", or 
shown as part of the first branch that merged them. I chose the latter, 
because it's not only simple, it's unambiguous (any balancing algorithm 
will depend on some random heuristic or other, and on how many commits are 
shown.
quoted
 - the old one did some formatting of the branch message that I don't 
   follow because I'm not a perl user. The new one just takes the 
   explanatory message for the branch merging as-is.
FETCH_HEAD has explanatory message in more or less "canonical"
form.  It has noise word "branch", and the current repository is
typically " of .".
Yeah, I actually looked at a few examples, so I knew what it was basically 
trying to do, and then I ignored it as not interesting to the exercise, 
which was to abuse the new revision listing library in interesting ways by 
calling it multiple times.

		Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help