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

10 messages from 2006-03-02 to 2006-03-03. Participants: Christopher Faylor, Shawn Pearce, Johannes Schindelin, Alex Riesen, Linus Torvalds, Junio C Hamano.
Thread: https://gitlist.dev/t/3525

## Christopher Faylor, 2006-03-02 16:44

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <20060302164405.GB7292@trixie.casa.cgf.cx>
URL: https://gitlist.dev/e/20060302164405.GB7292%40trixie.casa.cgf.cx

```
So to summarize:

If anyone has a problem with Cygwin where signals do not seem to be
working, I'd appreciate a bug report to the Cygwin list.  We really do
expect that things should work and want to fix things if they don't.

If that isn't possible to use the Cygwin list for some reason, I will
continue to read this mailing list and respond to Cygwin problems but I
would appreciate it if any Cygwin problem report contained details for
reproducing the problem.  We usually point people to this page
http://cygwin.com/problems.html when they have problems.  The basic take
away from that page is to provide the cygcheck output which shows what
settings have been used for your Cygwin installation.  The interesting
stuff in that output is the cygwin mount points, the CYGWIN environment
variable, and version information about the Cygwin DLL.

The Cygwin web site is http://cygwin.com/ and it has a lot of information
about Cygwin.  Some of it is undoubtedly out-of-date or unclear but we
do try to improve things if they are brought to our attention.

I don't see any reason to respond to this thread any further but I will
continue to rectify any misstatements that I see being made about
Windows or Cygwin here.

cgf (Cygwin Maintainer)

```

## Shawn Pearce, 2006-03-02 16:55

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <20060302165510.GB18929@spearce.org>
URL: https://gitlist.dev/e/20060302165510.GB18929%40spearce.org
In-Reply-To: <20060302164405.GB7292@trixie.casa.cgf.cx>

```
Maybe I missed this but why are people using the native Windows
ActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl
installation available?

I've been using the Cygwin Perl with GIT without any problems
whatsoever.  Including the open(I, "-|")... exec(@argv) code that
doesn't work correctly in ActiveState and started this whole thread.

-- 
Shawn.

```

## Johannes Schindelin, 2006-03-02 17:33

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <Pine.LNX.4.63.0603021832460.31727@wbgn013.biozentrum.uni-wuerzburg.de>
URL: https://gitlist.dev/e/Pine.LNX.4.63.0603021832460.31727%40wbgn013.biozentrum.uni-wuerzburg.de
In-Reply-To: <20060302164405.GB7292@trixie.casa.cgf.cx>

```
Hi,

On Thu, 2 Mar 2006, Christopher Faylor wrote:

> If anyone has a problem with Cygwin where signals do not seem to be
> working, I'd appreciate a bug report to the Cygwin list.  We really do
> expect that things should work and want to fix things if they don't.

I am glad to have you on this list. Thanks for all your efforts.

Ciao,
Dscho

```

## Alex Riesen, 2006-03-02 22:09

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <20060302220930.GE6183@steel.home>
URL: https://gitlist.dev/e/20060302220930.GE6183%40steel.home
In-Reply-To: <20060302165510.GB18929@spearce.org>

```
Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:
> Maybe I missed this but why are people using the native Windows
> ActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl
> installation available?

because the people _can't_ use cygwin's perl. There are a lot of
reasons mainly: administrative, perl script incompatibilities and
cygwin.dll incompatibilities (if you use perl from cygwin, it'll need
the correct cygwin.dll. And if a build process uses cygwin tools from,
for example, QNX Momentics it often comes to clashes).

> I've been using the Cygwin Perl with GIT without any problems
> whatsoever.  Including the open(I, "-|")... exec(@argv) code that
> doesn't work correctly in ActiveState and started this whole thread.

Unfortunately...

```

## Linus Torvalds, 2006-03-02 23:27

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <Pine.LNX.4.64.0603021521250.22647@g5.osdl.org>
URL: https://gitlist.dev/e/Pine.LNX.4.64.0603021521250.22647%40g5.osdl.org
In-Reply-To: <20060302220930.GE6183@steel.home>

```


On Thu, 2 Mar 2006, Alex Riesen wrote:

> Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:
> 
> > I've been using the Cygwin Perl with GIT without any problems
> > whatsoever.  Including the open(I, "-|")... exec(@argv) code that
> > doesn't work correctly in ActiveState and started this whole thread.
> 
> Unfortunately...

Here's a stupid first cut at git-fmt-merge-msg in C using the new revlist 
library interface.

It's not actually doing exactly the same thing, because I'm a lazy 
bastard, but some things it does better.

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.

The things it doesn't do:
 - the old one had a limit of 20, the new one has a limit of 10 commits 
   reported
 - 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.
 - 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.

But hey, this is all part of my cunning plan to make people get involved 
with the new rev-list libification, by giving them things that _almost_ 
work, but might need some tweaking.

		Linus

--- snip snip for "fmt-merge-msg.c" snip snip---
/*
 * fmt-merge-msg.c
 *
 * Magic auto-generation of merge messages.
 *
 * Copyright (C) 2006 Linus Torvalds and his army of programming ferrets
 */
#include "cache.h"
#include "commit.h"
#include "revision.h"

static void show_commit(struct commit *commit)
{
	char buffer[256];

	pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0, buffer, sizeof(buffer), 0);
	printf("   * %s\n", buffer);
}

int main(int argc, char **argv)
{
	struct rev_info revs;
	struct commit *commit;
	unsigned char sha1[20];
	char buffer[256];

	setup_revisions(0, NULL, &revs, NULL);
	if (get_sha1("HEAD", sha1) < 0)
		die("no HEAD revision");
	commit = lookup_commit_reference(sha1);
	if (!commit)
		die("no HEAD revision");

	commit->object.flags |= UNINTERESTING;
	insert_by_date(commit, &revs.commits);
	revs.topo_order = 1;
	revs.limited = 1;

	while (fgets(buffer, sizeof(buffer), stdin)) {
		int max;
		char *marker;

		if (get_sha1_hex(buffer, sha1) < 0)
			continue;
		commit = lookup_commit_reference(sha1);
		if (!commit)
			continue;

		/*
		 * Format after the SHA1:
		 *	<tab>marker<tab><type>'<name>' of <src>'
		 *
		 * where string is "not-for-merge" if
		 * we're not interested in this one,
		 * and empty otherwise.
		 */
		marker = buffer + 40;
		if (*marker++ != '\t')
			continue;
		if (*marker++ != '\t')
			continue;
		printf("Merge %s", marker);

		insert_by_date(commit, &revs.commits);
		prepare_revision_walk(&revs);

		max = 10;
		while ((commit = get_revision(&revs)) != NULL) {
			int n = --max;
			if (n > 0)
				show_commit(commit);
			else if (!n)
				printf("   ...");
		}
	}
}

```

## Christopher Faylor, 2006-03-03 00:14

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <20060303001434.GA7497@trixie.casa.cgf.cx>
URL: https://gitlist.dev/e/20060303001434.GA7497%40trixie.casa.cgf.cx
In-Reply-To: <20060302220930.GE6183@steel.home>

```
On Thu, Mar 02, 2006 at 11:09:30PM +0100, Alex Riesen wrote:
>Shawn Pearce, Thu, Mar 02, 2006 17:55:10 +0100:
>>Maybe I missed this but why are people using the native Windows
>>ActiveState Perl with GIT+Cygwin when Cygwin has a Cygwin-ized Perl
>>installation available?
>
>because the people _can't_ use cygwin's perl.  There are a lot of
>reasons mainly: administrative, perl script incompatibilities and
>cygwin.dll incompatibilities (if you use perl from cygwin, it'll need
>the correct cygwin.dll.  And if a build process uses cygwin tools from,
>for example, QNX Momentics it often comes to clashes).

(Hmm.  I wonder if QNX Momentics is YA GPL violator)

If you have multiple versions of the Cygwin DLL on your system and try
to use them all jumbled up together then, yes, you will have problems.
This isn't a perl-specific issue.  The solution is to put the latest
version of your Cygwin DLL in your path (presumably in /bin) and delete
all of the older ones.

The newest version is undoubtedly going to be the one downloaded from
the Cygwin web site (http://cygwin.com/) but you can get version
information from the cygwin DLL by using grep:

  grep -a "^%%% Cygwin" WHEREEVER/cygwin1.dll

if you are not inclined to install the newest version of Cygwin.

I'm sure that there are incompatibilities between ActiveState perl and
Cygwin's perl which make it hard to use the same scripts in each so I am
not doubting that some people might want to use only ActiveState perl.
I don't see how the multiple Cygwin DLL issue can be a problem only for
Cygwin perl vs. ActiveState perl.

cgf
(who sees a new full-time job looming in the git list)

```

## Junio C Hamano, 2006-03-03 00:34

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <7v1wxk5ptf.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7v1wxk5ptf.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <Pine.LNX.4.64.0603021521250.22647@g5.osdl.org>

```
Linus Torvalds <torvalds@osdl.org> 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

```

## Linus Torvalds, 2006-03-03 00:49

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <Pine.LNX.4.64.0603021643560.22647@g5.osdl.org>
URL: https://gitlist.dev/e/Pine.LNX.4.64.0603021643560.22647%40g5.osdl.org
In-Reply-To: <7v1wxk5ptf.fsf@assigned-by-dhcp.cox.net>

```


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.

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

```

## Junio C Hamano, 2006-03-03 01:25

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <7vveuw48uw.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vveuw48uw.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <Pine.LNX.4.64.0603021643560.22647@g5.osdl.org>

```
Linus Torvalds <torvalds@osdl.org> writes:

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

Abuse is exactly the word.  The reason it is an abuse is exactly
why you said "... but to fix it you'd have to actualyl walk the
parent list yourself, rather than letting get_revision do it all
for you."  Which relates to the fact that object.c layer is not
designed to be used multiple times...

Maybe we want to make object.c layer reusable first?

```

## Linus Torvalds, 2006-03-03 01:52

Subject: Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
Message-ID: <Pine.LNX.4.64.0603021747430.22647@g5.osdl.org>
URL: https://gitlist.dev/e/Pine.LNX.4.64.0603021747430.22647%40g5.osdl.org
In-Reply-To: <7vveuw48uw.fsf@assigned-by-dhcp.cox.net>

```


On Thu, 2 Mar 2006, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > 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.
> 
> Abuse is exactly the word.  The reason it is an abuse is exactly
> why you said "... but to fix it you'd have to actualyl walk the
> parent list yourself, rather than letting get_revision do it all
> for you."  Which relates to the fact that object.c layer is not
> designed to be used multiple times...
> 
> Maybe we want to make object.c layer reusable first?

No, the "abuse" is actually very much done that way on purpose. It's a bit 
strange to do "incremental" prepare_revision_walk() calls, but it all 
comes from the fact that the object structures are "persistent" across the 
calls, even if we remove them from the list when we walk them. 

So it's strange, but that was kind of part of the reason for doing it. 
It's a _good_ strangeness.

The thing about handling commits that were already in another branch but 
weren't shown is different: the way to handle that is to generate the 
_whole_ revision list in one go - instead of incrementally - and then for 
each branch you merge you show the top 10 "not yet shown" commits.

IOW, that thing would never use "get_revision()" at all, but would instead 
depend on "prepare_revision_walk()" generating the whole tree, and then 
you just walk the parent pointers from the branch heads by hand, marking 
then "seen" as you print them.

So the object layer and the revision parsing actually does exactly the 
right thing, you just have to decide on how to use them..

			Linus

```
