git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Should we support Perl 5.6?
@ 2006-02-20 18:37 Johannes Schindelin
  2006-02-20 19:10 ` Eric Wong
  0 siblings, 1 reply; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-20 18:37 UTC (permalink / raw)
  To: git

Hi,

I just had a failure when pulling, because since a few days (to be exact, 
since commit 1cb30387, git-fmt-merge-msg uses a syntax which is not 
understood by Perl 5.6.

It is this:

	open $fh, '-|', 'git-symbolic-ref', 'HEAD' or die "$!";

I know that there was already some discussion on this list, but I don't 
remember if we decided on leaving 5.6 behind or not.

Somebody remembers?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: Should we support Perl 5.6?
  2006-02-20 18:37 Should we support Perl 5.6? Johannes Schindelin
@ 2006-02-20 19:10 ` Eric Wong
  2006-02-20 21:01   ` Andreas Ericsson
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
  0 siblings, 2 replies; 78+ messages in thread
From: Eric Wong @ 2006-02-20 19:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> I just had a failure when pulling, because since a few days (to be exact, 
> since commit 1cb30387, git-fmt-merge-msg uses a syntax which is not 
> understood by Perl 5.6.
> 
> It is this:
> 
> 	open $fh, '-|', 'git-symbolic-ref', 'HEAD' or die "$!";

This is just 5.8 shorthand for the following (which is 5.6-compatible,
and probably for earlier versions, too):

	my $pid = open my $fh, '-|';
	defined $pid or die "Unable to fork: $!\n";
	if ($pid == 0) {
		exec 'git-symbolic-ref', 'HEAD' or die "$!";
	}
	<continue with original code here>

All of the Perl code I've written uses this method.

> I know that there was already some discussion on this list, but I don't 
> remember if we decided on leaving 5.6 behind or not.
> 
> Somebody remembers?

IIRC, there was no clear decision.

I still have some Debian Woody machines/chroots with 5.6 around in some
places.  I don't use git on them, but I may someday, but upgrading to
Sarge is more likely on those.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: Should we support Perl 5.6?
  2006-02-20 19:10 ` Eric Wong
@ 2006-02-20 21:01   ` Andreas Ericsson
  2006-02-20 21:15     ` Junio C Hamano
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
  1 sibling, 1 reply; 78+ messages in thread
From: Andreas Ericsson @ 2006-02-20 21:01 UTC (permalink / raw)
  To: Eric Wong; +Cc: Johannes Schindelin, git

Eric Wong wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
>>Hi,
>>
>>I just had a failure when pulling, because since a few days (to be exact, 
>>since commit 1cb30387, git-fmt-merge-msg uses a syntax which is not 
>>understood by Perl 5.6.
>>
>>It is this:
>>
>>	open $fh, '-|', 'git-symbolic-ref', 'HEAD' or die "$!";
> 
> 
> This is just 5.8 shorthand for the following (which is 5.6-compatible,
> and probably for earlier versions, too):
> 
> 	my $pid = open my $fh, '-|';
> 	defined $pid or die "Unable to fork: $!\n";
> 	if ($pid == 0) {
> 		exec 'git-symbolic-ref', 'HEAD' or die "$!";
> 	}
> 	<continue with original code here>
> 
> All of the Perl code I've written uses this method.
> 
> 
>>I know that there was already some discussion on this list, but I don't 
>>remember if we decided on leaving 5.6 behind or not.
>>
>>Somebody remembers?
> 
> 
> IIRC, there was no clear decision.
> 

I think we agreed not to bother at all with Perl 5.4 and earlier, and 
not to bend over backwards to support 5.6. This seems like a simple fix 
though, so I'm sure Junio will accept a patch.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: Should we support Perl 5.6?
  2006-02-20 21:01   ` Andreas Ericsson
@ 2006-02-20 21:15     ` Junio C Hamano
  0 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 21:15 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Johannes Schindelin, git

Andreas Ericsson <ae@op5.se> writes:

> I think we agreed not to bother at all with Perl 5.4 and earlier, and
> not to bend over backwards to support 5.6. This seems like a simple
> fix though, so I'm sure Junio will accept a patch.

Correct.  I wasn't being careful enough.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-20 19:10 ` Eric Wong
  2006-02-20 21:01   ` Andreas Ericsson
@ 2006-02-20 22:05   ` Junio C Hamano
  2006-02-20 22:12     ` [PATCH] rerere: " Junio C Hamano
                       ` (4 more replies)
  1 sibling, 5 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 22:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Eric Wong, git


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * Eric, thanks for the hint.  I have this four-patch series.
   Could people with perl 5.6 please check them?

 git-fmt-merge-msg.perl |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

615782c9609bf23be55b403e994d88c1047be996
diff --git a/git-fmt-merge-msg.perl b/git-fmt-merge-msg.perl
index c34ddc5..a77e94e 100755
--- a/git-fmt-merge-msg.perl
+++ b/git-fmt-merge-msg.perl
@@ -28,11 +28,12 @@ sub andjoin {
 }
 
 sub repoconfig {
-	my $fh;
 	my $val;
 	eval {
-		open $fh, '-|', 'git-repo-config', '--get', 'merge.summary'
-		    or die "$!";
+		my $pid = open(my $fh, '-|');
+		if (!$pid) {
+			exec('git-repo-config', '--get', 'merge.summary');
+		}
 		($val) = <$fh>;
 		close $fh;
 	};
@@ -41,25 +42,32 @@ sub repoconfig {
 
 sub current_branch {
 	my $fh;
-	open $fh, '-|', 'git-symbolic-ref', 'HEAD' or die "$!";
+	my $pid = open($fh, '-|');
+	die "$!" unless defined $pid;
+	if (!$pid) {
+	    exec('git-symbolic-ref', 'HEAD') or die "$!";
+	}
 	my ($bra) = <$fh>;
 	chomp($bra);
+	close $fh or die "$!";
 	$bra =~ s|^refs/heads/||;
 	if ($bra ne 'master') {
 		$bra = " into $bra";
 	} else {
 		$bra = "";
 	}
-
 	return $bra;
 }
 
 sub shortlog {
 	my ($tip, $limit) = @_;
 	my ($fh, @result);
-	open $fh, '-|', ('git-log', "--max-count=$limit", '--topo-order',
-			 '--pretty=oneline', $tip, '^HEAD')
-	    or die "$!";
+	my $pid = open($fh, '-|');
+	die "$!" unless defined $pid;
+	if (!$pid) {
+	    exec('git-log', "--max-count=$limit", '--topo-order',
+		 '--pretty=oneline', $tip, '^HEAD') or die "$!";
+	}
 	while (<$fh>) {
 		s/^[0-9a-f]{40}\s+//;
 		push @result, $_;
-- 
1.2.2.g5be4ea

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [PATCH] rerere: avoid open "-|" list form for Perl 5.6
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
@ 2006-02-20 22:12     ` Junio C Hamano
  2006-02-20 22:12     ` [PATCH] send-email: " Junio C Hamano
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 22:12 UTC (permalink / raw)
  To: git; +Cc: Eric Wong


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 git-rerere.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

46fa107ab91b25eb928a9945ce4e9143b9c36df3
diff --git a/git-rerere.perl b/git-rerere.perl
index df11951..d3664ff 100755
--- a/git-rerere.perl
+++ b/git-rerere.perl
@@ -131,7 +131,11 @@ sub record_preimage {
 sub find_conflict {
 	my $in;
 	local $/ = "\0";
-	open $in, '-|', qw(git ls-files -z -u) or die "$!: ls-files";
+	my $pid = open($in, '-|');
+	die "$!" unless defined $pid;
+	if (!$pid) {
+		exec(qw(git ls-files -z -u)) or die "$!: ls-files";
+	}
 	my %path = ();
 	my @path = ();
 	while (<$in>) {
-- 
1.2.2.g5be4ea

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [PATCH] send-email: avoid open "-|" list form for Perl 5.6
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
  2006-02-20 22:12     ` [PATCH] rerere: " Junio C Hamano
@ 2006-02-20 22:12     ` Junio C Hamano
  2006-02-20 22:12     ` [PATCH] svmimport: " Junio C Hamano
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 22:12 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Ryan Anderson


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 git-send-email.perl |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)

044ece3bc8bde227babd2f710f8216f2cb631034
diff --git a/git-send-email.perl b/git-send-email.perl
index 13b85dd..b4f04f9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -59,24 +59,29 @@ my $rc = GetOptions("from=s" => \$from,
 
 # Now, let's fill any that aren't set in with defaults:
 
-open(GITVAR,"-|","git-var","-l")
-	or die "Failed to open pipe from git-var: $!";
-
-my ($author,$committer);
-while(<GITVAR>) {
-	chomp;
-	my ($var,$data) = split /=/,$_,2;
-	my @fields = split /\s+/, $data;
-
-	my $ident = join(" ", @fields[0...(@fields-3)]);
-
-	if ($var eq 'GIT_AUTHOR_IDENT') {
-		$author = $ident;
-	} elsif ($var eq 'GIT_COMMITTER_IDENT') {
-		$committer = $ident;
-	}
+sub gitvar {
+    my ($var) = @_;
+    my $fh;
+    my $pid = open($fh, '-|');
+    die "$!" unless defined $pid;
+    if (!$pid) {
+	exec('git-var', $var) or die "$!";
+    }
+    my ($val) = <$fh>;
+    close $fh or die "$!";
+    chomp($val);
+    return $val;
+}
+
+sub gitvar_ident {
+    my ($name) = @_;
+    my $val = gitvar($name);
+    my @field = split(/\s+/, $val);
+    return join(' ', @field[0...(@field-3)]);
 }
-close(GITVAR);
+
+$author = gitvar_ident('GIT_AUTHOR_IDENT');
+$committer = gitvar_ident('GIT_COMMITTER_IDENT');
 
 my $prompting = 0;
 if (!defined $from) {
-- 
1.2.2.g5be4ea

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [PATCH] svmimport: avoid open "-|" list form for Perl 5.6
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
  2006-02-20 22:12     ` [PATCH] rerere: " Junio C Hamano
  2006-02-20 22:12     ` [PATCH] send-email: " Junio C Hamano
@ 2006-02-20 22:12     ` Junio C Hamano
  2006-02-20 22:19     ` [PATCH] cvsimport: " Junio C Hamano
  2006-02-21 17:30     ` [PATCH] fmt-merge-msg: " Alex Riesen
  4 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 22:12 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Matthias Urlichs


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 git-svnimport.perl |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

f7e8f8415c5c88082daecac44cfbba561113a3d9
diff --git a/git-svnimport.perl b/git-svnimport.perl
index c536d70..ee2940f 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -10,7 +10,6 @@
 # The head revision is on branch "origin" by default.
 # You can change that with the '-o' option.
 
-require 5.008; # for shell-safe open("-|",LIST)
 use strict;
 use warnings;
 use Getopt::Std;
@@ -322,8 +321,12 @@ sub get_file($$$) {
 		return undef unless defined $name;
 	}
 
-	open my $F, '-|', "git-hash-object", "-w", $name
+	my $pid = open(my $F, '-|');
+	die $! unless defined $pid;
+	if (!$pid) {
+	    exec("git-hash-object", "-w", $name)
 		or die "Cannot create object: $!\n";
+	}
 	my $sha = <$F>;
 	chomp $sha;
 	close $F;
@@ -398,7 +401,12 @@ sub copy_path($$$$$$$$) {
 			$srcpath =~ s#/*$#/#;
 	}
 	
-	open my $f,"-|","git-ls-tree","-r","-z",$gitrev,$srcpath;
+	my $pid = open my $f,'-|';
+	die $! unless defined $pid;
+	if (!$pid) {
+		exec("git-ls-tree","-r","-z",$gitrev,$srcpath)
+			or die $!;
+	}
 	local $/ = "\0";
 	while(<$f>) {
 		chomp;
@@ -554,7 +562,11 @@ sub commit {
 				@o1 = @old;
 				@old = ();
 			}
-			open my $F, "-|", "git-ls-files", "-z", @o1 or die $!;
+			my $pid = open my $F, "-|";
+			die "$!" unless defined $pid;
+			if (!$pid) {
+				exec("git-ls-files", "-z", @o1) or die $!;
+			}
 			@o1 = ();
 			local $/ = "\0";
 			while(<$F>) {
-- 
1.2.2.g5be4ea

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [PATCH] cvsimport: avoid open "-|" list form for Perl 5.6
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
                       ` (2 preceding siblings ...)
  2006-02-20 22:12     ` [PATCH] svmimport: " Junio C Hamano
@ 2006-02-20 22:19     ` Junio C Hamano
  2006-02-21 17:30     ` [PATCH] fmt-merge-msg: " Alex Riesen
  4 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-20 22:19 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Matthias Urlichs, Martin Langhoff


Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 * Fifth of the four patch series.  I cannot count ;-).

 git-cvsimport.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

eb815c1bb8a40ae18d80e99f8547137ea05318bf
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 24f9834..b46469a 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -846,8 +846,12 @@ while(<CVS>) {
 			print "Drop $fn\n" if $opt_v;
 		} else {
 			print "".($init ? "New" : "Update")." $fn: $size bytes\n" if $opt_v;
-			open my $F, '-|', "git-hash-object -w $tmpname"
+			my $pid = open(my $F, '-|');
+			die $! unless defined $pid;
+			if (!$pid) {
+			    exec("git-hash-object", "-w", $tmpname)
 				or die "Cannot create object: $!\n";
+			}
 			my $sha = <$F>;
 			chomp $sha;
 			close $F;
-- 
1.2.2.g5be4ea

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
                       ` (3 preceding siblings ...)
  2006-02-20 22:19     ` [PATCH] cvsimport: " Junio C Hamano
@ 2006-02-21 17:30     ` Alex Riesen
  2006-02-21 20:36       ` Sam Vilain
  2006-02-21 20:56       ` Eric Wong
  4 siblings, 2 replies; 78+ messages in thread
From: Alex Riesen @ 2006-02-21 17:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Eric Wong, git

On 2/20/06, Junio C Hamano <junkio@cox.net> wrote:
>  * Eric, thanks for the hint.  I have this four-patch series.
>    Could people with perl 5.6 please check them?

Does not work here (ActiveState Build 811, Perl 5.8.6):

$ perl -e 'open(F, "-|")'
'-' is not recognized as an internal or external command,
operable program or batch file.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 17:30     ` [PATCH] fmt-merge-msg: " Alex Riesen
@ 2006-02-21 20:36       ` Sam Vilain
  2006-02-21 21:57         ` Alex Riesen
  2006-02-21 20:56       ` Eric Wong
  1 sibling, 1 reply; 78+ messages in thread
From: Sam Vilain @ 2006-02-21 20:36 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Johannes Schindelin, Eric Wong, git

Alex Riesen wrote:
> On 2/20/06, Junio C Hamano <junkio@cox.net> wrote:
> 
>> * Eric, thanks for the hint.  I have this four-patch series.
>>   Could people with perl 5.6 please check them?
> 
> 
> Does not work here (ActiveState Build 811, Perl 5.8.6):
> 
> $ perl -e 'open(F, "-|")'
> '-' is not recognized as an internal or external command,
> operable program or batch file.

Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.

Sam.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 17:30     ` [PATCH] fmt-merge-msg: " Alex Riesen
  2006-02-21 20:36       ` Sam Vilain
@ 2006-02-21 20:56       ` Eric Wong
  2006-02-21 22:04         ` Alex Riesen
  1 sibling, 1 reply; 78+ messages in thread
From: Eric Wong @ 2006-02-21 20:56 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Johannes Schindelin, git

Alex Riesen <raa.lkml@gmail.com> wrote:
> On 2/20/06, Junio C Hamano <junkio@cox.net> wrote:
> >  * Eric, thanks for the hint.  I have this four-patch series.
> >    Could people with perl 5.6 please check them?
> 
> Does not work here (ActiveState Build 811, Perl 5.8.6):
> 
> $ perl -e 'open(F, "-|")'
> '-' is not recognized as an internal or external command,
> operable program or batch file.

Both "-|" and "|-" forms of open() use fork() internally.  Iirc, fork()
doesn't work too well on that platform.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 20:36       ` Sam Vilain
@ 2006-02-21 21:57         ` Alex Riesen
  2006-02-21 22:19           ` Johannes Schindelin
  2006-02-21 22:38           ` Sam Vilain
  0 siblings, 2 replies; 78+ messages in thread
From: Alex Riesen @ 2006-02-21 21:57 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Junio C Hamano, Johannes Schindelin, Eric Wong, git

Sam Vilain, Tue, Feb 21, 2006 21:36:50 +0100:
> >
> >>* Eric, thanks for the hint.  I have this four-patch series.
> >>  Could people with perl 5.6 please check them?
> >
> >
> >Does not work here (ActiveState Build 811, Perl 5.8.6):
> >
> >$ perl -e 'open(F, "-|")'
> >'-' is not recognized as an internal or external command,
> >operable program or batch file.
> 
> Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> 

Sometimes an upgrade is just out of question. Besides, that'd mean an
upgrade to another operating system, because very important scripts
over here a just not portable to anything else but
    "ActiveState Perl on Windows (TM)"
I just have no choice.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 20:56       ` Eric Wong
@ 2006-02-21 22:04         ` Alex Riesen
       [not found]           ` <1cf1c57a0602211412r1988b14ao435edd29207dc0d0@mail.gmail.com>
  0 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-02-21 22:04 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, Johannes Schindelin, git

Eric Wong, Tue, Feb 21, 2006 21:56:18 +0100:
> > >  * Eric, thanks for the hint.  I have this four-patch series.
> > >    Could people with perl 5.6 please check them?
> > 
> > Does not work here (ActiveState Build 811, Perl 5.8.6):
> > 
> > $ perl -e 'open(F, "-|")'
> > '-' is not recognized as an internal or external command,
> > operable program or batch file.
> 
> Both "-|" and "|-" forms of open() use fork() internally.  Iirc, fork()
> doesn't work too well on that platform.
> 

AFAICS, it does not exist. There is emulation of it in that active-perl,
though so this works:

    if ( !fork ) { something }

but not "too well" (you have to be carefule not spawn too many (which
is around 50) processes. Perl'll crash otherwise).

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
       [not found]           ` <1cf1c57a0602211412r1988b14ao435edd29207dc0d0@mail.gmail.com>
@ 2006-02-21 22:13             ` Ron Parker
  0 siblings, 0 replies; 78+ messages in thread
From: Ron Parker @ 2006-02-21 22:13 UTC (permalink / raw)
  To: git

On 2/21/06, Alex Riesen <raa.lkml@gmail.com> wrote:

> AFAICS, it does not exist. There is emulation of it in that active-perl,
> though so this works:
>
>     if ( !fork ) { something }
>
> but not "too well" (you have to be carefule not spawn too many (which
> is around 50) processes. Perl'll crash otherwise).

IIRC this has to do with some child-process thread limits in Windows.

--
Windows, the multi-thrashing OS.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 21:57         ` Alex Riesen
@ 2006-02-21 22:19           ` Johannes Schindelin
  2006-02-21 22:35             ` Eric Wong
                               ` (2 more replies)
  2006-02-21 22:38           ` Sam Vilain
  1 sibling, 3 replies; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-21 22:19 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Sam Vilain, Junio C Hamano, Eric Wong, git

Hi,

On Tue, 21 Feb 2006, Alex Riesen wrote:

> Sam Vilain, Tue, Feb 21, 2006 21:36:50 +0100:
> > >
> > >>* Eric, thanks for the hint.  I have this four-patch series.
> > >>  Could people with perl 5.6 please check them?
> > >
> > >
> > >Does not work here (ActiveState Build 811, Perl 5.8.6):
> > >
> > >$ perl -e 'open(F, "-|")'
> > >'-' is not recognized as an internal or external command,
> > >operable program or batch file.
> > 
> > Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> > 
> 
> Sometimes an upgrade is just out of question. Besides, that'd mean an
> upgrade to another operating system, because very important scripts
> over here a just not portable to anything else but
>     "ActiveState Perl on Windows (TM)"
> I just have no choice.

Maybe I am stating the obvious, but it seems that

	open (F, "git-blabla -option |");

would be more portable.

Alex, would this work on ActiveState?

Perl gurus, is the latter way to open a pipe considered awful or what?

Ciao,
Dscho

P.S.: Eric, we rely on fork() anyway. Most of git's programs just don't 
work without a fork().

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 22:19           ` Johannes Schindelin
@ 2006-02-21 22:35             ` Eric Wong
  2006-02-21 22:38             ` Shawn Pearce
  2006-02-21 23:00             ` Martin Langhoff
  2 siblings, 0 replies; 78+ messages in thread
From: Eric Wong @ 2006-02-21 22:35 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alex Riesen, Sam Vilain, Junio C Hamano, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Tue, 21 Feb 2006, Alex Riesen wrote:
> 
> > Sam Vilain, Tue, Feb 21, 2006 21:36:50 +0100:
> > > >
> > > >>* Eric, thanks for the hint.  I have this four-patch series.
> > > >>  Could people with perl 5.6 please check them?
> > > >
> > > >
> > > >Does not work here (ActiveState Build 811, Perl 5.8.6):
> > > >
> > > >$ perl -e 'open(F, "-|")'
> > > >'-' is not recognized as an internal or external command,
> > > >operable program or batch file.
> > > 
> > > Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> > > 
> > 
> > Sometimes an upgrade is just out of question. Besides, that'd mean an
> > upgrade to another operating system, because very important scripts
> > over here a just not portable to anything else but
> >     "ActiveState Perl on Windows (TM)"
> > I just have no choice.
> 
> Maybe I am stating the obvious, but it seems that
> 
> 	open (F, "git-blabla -option |");
> 
> would be more portable.
> 
> Alex, would this work on ActiveState?
> 
> Perl gurus, is the latter way to open a pipe considered awful or what?

It's OK as long as all arguments are are shell-safe (quoted/escaped
properly).  Shouldn't be a problem with constant strings at all.

> P.S.: Eric, we rely on fork() anyway. Most of git's programs just don't 
> work without a fork().

Yes, apparently there's some fork() emulation in some *doze places and
not others.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 22:19           ` Johannes Schindelin
  2006-02-21 22:35             ` Eric Wong
@ 2006-02-21 22:38             ` Shawn Pearce
  2006-02-21 23:00             ` Martin Langhoff
  2 siblings, 0 replies; 78+ messages in thread
From: Shawn Pearce @ 2006-02-21 22:38 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Maybe I am stating the obvious, but it seems that
> 
> 	open (F, "git-blabla -option |");
> 
> would be more portable.

Yes but that gets broken up and processed according to your shell.
Which could be ugly if you try to include shell meta-characters.
On the other hand if the entire string passed to open is a constant
in the script then there's really no danger and it would be more
portable.
 
> P.S.: Eric, we rely on fork() anyway. Most of git's programs just don't 
> work without a fork().

Which is why GIT requires Cygwin on Windows.  So why not use
the Cygwin perl when using GIT?  I think that uses Cygwin's fork
emulation to implement fork, rather than the ActiveState emulation
of fork.

Of course fork on Cygwin is painfully slow.  :-|

-- 
Shawn.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 21:57         ` Alex Riesen
  2006-02-21 22:19           ` Johannes Schindelin
@ 2006-02-21 22:38           ` Sam Vilain
  2006-02-22 16:35             ` Alex Riesen
  1 sibling, 1 reply; 78+ messages in thread
From: Sam Vilain @ 2006-02-21 22:38 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Johannes Schindelin, Eric Wong, git

Alex Riesen wrote:
>>>Does not work here (ActiveState Build 811, Perl 5.8.6):
>>>$ perl -e 'open(F, "-|")'
>>>'-' is not recognized as an internal or external command,
>>>operable program or batch file.
>>Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> Sometimes an upgrade is just out of question. Besides, that'd mean an
> upgrade to another operating system, because very important scripts
> over here a just not portable to anything else but
>     "ActiveState Perl on Windows (TM)"
> I just have no choice.

Sure, but perhaps IPC::Open2 or some other CPAN module has solved this 
problem already.

I guess what I'm saying is that if you want to limit the modules that 
Perl script uses, you end up either impacting on the portability of the 
script or rediscovering problems with early wheel designs.

Sam.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 22:19           ` Johannes Schindelin
  2006-02-21 22:35             ` Eric Wong
  2006-02-21 22:38             ` Shawn Pearce
@ 2006-02-21 23:00             ` Martin Langhoff
  2 siblings, 0 replies; 78+ messages in thread
From: Martin Langhoff @ 2006-02-21 23:00 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Alex Riesen, Sam Vilain, Junio C Hamano, Eric Wong, git

On 2/22/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Maybe I am stating the obvious, but it seems that
>
>         open (F, "git-blabla -option |");
>
> would be more portable.


And

    open (F, "git-blabla|", '-option', '$%!|');

would be portable AND safe ;-)

cheers,


martin

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-21 22:38           ` Sam Vilain
@ 2006-02-22 16:35             ` Alex Riesen
  2006-02-22 19:44               ` Johannes Schindelin
                                 ` (3 more replies)
  0 siblings, 4 replies; 78+ messages in thread
From: Alex Riesen @ 2006-02-22 16:35 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Junio C Hamano, Johannes Schindelin, Eric Wong, git

On 2/21/06, Sam Vilain <sam@vilain.net> wrote:
> Alex Riesen wrote:
> >>>Does not work here (ActiveState Build 811, Perl 5.8.6):
> >>>$ perl -e 'open(F, "-|")'
> >>>'-' is not recognized as an internal or external command,
> >>>operable program or batch file.
> >>Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> > Sometimes an upgrade is just out of question. Besides, that'd mean an
> > upgrade to another operating system, because very important scripts
> > over here a just not portable to anything else but
> >     "ActiveState Perl on Windows (TM)"
> > I just have no choice.
>
> Sure, but perhaps IPC::Open2 or some other CPAN module has solved this
> problem already.

IPC::Open2 works! Well "kind of": there are still strange segfaults regarding
stack sometimes. And I don't know yet whether and how the arguments are escaped
(Windows has no argument array. It has that bloody stupid one-line command line)

> I guess what I'm saying is that if you want to limit the modules that
> Perl script uses, you end up either impacting on the portability of the
> script or rediscovering problems with early wheel designs.

IPC::Open{2,3} seem to be installed on every system I have access to.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 16:35             ` Alex Riesen
@ 2006-02-22 19:44               ` Johannes Schindelin
  2006-02-22 19:51               ` Sam Vilain
                                 ` (2 subsequent siblings)
  3 siblings, 0 replies; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-22 19:44 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Sam Vilain, Junio C Hamano, Eric Wong, git

Hi,

On Wed, 22 Feb 2006, Alex Riesen wrote:

> IPC::Open{2,3} seem to be installed on every system I have access to.

I can confirm that the platforms I usually work on also provide it 
(Linux, Linux, old IRIX, old macosx, MinGW32).

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 16:35             ` Alex Riesen
  2006-02-22 19:44               ` Johannes Schindelin
@ 2006-02-22 19:51               ` Sam Vilain
  2006-02-22 19:54                 ` Junio C Hamano
  2006-02-22 22:00               ` Johannes Schindelin
  2006-02-24 12:02               ` Eric Wong
  3 siblings, 1 reply; 78+ messages in thread
From: Sam Vilain @ 2006-02-22 19:51 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git

Alex Riesen wrote:
>>I guess what I'm saying is that if you want to limit the modules that
>>Perl script uses, you end up either impacting on the portability of the
>>script or rediscovering problems with early wheel designs.
> IPC::Open{2,3} seem to be installed on every system I have access to.

Checking in Module::CoreList, that module goes right back to the Perl 
5.0 release, so every normal Perl 5 distribution should have it.

Sam.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 19:51               ` Sam Vilain
@ 2006-02-22 19:54                 ` Junio C Hamano
  0 siblings, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-22 19:54 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git

Sam Vilain <sam@vilain.net> writes:

> Checking in Module::CoreList, that module goes right back to the Perl
> 5.0 release, so every normal Perl 5 distribution should have it.

Good digging, but IIRC this thread started because something
that _claims_ to be 5.8 does not grok open(F, '-|') correctly,
so...

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 16:35             ` Alex Riesen
  2006-02-22 19:44               ` Johannes Schindelin
  2006-02-22 19:51               ` Sam Vilain
@ 2006-02-22 22:00               ` Johannes Schindelin
  2006-02-22 22:25                 ` Junio C Hamano
  2006-02-23  8:00                 ` Alex Riesen
  2006-02-24 12:02               ` Eric Wong
  3 siblings, 2 replies; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-22 22:00 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Eric Wong, git

Hi,

On Wed, 22 Feb 2006, Alex Riesen wrote:

> IPC::Open2 works!

Note that there is a notable decrease in performance in my preliminary 
tests (about 10%).

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 22:00               ` Johannes Schindelin
@ 2006-02-22 22:25                 ` Junio C Hamano
  2006-02-23  8:00                 ` Alex Riesen
  1 sibling, 0 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-22 22:25 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Johannes Schindelin

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 22 Feb 2006, Alex Riesen wrote:
>
>> IPC::Open2 works!
>
> Note that there is a notable decrease in performance in my preliminary 
> tests (about 10%).

Doesn't open(F, "| foo bar") or open(F, "foo bar |") with
careful shell quoting work?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 22:00               ` Johannes Schindelin
  2006-02-22 22:25                 ` Junio C Hamano
@ 2006-02-23  8:00                 ` Alex Riesen
  2006-02-23  8:45                   ` Junio C Hamano
  1 sibling, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-02-23  8:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Eric Wong, git

On 2/22/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > IPC::Open2 works!
>
> Note that there is a notable decrease in performance in my preliminary
> tests (about 10%).

I'll keep that in mind. But there are places where a safe pipe is unavoidable
(filenames. No amount of careful quoting will save you).

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23  8:00                 ` Alex Riesen
@ 2006-02-23  8:45                   ` Junio C Hamano
  2006-02-23  9:35                     ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Junio C Hamano @ 2006-02-23  8:45 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git

"Alex Riesen" <raa.lkml@gmail.com> writes:

> I'll keep that in mind. But there are places where a safe pipe is unavoidable
> (filenames. No amount of careful quoting will save you).

Huh?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23  8:45                   ` Junio C Hamano
@ 2006-02-23  9:35                     ` Alex Riesen
  2006-02-23  9:41                       ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-02-23  9:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
>
> > I'll keep that in mind. But there are places where a safe pipe is unavoidable
> > (filenames. No amount of careful quoting will save you).
>
> Huh?
>

Because you never know what did the next interpreter took for unquoting:
$SHELL, /bin/sh cmd /c, or something else.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23  9:35                     ` Alex Riesen
@ 2006-02-23  9:41                       ` Alex Riesen
  2006-02-23  9:48                         ` Andreas Ericsson
  0 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-02-23  9:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 2/23/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
> > "Alex Riesen" <raa.lkml@gmail.com> writes:
> >
> > > I'll keep that in mind. But there are places where a safe pipe is unavoidable
> > > (filenames. No amount of careful quoting will save you).
> >
> > Huh?
>
> Because you never know what did the next interpreter took for unquoting:
> $SHELL, /bin/sh cmd /c, or something else.
>
And that stupid activestate thing actually doesn't use any. Just tried:

  perl -e '$,=" ";open(F, "sleep 1000 ; # @ARGV |") and print <F>'

It passed the whole string "1000 ; # @ARGV" to sleep from $PATH.
It failed to sleep at all, of course. The same code works perfectly on
almost any UNIX system.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23  9:41                       ` Alex Riesen
@ 2006-02-23  9:48                         ` Andreas Ericsson
  2006-02-23 10:10                           ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Andreas Ericsson @ 2006-02-23  9:48 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git

Alex Riesen wrote:
> On 2/23/06, Alex Riesen <raa.lkml@gmail.com> wrote:
> 
>>On 2/23/06, Junio C Hamano <junkio@cox.net> wrote:
>>
>>>"Alex Riesen" <raa.lkml@gmail.com> writes:
>>>
>>>
>>>>I'll keep that in mind. But there are places where a safe pipe is unavoidable
>>>>(filenames. No amount of careful quoting will save you).
>>>
>>>Huh?
>>
>>Because you never know what did the next interpreter took for unquoting:
>>$SHELL, /bin/sh cmd /c, or something else.
>>
> 
> And that stupid activestate thing actually doesn't use any. Just tried:
> 
>   perl -e '$,=" ";open(F, "sleep 1000 ; # @ARGV |") and print <F>'
> 
> It passed the whole string "1000 ; # @ARGV" to sleep from $PATH.
> It failed to sleep at all, of course. The same code works perfectly on
> almost any UNIX system.


Not to be unhelpful or anything, but activestate perl seems to be quite 
a lot of bother. Is it worth supporting it?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23  9:48                         ` Andreas Ericsson
@ 2006-02-23 10:10                           ` Alex Riesen
  2006-02-23 13:29                             ` Andreas Ericsson
  0 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-02-23 10:10 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, git

On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> Not to be unhelpful or anything, but activestate perl seems to be quite
> a lot of bother. Is it worth supporting it?

It's not activestate perl actually. It's only one platform it also
_has_ to support.
Is it worth supporting Windows?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 10:10                           ` Alex Riesen
@ 2006-02-23 13:29                             ` Andreas Ericsson
  2006-02-23 14:07                               ` Alex Riesen
  2006-02-26 20:33                               ` Christopher Faylor
  0 siblings, 2 replies; 78+ messages in thread
From: Andreas Ericsson @ 2006-02-23 13:29 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git

Alex Riesen wrote:
> On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> 
>>Not to be unhelpful or anything, but activestate perl seems to be quite
>>a lot of bother. Is it worth supporting it?
> 
> 
> It's not activestate perl actually. It's only one platform it also
> _has_ to support.
> Is it worth supporting Windows?


With or without cygwin? With cygwin, I'd say "yes, unless it makes 
things terribly difficult to maintain and so long as we don't take 
performance hits on unices". Without cygwin, I'd say "What? It runs on 
windows?".

If we claim to support windows but do a poor job of it, no-one else will 
start working on a windows-port. If we don't claim to support windows 
but say that "it's known to work with cygwin, although be aware of these 
performance penalties...", eventually someone will come along with their 
shiny Visual Express and hack up support for it, even if some tools will 
be missing and others unnecessarily complicated.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 13:29                             ` Andreas Ericsson
@ 2006-02-23 14:07                               ` Alex Riesen
  2006-02-23 14:22                                 ` Andreas Ericsson
                                                   ` (2 more replies)
  2006-02-26 20:33                               ` Christopher Faylor
  1 sibling, 3 replies; 78+ messages in thread
From: Alex Riesen @ 2006-02-23 14:07 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, git

On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> >
> >>Not to be unhelpful or anything, but activestate perl seems to be quite
> >>a lot of bother. Is it worth supporting it?
> >
> >
> > It's not activestate perl actually. It's only one platform it also
> > _has_ to support.
> > Is it worth supporting Windows?
>
> With or without cygwin? With cygwin, I'd say "yes, unless it makes
> things terribly difficult to maintain and so long as we don't take
> performance hits on unices". Without cygwin, I'd say "What? It runs on
> windows?".

There not much difference with or without cygwin. The penalties of
doing any kind of support for it will pile up (as they started to do
with pipes).
Someday we'll have to start dropping features on Windows or restrict them
beyond their usefullness. The fork emulation in cygwin isn't perfect,
signals do not work reliably (if at all), filesystem is slow and locked down,
and exec-attribute is NOT really useful even on NTFS (it is somehow related
to execute permission and open files. I still cannot figure out how exactly
are they related).

> If we claim to support windows but do a poor job of it, no-one else will
> start working on a windows-port. If we don't claim to support windows
> but say that "it's known to work with cygwin, although be aware of these
> performance penalties...", eventually someone will come along with their
> shiny Visual Express and hack up support for it, even if some tools will
> be missing and others unnecessarily complicated.

That seem to be the case, except for shiny.
(I really don't know what could possibly mean by that. It stinks, smears,
and sometimes bounces. Never saw it shining).

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 14:07                               ` Alex Riesen
@ 2006-02-23 14:22                                 ` Andreas Ericsson
  2006-02-23 17:13                                 ` Linus Torvalds
  2006-02-26 19:55                                 ` Christopher Faylor
  2 siblings, 0 replies; 78+ messages in thread
From: Andreas Ericsson @ 2006-02-23 14:22 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, git

Alex Riesen wrote:
> On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
> 
>>If we claim to support windows but do a poor job of it, no-one else will
>>start working on a windows-port. If we don't claim to support windows
>>but say that "it's known to work with cygwin, although be aware of these
>>performance penalties...", eventually someone will come along with their
>>shiny Visual Express and hack up support for it, even if some tools will
>>be missing and others unnecessarily complicated.
> 
> 
> That seem to be the case, except for shiny.
> (I really don't know what could possibly mean by that. It stinks, smears,
> and sometimes bounces. Never saw it shining).
> 

The logo has a little glint thing on it, like those things that go 
'ting' on a front tooth in commercials for toothpaste and particularly 
healthy chewing gum.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 14:07                               ` Alex Riesen
  2006-02-23 14:22                                 ` Andreas Ericsson
@ 2006-02-23 17:13                                 ` Linus Torvalds
  2006-02-23 19:32                                   ` Junio C Hamano
  2006-02-23 21:43                                   ` Alex Riesen
  2006-02-26 19:55                                 ` Christopher Faylor
  2 siblings, 2 replies; 78+ messages in thread
From: Linus Torvalds @ 2006-02-23 17:13 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Andreas Ericsson, Junio C Hamano, git



On Thu, 23 Feb 2006, Alex Riesen wrote:
>
> Someday we'll have to start dropping features on Windows or restrict them
> beyond their usefullness.

One thing that would help a bit would be to avoid shell.

There are many portable interpreters out there, and I don't mean perl. And 
writing a small "specialized for git" one isn't even that hard. In fact, 
most of the shell (and bash) hackery we do now would be unnecessary if we 
just made a small "git interpreter" that ran "git scripts".

The fact that it would also help portability is just an added advantage.

		Linus

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 17:13                                 ` Linus Torvalds
@ 2006-02-23 19:32                                   ` Junio C Hamano
  2006-02-23 19:38                                     ` Johannes Schindelin
  2006-02-23 19:51                                     ` Linus Torvalds
  2006-02-23 21:43                                   ` Alex Riesen
  1 sibling, 2 replies; 78+ messages in thread
From: Junio C Hamano @ 2006-02-23 19:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

Linus Torvalds <torvalds@osdl.org> writes:

> There are many portable interpreters out there, and I don't mean perl. And 
> writing a small "specialized for git" one isn't even that hard. In fact, 
> most of the shell (and bash) hackery we do now would be unnecessary if we 
> just made a small "git interpreter" that ran "git scripts".

Before anybody mentions tcl ;-).

I agree with the above in principle, but I am afraid that is
only half of the solution to the problem Alex is having.

In the longer term, libified git with script language bindings
would make the way git things work together a lot better.  I've
always wanted to make merge-base a subroutine callable from
other things, so that I can say "git diff A...B" to mean "diff
up to B since B forked from A" ;-).

That way, we would eliminate the current common pattern of
piping rev-list output to diff-tree, or ls-files/diff-files
output to update-index --stdin.  These components live in the
single process, a calling "git script", and will talk with each
other internally.

But we do need to talk to non-git things.  git-grep needs a way
for ls-files to drive xargs/grep, for example.  diff --cc reads
from GNU diff output.  And for these external tools, the way
they expect the input to be fed to them or their output is taken
out is via UNIXy pipe.

And the breakage Alex wants to work around is that the platform
is not friendly to pipes, if you deny Cygwin.  So I suspect
avoiding shell would not help much.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 19:32                                   ` Junio C Hamano
@ 2006-02-23 19:38                                     ` Johannes Schindelin
  2006-02-23 19:54                                       ` Linus Torvalds
  2006-02-23 19:51                                     ` Linus Torvalds
  1 sibling, 1 reply; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-23 19:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git

Hi,

On Thu, 23 Feb 2006, Junio C Hamano wrote:

> Linus Torvalds <torvalds@osdl.org> writes:
> 
> > There are many portable interpreters out there, and I don't mean perl. And 
> > writing a small "specialized for git" one isn't even that hard. In fact, 
> > most of the shell (and bash) hackery we do now would be unnecessary if we 
> > just made a small "git interpreter" that ran "git scripts".
> 
> Before anybody mentions tcl ;-).

Darn, I had my suggestion sent out: Java ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 19:32                                   ` Junio C Hamano
  2006-02-23 19:38                                     ` Johannes Schindelin
@ 2006-02-23 19:51                                     ` Linus Torvalds
  2006-02-23 20:31                                       ` Sam Vilain
  1 sibling, 1 reply; 78+ messages in thread
From: Linus Torvalds @ 2006-02-23 19:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git



On Thu, 23 Feb 2006, Junio C Hamano wrote:
>
> Linus Torvalds <torvalds@osdl.org> writes: 
> > There are many portable interpreters out there, and I don't mean perl. And 
> > writing a small "specialized for git" one isn't even that hard. In fact, 
> > most of the shell (and bash) hackery we do now would be unnecessary if we 
> > just made a small "git interpreter" that ran "git scripts".
> 
> Before anybody mentions tcl ;-).

Well, I was thinking more of the "embeddable" ones - things that are so 
small that they can be compiled with the project. Things like Lua.

Now, Lua is not really very useful for this use case: our scripts are much 
more about combining other programs - piping the output from one to the 
other - than about any traditional scripting. Which, afaik, Lua isn't good 
at.

> I agree with the above in principle, but I am afraid that is
> only half of the solution to the problem Alex is having.
> 
> In the longer term, libified git with script language bindings
> would make the way git things work together a lot better.  I've
> always wanted to make merge-base a subroutine callable from
> other things, so that I can say "git diff A...B" to mean "diff
> up to B since B forked from A" ;-).

Yeah, we should libify some of it, to make things easier. That said, I 
don't belive in the "big-picture" libification. The fact is, a lot of git 
really _is_ about piping things from one part to another, and library 
interfaces work horribly badly for that. You really want more of a 
"stream" interface, and that's just not something I see happening.

I think one of the strengths of git is that you can use it in a very 
traditional UNIX manner, and do your own pipelines. And that will 
obviously NEVER work well under Windows, if only because it's not the 
natural way to do things.

Again, libification does nothing for that thing.

What I'd suggest using an embedded interpreter for is literally just the 
common helper scripts. We'll never make 

	git-rev-list --header a..b -- tree | 
		grep -z '^author.*torvalds' |
		..

style interesting power-user pipelines work in windows, but we _can_ make 
the things like "git commit" work natively in windows without having to 
re-write it in C by just having an embedded interpreter.

And I very much mean _embedded_. Otherwise we'll just have all the same 
problems with perl and bash and versioning. 

> But we do need to talk to non-git things.  git-grep needs a way
> for ls-files to drive xargs/grep, for example.  diff --cc reads
> from GNU diff output.  And for these external tools, the way
> they expect the input to be fed to them or their output is taken
> out is via UNIXy pipe.

I was really thinking more of a simple shell-like script interpreter. 
Something that we can make portable, by virtue of it _not_ being real 
shell. For example, the "find | xargs" stuff we do is really not that hard 
to do portably even on windows using standard C, it's just that you can't 
do it THAT WAY portably without assuming that it's a full cygwin thing.

		Linus

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 19:38                                     ` Johannes Schindelin
@ 2006-02-23 19:54                                       ` Linus Torvalds
  2006-02-23 20:19                                         ` Johannes Schindelin
  0 siblings, 1 reply; 78+ messages in thread
From: Linus Torvalds @ 2006-02-23 19:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git



On Thu, 23 Feb 2006, Johannes Schindelin wrote:
> > 
> > Before anybody mentions tcl ;-).
> 
> Darn, I had my suggestion sent out: Java ;-)

I do see the smileys, but the fact is, "perl" is a hell of a lot more 
portable than either, if we want to talk executing processes and pipelines 
etc. But even perl is clearly not portable enough, and has tons of version 
skew.

Java, afaik, has absolutely _zero_ support for creating a new process and 
piping its output to another one and doing things like safe argument 
expansion. Which is what almost all of the git scripts are all about.

		Linus

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 19:54                                       ` Linus Torvalds
@ 2006-02-23 20:19                                         ` Johannes Schindelin
  0 siblings, 0 replies; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-23 20:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git

Hi,

On Thu, 23 Feb 2006, Linus Torvalds wrote:

> On Thu, 23 Feb 2006, Johannes Schindelin wrote:
> > > 
> > > Before anybody mentions tcl ;-).
> > 
> > Darn, I had my suggestion sent out: Java ;-)
> 
> I do see the smileys, but the fact is, "perl" is a hell of a lot more 
> portable than either, if we want to talk executing processes and pipelines 
> etc. But even perl is clearly not portable enough, and has tons of version 
> skew.
> 
> Java, afaik, has absolutely _zero_ support for creating a new process and 
> piping its output to another one and doing things like safe argument 
> expansion. Which is what almost all of the git scripts are all about.

You are right, but for the wrong reason. Java is actually a wonderful 
thing to create new processes and talk between threads.

But Java is HUGE. No, it is rather HOOODGEEE.

And I don't know if something like Lua does any good. The problem is not 
so much the language. It is the fork().

AFAIAC, cygwin is pretty good at hiding Windows behind sortofa POSIX 
layer. <tongue-in-cheek>It hides it behind a POSIX layer *and* a 
performance hit.</tongue-in-cheek>

I would rather like to see how all the fork()ing and |'ing can be done 
with MinGW32.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 19:51                                     ` Linus Torvalds
@ 2006-02-23 20:31                                       ` Sam Vilain
  2006-02-24  6:43                                         ` Linus Torvalds
  0 siblings, 1 reply; 78+ messages in thread
From: Sam Vilain @ 2006-02-23 20:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git

Linus Torvalds wrote:
>>>There are many portable interpreters out there, and I don't mean perl. And 
>>>writing a small "specialized for git" one isn't even that hard. In fact, 
>>>most of the shell (and bash) hackery we do now would be unnecessary if we 
>>>just made a small "git interpreter" that ran "git scripts".
>>Before anybody mentions tcl ;-).
> Well, I was thinking more of the "embeddable" ones - things that are so 
> small that they can be compiled with the project. Things like Lua.
   [...]
> I was really thinking more of a simple shell-like script interpreter. 

I like the term "Domain Specific Language" to refer to this sort of 
thing.  It even hints at using the right kind of tools to achieve it, too :)

Sam.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 17:13                                 ` Linus Torvalds
  2006-02-23 19:32                                   ` Junio C Hamano
@ 2006-02-23 21:43                                   ` Alex Riesen
  1 sibling, 0 replies; 78+ messages in thread
From: Alex Riesen @ 2006-02-23 21:43 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andreas Ericsson, Junio C Hamano, git

Linus Torvalds, Thu, Feb 23, 2006 18:13:43 +0100:
> > Someday we'll have to start dropping features on Windows or restrict them
> > beyond their usefullness.
> 
> One thing that would help a bit would be to avoid shell.
> 
> There are many portable interpreters out there, and I don't mean perl. And 
> writing a small "specialized for git" one isn't even that hard. In fact, 
> most of the shell (and bash) hackery we do now would be unnecessary if we 
> just made a small "git interpreter" that ran "git scripts".
> 
> The fact that it would also help portability is just an added advantage.
> 

I actually was dreaming about taking a vacation and rewrite at least
the most important scripts in C, but without cygwin. Implement the
needed subset of POSIX in compat/, workaround fork.

That'd help me to present git to my collegues without requiring them
to install cygwin, perl and python first. It is a real problem to
explain why a new tool is better than the old one if the problem start
right from installation, and it probably wont matter how bad the old
tool is (it is, they know that too, but it has windows, doors and a
mostly running man for busy-waiting cursor).

A gits own interpreter would be more than, of course.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 20:31                                       ` Sam Vilain
@ 2006-02-24  6:43                                         ` Linus Torvalds
  0 siblings, 0 replies; 78+ messages in thread
From: Linus Torvalds @ 2006-02-24  6:43 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Junio C Hamano, git



On Fri, 24 Feb 2006, Sam Vilain wrote:
> 
> I like the term "Domain Specific Language" to refer to this sort of thing.  It
> even hints at using the right kind of tools to achieve it, too :)

Just for fun, I wrote a first cut at a script engine for passing pipes 
around.

It's designed so that the "fork+exec with a pipe" should be easily 
replaced by "spawn with a socket" if that's what the target wants, but 
it also has some rather strange syntax, so I'm in no way claiming that 
this is a sane approach.

It was fun to write, though. You can already do some strange things with 
it, like writing a script like this

	set @ --since=2.months.ago Makefile
	exec git-rev-parse --default HEAD $@
		stdout arguments
	exec git-rev-list $arguments
		stdout revlist
	exec git-diff-tree --pretty --stdin
		stdin revlist
		stdout diff-tree-output
	exec less -S
		stdin diff-tree-output

which kind of shows the idea (it sets the "@" variable by hand, because 
the silly "git-script" thing doesn't set it itself).

I'm not sure this is worth pursuing (it really is a very strange kind of 
script syntax), but it was amusing to do. 

No docs - if you want to know how it works, you'll just have to read the 
equally strange sources.

		Linus

----
diff-tree 3e7dbcaae63278ccd413d93ecf9cba65a0d07021 (from d27d5b3c5b97ca30dfc5c448dc8cdae914131051)
Author: Linus Torvalds <torvalds@osdl.org>
Date:   Thu Feb 23 22:06:12 2006 -0800

    Add really strange script engine

diff --git a/Makefile b/Makefile
index 0c04882..247030b 100644
--- a/Makefile
+++ b/Makefile
@@ -164,7 +164,7 @@ PROGRAMS = \
 	git-upload-pack$X git-verify-pack$X git-write-tree$X \
 	git-update-ref$X git-symbolic-ref$X git-check-ref-format$X \
 	git-name-rev$X git-pack-redundant$X git-repo-config$X git-var$X \
-	git-describe$X git-merge-tree$X
+	git-describe$X git-merge-tree$X git-script$X
 
 # what 'all' will build and 'install' will install, in gitexecdir
 ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS)
@@ -204,7 +204,7 @@ LIB_OBJS = \
 	quote.o read-cache.o refs.o run-command.o \
 	server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
 	tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
-	fetch-clone.o \
+	fetch-clone.o execute.o \
 	$(DIFF_OBJS)
 
 LIBS = $(LIB_FILE)
diff --git a/cache.h b/cache.h
index 5020f07..e4e66ce 100644
--- a/cache.h
+++ b/cache.h
@@ -352,4 +352,7 @@ extern int copy_fd(int ifd, int ofd);
 extern int receive_unpack_pack(int fd[2], const char *me, int quiet);
 extern int receive_keep_pack(int fd[2], const char *me, int quiet);
 
+/* script execution engine.. */
+extern int execute(const char *name, char *buf, unsigned int size);
+
 #endif /* CACHE_H */
diff --git a/execute.c b/execute.c
new file mode 100644
index 0000000..abb6801
--- /dev/null
+++ b/execute.c
@@ -0,0 +1,622 @@
+/*
+ * Stupid git script execution engine
+ *
+ * Copyrigt (C) 2006, Linus Torvalds
+ *
+ * There's one rule here: only ever expand a single level of variables.
+ * In particular - we never expand as a string, and keep everything as
+ * a list of entries. Always.
+ *
+ * This avoids all issues with quoting etc, since it's never an issue.
+ * When we execute a program, we have a list of arguments, no quoting
+ * or string parsing involved.
+ */
+#include "cache.h"
+#include <sys/wait.h>
+
+enum vartype {
+	var_none,
+	var_fd,
+	var_array
+};
+
+struct argument {
+	enum vartype type;
+	int fd, members, allocs, error;
+	const char **array;
+};
+
+struct variable {
+	const char *name;
+	struct variable *next;
+	struct argument value;
+};
+
+struct cmd_struct {
+	const char *line;
+	unsigned int len;
+	struct cmd_struct *subcmd;
+	struct cmd_struct *next;
+};
+
+struct parse_buf {
+	const char *name;
+	const char *error;
+	char *prog;
+	unsigned int size;
+	unsigned int offset;
+	unsigned int line;
+	unsigned int linestart;
+};
+
+static struct variable *vars = NULL;
+static void run_program(struct cmd_struct *cmd);
+
+static int countline(struct parse_buf *buf)
+{
+	int count = 0;
+	unsigned offset;
+
+	for (offset = buf->offset; offset < buf->size; offset++) {
+		unsigned char c = buf->prog[offset];
+		switch (c) {
+		case '\n':
+			buf->line++;
+		/* fallthrough */
+		case '\r':
+			count = 0;
+			buf->offset = offset + 1;
+			buf->prog[offset] = 0;
+			continue;
+		case ' ':
+			count++;
+			continue;
+		case '\t':
+			count = (count + 8) & ~7;
+			continue;
+		default:
+			buf->linestart = offset;
+			return count;
+		}
+	}
+	buf->offset = offset;
+	return -2;
+}
+
+/*
+ * When this is called, we've already done the indentation check,
+ * and "buf->linestart" points to the actual start of the command.
+ */
+static struct cmd_struct *parse_one_line(struct parse_buf *buf)
+{
+	unsigned int offset;
+	struct cmd_struct *cmd = xmalloc(sizeof(*cmd));
+	memset(cmd, 0, sizeof(*cmd));
+
+	offset = buf->linestart;
+	cmd->line = buf->prog + offset;
+	for ( ; offset < buf->size; offset++) {
+		unsigned char c = buf->prog[offset];
+		switch (c) {
+		case '\n':
+			buf->prog[offset++] = 0;
+			buf->line++;
+			break;
+		default:
+			continue;
+		}
+		break;
+	}
+	buf->offset = offset;
+	return cmd;
+}
+
+static struct cmd_struct *parse(struct parse_buf *buf, int indent)
+{
+	struct cmd_struct *first = NULL, *last = NULL;
+
+	for (;;) {
+		struct cmd_struct *now;
+		int newindent = countline(buf);
+
+		if (newindent < indent)
+			break;
+		if (!first)
+			indent = newindent;
+		if (newindent > indent) {
+			struct cmd_struct *subcmd;
+			if (last->subcmd) {
+				buf->error = "bad indentation";
+				return NULL;
+			}
+			subcmd = parse(buf, newindent);
+			if (!subcmd)
+				return NULL;
+			last->subcmd = subcmd;
+			continue;
+		}
+		now = parse_one_line(buf);
+		if (!now)
+			return NULL;
+		if (last)
+			last->next = now;
+		else
+			first = now;
+		last = now;
+	}
+	return first;
+}
+
+static struct cmd_struct *exec_bad(struct cmd_struct *cmd, struct argument *arg)
+{
+	printf("unrecognized command: '%s'\n", cmd->line);
+	return NULL;
+}
+
+static struct cmd_struct *exec_echo(struct cmd_struct *cmd, struct argument *arg)
+{
+	int i;
+	for (i = 0; i < arg->members; i++)
+		printf("%s%c", arg->array[i], i == arg->members-1 ? '\n': ' ');
+	return cmd->next;
+}
+
+static struct variable *find_variable(const char *name)
+{
+	struct variable *var = vars;
+	while (var) {
+		if (!strcmp(var->name, name))
+			return var;
+		var = var->next;
+	}
+	return NULL;
+}
+
+static struct variable *create_variable(const char *name)
+{
+	struct variable *var = find_variable(name);
+
+	if (!var) {
+		var = xmalloc(sizeof(*var));
+		memset(var, 0, sizeof(*var));
+		var->name = name;
+		var->next = vars;
+		vars = var;
+	}
+	return var;
+}
+
+static struct cmd_struct *exec_set(struct cmd_struct *cmd, struct argument *arg)
+{
+	int count = arg->members;
+	struct variable *var;
+	const char *name;
+	unsigned size;
+
+	if (!count)
+		return cmd->next;
+	name = arg->array[0];
+	var = create_variable(arg->array[0]);
+
+	var->value.members = count-1;
+	size = count * sizeof(var->value.array[0]);
+	var->value.array = xmalloc(size);
+	memcpy(var->value.array, arg->array+1, size);
+
+	return cmd->next;
+}
+
+static void free_arg_list(struct argument *arg)
+{
+	/*
+	 * We can't free the actual entries, since we re-use them
+	 * on expansion. Right or wrong, that's how it is...
+	 */
+	free(arg->array);
+}
+
+static void drop_variable(struct variable *var)
+{
+	free_arg_list(&var->value);
+	free(var);
+}
+
+static struct cmd_struct *exec_unset(struct cmd_struct *cmd, struct argument *arg)
+{
+	int i;
+
+	for (i = 0; i < arg->members; i++) {
+		const char *name = arg->array[i];
+		struct variable *var, **p = &vars;
+
+		while ((var = *p) != NULL) {
+			if (!strcmp(var->name, name)) {
+				*p = var->next;
+				drop_variable(var);
+				break;
+			}
+			p = &var->next;
+		}
+	}
+	return cmd->next;
+}
+
+static struct cmd_struct *exec_exit(struct cmd_struct *cmd, struct argument *arg)
+{
+	int value = 0;
+	if (arg->members)
+		value = atoi(arg->array[0]);
+	exit(value);
+}
+
+static struct cmd_struct *exec_else(struct cmd_struct *cmd, struct argument *arg)
+{
+	return cmd->next;
+}
+
+static struct cmd_struct *exec_if(struct cmd_struct *cmd, struct argument *arg)
+{
+	struct cmd_struct *pos, *neg;
+
+	pos = cmd->subcmd;
+	neg = cmd->next;
+	if (neg) {
+		if (!strncmp(neg->line, "else", 4))
+			neg = neg->subcmd;
+		else
+			neg = NULL;
+	}
+	if (!arg->members)
+		pos = neg;
+	run_program(pos);
+	return cmd->next;
+}
+
+static int match_cmd(const char *match, struct cmd_struct *cmd)
+{
+	int len = strlen(match), cmdlen = strlen(cmd->line);
+	if (cmdlen < len)
+		return 0;
+	if (cmdlen > len && !isspace(cmd->line[len]))
+		return 0;
+	return !memcmp(match, cmd->line, len);
+}
+
+static int set_input(int *redirect, const char *val)
+{
+	struct variable *var;
+
+	while (isspace(*val))
+		val++;
+	var = find_variable(val);
+	if (!var || var->value.type != var_fd)
+		die("bad 'fd' variable %s", val);
+
+	*redirect = var->value.fd;
+	var->value.fd = -1;
+	return 0;
+}
+
+static int set_output(int *redirect, const char *val)
+{
+	int fd[2];
+	struct variable *var;
+
+	while (isspace(*val))
+		val++;
+	var = create_variable(val);
+
+	if (pipe(fd) < 0)
+		die("unable to pipe");
+	var->value.type = var_fd;
+	var->value.fd = fd[0];
+	*redirect = fd[1];
+	return 0;
+}
+
+/*
+ * Only these routines should need to be ported to a "spawn()" interface
+ */
+static struct cmd_struct *exec_exec(struct cmd_struct *cmd, struct argument *arg)
+{
+	int redirect[3];
+	pid_t pid;
+	int nr = arg->members;
+	struct cmd_struct *io;
+
+	if (!nr) {
+		run_program(cmd->subcmd);
+		return cmd->next;
+	}
+
+	memset(redirect, 0, sizeof(redirect));
+	for (io = cmd->subcmd; io ; io = io->next) {
+		if (match_cmd("stdin", io)) {
+			set_input(redirect+0, io->line + 5);
+			continue;
+		}
+		if (match_cmd("stdout", io)) {
+			set_output(redirect+1, io->line + 6);
+			continue;
+		}
+		if (match_cmd("stderr", io)) {
+			set_output(redirect+2, io->line + 6);
+			continue;
+		}
+	}
+
+	/*
+	 * HERE! Use spawn if necessary - the fd redirect table has been set up
+	 */
+	pid = vfork();
+	if (pid < 0) {
+		error("vfork failed (%s)", strerror(errno));
+		return NULL;
+	}
+
+	if (!pid) {
+		int retval;
+		if (redirect[0]) {
+			dup2(redirect[0], 0);
+			close(redirect[0]);
+		}
+		if (redirect[1]) {
+			dup2(redirect[1], 1);
+			close(redirect[1]);
+		}
+		if (redirect[2]) {
+			dup2(redirect[2], 2);
+			close(redirect[2]);
+		}
+		retval = execvp(arg->array[0], (char *const*) arg->array);
+		exit(255);
+	}
+
+	if (redirect[0])
+		close(redirect[0]);
+	if (redirect[1])
+		close(redirect[1]);
+	if (redirect[2])
+		close(redirect[2]);
+
+	/*
+	 * If we don't have anybody waiting for output,
+	 * wait for it
+	 */
+	if (!redirect[1]) {
+		int status;
+		while (waitpid(pid, &status, 0) < 0) {
+			if (errno == EINTR)
+				continue;
+			error("unable to wait for child (%s)", strerror(errno));
+			return NULL;
+		}
+		/* FIXME! Put exit status in a variable! */
+	}
+	run_program(cmd->subcmd);
+	return cmd->next;
+}
+
+static struct cmd_struct *exec_nop(struct cmd_struct *cmd, struct argument *arg)
+{
+	return cmd->next;
+}
+
+static const struct cmd_def {
+	const char *n;
+	int len;
+	struct cmd_struct *(*exec)(struct cmd_struct *, struct argument *);
+} cmds[] = {
+	{ "bad", 0, exec_bad },
+	{ "set", 3, exec_set },
+	{ "unset", 5, exec_unset },
+	{ "echo", 4, exec_echo },
+	{ "exit", 4, exec_exit },
+	{ "if", 2, exec_if },
+	{ "else", 4, exec_else },
+	{ "exec", 4, exec_exec },
+	{ "stdin", 5, exec_nop },
+	{ "stdout", 6, exec_nop },
+	{ "stderr", 6, exec_nop },
+};
+
+static void add_argument(struct argument *arg, const char *n)
+{
+	int allocs = arg->allocs, members = arg->members;
+
+	if (members+1 >= allocs) {
+		allocs = (allocs * 3) / 2 + 32;
+		arg->array = xrealloc(arg->array, allocs*sizeof(arg->array[0]));
+		arg->allocs = allocs;
+	}
+	arg->array[members++] = n;
+	arg->array[members] = NULL;
+	arg->members = members;
+}
+
+static int get_word(const char *line, const char **res)
+{
+	int quoted = 0;
+	int offset = 0;
+	int stop = 0;
+	char *buf;
+
+	for (;;) {
+		unsigned char c = line[offset];
+		if (!c)
+			break;
+		offset++;
+		if (c == '\\') {
+			quoted ^= 1;
+			continue;
+		}
+		if (quoted) {
+			quoted = 0;
+			continue;
+		}
+		if (stop) {
+			if (c == stop)
+				stop = 0;
+			continue;
+		}
+		if (c == '\'' || c == '"') {
+			stop = c;
+			continue;
+		}
+		if (!isspace(c)) {
+			continue;
+		}
+		offset--;
+		break;
+	}
+	if (quoted || stop)
+		return -1;
+	buf = xmalloc(offset+1);
+	memcpy(buf, line, offset);
+	buf[offset] = 0;
+	*res = buf;
+	return offset;
+}
+
+static int expand_word(const char *line, struct argument *arg)
+{
+	const char *word;
+	int offset = get_word(line, &word);
+
+	if (offset > 0)
+		add_argument(arg, word);
+	return offset;
+}
+
+static void convert_fd_into_array(struct variable *var)
+{
+	int fd = var->value.fd;
+	char buffer[8192];
+	int len, offset, last;
+
+	var->value.fd = -1;
+	var->value.type = var_array;
+	len = 0;
+	for (;;) {
+		int ret = read(fd, buffer + len, sizeof(buffer) - len);
+		if (!ret)
+			break;
+		if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			break;
+		}
+		len += ret;
+		if (len >= sizeof(buffer))
+			break;
+	}
+
+	last = 0;
+	for (offset = 0; offset < len; offset++) {
+		unsigned char c = buffer[offset];
+		if (c == '\n') {
+			buffer[offset] = 0;
+			add_argument(&var->value, buffer+last);
+			last = offset+1;
+			continue;
+		}
+	}		
+}
+
+static int expand_variable(const char *line, struct argument *arg)
+{
+	const char *word;
+	int offset = get_word(line+1, &word);
+
+	if (offset > 0) {
+		struct variable *var = find_variable(word);
+		offset++;	/* The '$' character itself */
+		if (var) {
+			int i;
+			if (var->value.type == var_fd)
+				convert_fd_into_array(var);
+			for (i = 0; i < var->value.members; i++)
+				add_argument(arg, var->value.array[i]);
+		}
+	}
+	return offset;
+}
+
+static int expand_value(const char *line, struct argument *arg)
+{
+	unsigned char c = *line;
+
+	switch (c) {
+	case '$':
+		return expand_variable(line, arg);
+	default:
+		return expand_word(line, arg);
+	}
+}
+
+static struct argument *expand_line(const char *line)
+{
+	struct argument *arg;
+
+	arg = xmalloc(sizeof(*arg));
+	memset(arg, 0, sizeof(*arg));
+	arg->type = var_array;
+	for (;;) {
+		int n;
+		while (isspace(*line)) {
+			line++;
+		}
+		if (!*line)
+			break;
+		n = expand_value(line, arg);
+		if (n <= 0)
+			break;
+		line += n;
+	}
+	return arg;
+}
+
+static void run_program(struct cmd_struct *cmd)
+{
+	while (cmd) {
+		int i;
+		const struct cmd_def *run = cmds+0;
+		struct argument *arg = NULL;
+		int cmdlen = strlen(cmd->line);
+
+		for (i = 1; i < sizeof(cmds)/sizeof(cmds[0]); i++) {
+			const struct cmd_def *def = cmds + i;
+			int len = def->len;
+			if (len > cmdlen)
+				continue;
+			if (len < cmdlen && !isspace(cmd->line[len]))
+				continue;
+			if (memcmp(cmd->line, def->n, len))
+				continue;
+			run = def;
+			arg = expand_line(cmd->line + len);
+			break;
+		}
+		cmd = run->exec(cmd, arg);
+	}
+}
+
+int execute(const char *name, char *buf, unsigned int size)
+{
+	struct parse_buf p;
+	struct cmd_struct *program;
+
+	p.name = name;
+	p.prog = buf;
+	p.size = size;
+	p.offset = 0;
+	p.line = 1;
+	p.error = "empty program";
+
+	program = parse(&p, -1);
+	if (!program || p.offset != p.size)
+		die("parse error at %s:%d: %s", p.name, p.line, p.error);
+
+	run_program(program);
+	return 0;
+}
diff --git a/script.c b/script.c
new file mode 100644
index 0000000..ae85598
--- /dev/null
+++ b/script.c
@@ -0,0 +1,58 @@
+/*
+ * Silly git script language
+ *
+ * Copyright (C) 2006, Linus Torvalds
+ */
+#include "cache.h"
+
+static const char script_usage[] = "git-script <scriptfile>";
+
+int main(int argc, char **argv)
+{
+	int fd;
+	char *buf;
+	const char *filename;
+	unsigned int size, alloc;
+
+	fd = 0;
+	switch (argc) {
+	case 1:
+		filename = "stdin";
+		fd = dup(0);
+		close(0);
+		open("/dev/null", O_RDONLY);
+		break;
+	case 2:
+		filename = argv[1];
+		fd = open(filename, O_RDONLY);
+		if (fd < 0)
+			die("unable to open '%s': %s", filename, strerror(errno));
+		break;
+	default:
+		usage(script_usage);
+	}
+
+	buf = NULL;
+	alloc = 0;
+	size = 0;
+	for (;;) {
+		int nr;
+		if (size >= alloc) {
+			alloc = (alloc * 3) / 2 + 8192;
+			buf = xrealloc(buf, alloc);
+		}
+		nr = read(fd, buf + size, alloc - size);
+		if (!nr)
+			break;
+		if (nr < 0) {
+			if (errno == EAGAIN || errno == EINTR)
+				continue;
+			die("script read failed (%s)", strerror(errno));
+		}
+		size += nr;
+	}
+	close(fd);
+
+	execute(filename, buf, size);
+	return 0;
+}

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-22 16:35             ` Alex Riesen
                                 ` (2 preceding siblings ...)
  2006-02-22 22:00               ` Johannes Schindelin
@ 2006-02-24 12:02               ` Eric Wong
  2006-02-24 13:44                 ` Johannes Schindelin
  3 siblings, 1 reply; 78+ messages in thread
From: Eric Wong @ 2006-02-24 12:02 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Sam Vilain, Junio C Hamano, Johannes Schindelin, git

Alex Riesen <raa.lkml@gmail.com> wrote:
> On 2/21/06, Sam Vilain <sam@vilain.net> wrote:
> > Alex Riesen wrote:
> > >>>Does not work here (ActiveState Build 811, Perl 5.8.6):
> > >>>$ perl -e 'open(F, "-|")'
> > >>>'-' is not recognized as an internal or external command,
> > >>>operable program or batch file.
> > >>Portability, Ease of Coding, Few CPAN Module Dependencies.  Pick any two.
> > > Sometimes an upgrade is just out of question. Besides, that'd mean an
> > > upgrade to another operating system, because very important scripts
> > > over here a just not portable to anything else but
> > >     "ActiveState Perl on Windows (TM)"
> > > I just have no choice.
> >
> > Sure, but perhaps IPC::Open2 or some other CPAN module has solved this
> > problem already.
> 
> IPC::Open2 works! Well "kind of": there are still strange segfaults regarding
> stack sometimes. And I don't know yet whether and how the arguments are escaped
> (Windows has no argument array. It has that bloody stupid one-line command line)

It seems that ActiveState has more problems with pipes than it does with fork.
If it supports redirects reasonably well, this avoids pipes entirely and
may be more stable as a result (but possibly slower):

# IO::File is standard in Perl 5.x, new_tmpfile
# returns an open filehandle to an already unlinked file

use IO::File;
my $out = IO::File->new_tmpfile;
file
my $pid = fork;
defined $pid or die $!;
if (!$pid) {
	# redirects STDOUT to $out file
	open STDOUT, '>&', $out or die $!;
	exec('foo','bar');
}
waitpid $pid, 0;
seek $out, 0, 0;
while (<$out>) {
	...
}

Writing and reading from a tempfile are very fast for me in Linux, and probably
not much slower than pipes.  Of course I'm still assuming file descriptors stay
shared after a 'fork', which may be asking too much on Windows.  Using something
from File::Temp to get a temp filename would still work.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-24 12:02               ` Eric Wong
@ 2006-02-24 13:44                 ` Johannes Schindelin
  2006-02-24 16:14                   ` Linus Torvalds
  0 siblings, 1 reply; 78+ messages in thread
From: Johannes Schindelin @ 2006-02-24 13:44 UTC (permalink / raw)
  To: Eric Wong; +Cc: Alex Riesen, Sam Vilain, Junio C Hamano, git

Hi,

On Fri, 24 Feb 2006, Eric Wong wrote:

> Writing and reading from a tempfile are very fast for me in Linux, and 
> probably not much slower than pipes.

Sorry, but no. Really no. Pipes have several advantages over temporary 
files:

- The second program can already work on the data before the first 
  finishes.
- Most simple temp file handling has security issues.
- You need write access.

Hth,
Dscho

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-24 13:44                 ` Johannes Schindelin
@ 2006-02-24 16:14                   ` Linus Torvalds
  0 siblings, 0 replies; 78+ messages in thread
From: Linus Torvalds @ 2006-02-24 16:14 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Eric Wong, Alex Riesen, Sam Vilain, Junio C Hamano, git



On Fri, 24 Feb 2006, Johannes Schindelin wrote:
> 
> Sorry, but no. Really no. Pipes have several advantages over temporary 
> files:
> 
> - The second program can already work on the data before the first 
>   finishes.

This really is a _huge_ issue in general, although probably not a very 
big one in this case.

This is what I talked about when I said "streaming" data. Look at the 
difference between

	git whatchanged -s drivers/usb

and

	git log drivers/usb

in the kernel repo. They give almost the same output, but...

Notice how one starts _immediately_, while the other starts after a few 
seconds (or, if you have a slow machine, and an unpacked archive, after 
tens of seconds or longer).

And the reason is that "git log" uses "git-rev-list" with a path limiter, 
and currently that ends up having to walk basically the whole history in 
order to generate a minimal graph.

In contrast, "git-whatchanged" uses "git-diff-tree" to limit the output, 
and git-diff-tree doesn't care about "minimal graph" or crud like that: it 
just cares about discarding any local commits that aren't interesting. It 
doesn't need to worry about updating parent chains etc, so it can do it 
all incrementally - and can thus start output as soon as it gets anything 
at all.

Now, maybe you think that "a few seconds" isn't a big deal. Sure, it's 
actually fast as hell, considering what it is doing, and anybody should be 
really really impressed that we can do that at all.

But (a) it _is_ a huge deal. Responsiveness is really important. And 
worse: (b) it scales badly with repository size. Creating the whole 
data-set before starting to output it really doesn't scale.

Now, I have ways to make "git-rev-list" better. It doesn't really need to 
walk the _whole_ history for its path limiting before it can start 
outputting stuff: it really _could_ do things more incrementally. However, 
it's a real bitch sometimes to work with incremental data when you don't 
know everything, so it gets a lot more complicated. 

So my point isn't that "git log drivers/usb" will get less and less 
responsive over time. I can fix that - eventually. My point is that in 
order to make it more responsive, I need to make it less synchronous. More 
"streaming". 

And that is where a pipe is so much better than a file. It's very 
fundamentally a streaming interface.

However, I suspect some of these issues are non-issues for the perl 
programs that work with a few entries at a time.

		Linus

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 14:07                               ` Alex Riesen
  2006-02-23 14:22                                 ` Andreas Ericsson
  2006-02-23 17:13                                 ` Linus Torvalds
@ 2006-02-26 19:55                                 ` Christopher Faylor
  2006-02-26 20:18                                   ` Linus Torvalds
                                                     ` (2 more replies)
  2 siblings, 3 replies; 78+ messages in thread
From: Christopher Faylor @ 2006-02-26 19:55 UTC (permalink / raw)
  To: git

On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
>On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
>>>>Not to be unhelpful or anything, but activestate perl seems to be quite
>>>>a lot of bother.  Is it worth supporting it?
>>>
>>>
>>>It's not activestate perl actually.  It's only one platform it also
>>>_has_ to support.  Is it worth supporting Windows?
>>
>>With or without cygwin?  With cygwin, I'd say "yes, unless it makes
>>things terribly difficult to maintain and so long as we don't take
>>performance hits on unices".  Without cygwin, I'd say "What?  It runs
>>on windows?".
>
>There not much difference with or without cygwin.  The penalties of
>doing any kind of support for it will pile up (as they started to do
>with pipes).  Someday we'll have to start dropping features on Windows
>or restrict them beyond their usefullness.  The fork emulation in
>cygwin isn't perfect,

If the speed of cygwin's fork is an issue then I'd previously suggested
using spawn*.  The spawn family of functions were designed to emulate
Windows functions of the same name.  They start a new process without
the requirement of forking.

>signals do not work reliably (if at all),

I'm not sure if you're mixing cygwin with windows here but if signals do
not work reliably in Cygwin then that is something that we'd like to
know about.  Signals *obviously* have to work fairly well for programs
like ssh, bash, and X to work, however.

Native Windows, OTOH, hardly has any signals at all and deals with
signals in a way that is only vaguely like linux.

>filesystem is slow and locked down, and exec-attribute is NOT really
>useful even on NTFS (it is somehow related to execute permission and
>open files.  I still cannot figure out how exactly are they related).

Again, it's not clear if you're talking about Windows or Cygwin but
under Cygwin, in the default configuration, the exec attribute means the
same thing to cygwin as it does to linux.

As always, if you have questions or problems with cygwin, you can ask in
the proper forum.  The available cygwin mailing lists are here:
http://cygwin.com/lists.html.

Would getting git into the cygwin distribution solve any problems with
git adoption on Windows?  This would get an automatic green light from
anyone who was interested, if so.  Someone would just have to send an
"ITP" (Intent To Package) to the cygwin-apps mailing list and provide a
package using the guidelines here: http://cygwin.com/setup.html .

cgf
--
Christopher Faylor			spammer? ->	aaaspam@sourceware.org
Cygwin Co-Project Leader
TimeSys, Inc.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-26 19:55                                 ` Christopher Faylor
@ 2006-02-26 20:18                                   ` Linus Torvalds
  2006-02-26 20:40                                     ` Christopher Faylor
  2006-02-26 23:17                                   ` NT directory traversal speed on 25K files on Cygwin Rutger Nijlunsing
  2006-03-02 14:10                                   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Alex Riesen
  2 siblings, 1 reply; 78+ messages in thread
From: Linus Torvalds @ 2006-02-26 20:18 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git



On Sun, 26 Feb 2006, Christopher Faylor wrote:
> 
> If the speed of cygwin's fork is an issue then I'd previously suggested
> using spawn*.  The spawn family of functions were designed to emulate
> Windows functions of the same name.  They start a new process without
> the requirement of forking.

I thought that cygwin didn't implement the posix_spawn*() family?

Anyway, we probably _can_ use posix_spawn() in various places, and 
especially if that helps windows performance, we should.

		Linus

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-23 13:29                             ` Andreas Ericsson
  2006-02-23 14:07                               ` Alex Riesen
@ 2006-02-26 20:33                               ` Christopher Faylor
  1 sibling, 0 replies; 78+ messages in thread
From: Christopher Faylor @ 2006-02-26 20:33 UTC (permalink / raw)
  To: git

On Thu, Feb 23, 2006 at 02:29:48PM +0100, Andreas Ericsson wrote:
>Alex Riesen wrote:
>>On 2/23/06, Andreas Ericsson <ae@op5.se> wrote:
>>
>>>Not to be unhelpful or anything, but activestate perl seems to be quite
>>>a lot of bother. Is it worth supporting it?
>>
>>
>>It's not activestate perl actually. It's only one platform it also
>>_has_ to support.
>>Is it worth supporting Windows?
>
>
>With or without cygwin? With cygwin, I'd say "yes, unless it makes 
>things terribly difficult to maintain and so long as we don't take 
>performance hits on unices". Without cygwin, I'd say "What? It runs on 
>windows?".
>
>If we claim to support windows but do a poor job of it, no-one else will 
>start working on a windows-port. If we don't claim to support windows 
>but say that "it's known to work with cygwin, although be aware of these 
>performance penalties...", eventually someone will come along with their 
>shiny Visual Express and hack up support for it, even if some tools will 
>be missing and others unnecessarily complicated.

Well, with Cygwin, you've at least got the ear of one of the Cygwin
maintainers, which should be worth something.

Even if I disappear, you can always send concerns to the Cygwin mailing
list.  Do the ActiveState folks respond to complaints about things as
basic as pipes not working in perl?

Cygwin's goal is to make Windows look as much like Linux as we can
manage, so, unless we're total incompetents (which has been hinted in
this mailing list from time to time), it has *got* to be better,
source-code-wise to target Windows-running-Cygwin than
just-plain-Windows.  However, as has been noted, that means that there
will be a speed tradeoff.

I think that, for most projects, the convenience of not having to
clutter the code with substantial accommodations for the windows/POSIX
mismatch usually offsets the annoyance of the speed penalty.  Maybe
that's not the case for git, however.

Anyway, we're willing, within the limits of available time, to help out
where git uncovers issues with Cygwin.  I just fixed some stuff in
dirent.h in the last Cygwin release, as a direct result of people noting
a problem here.  Basically, I don't want git to be a morasse of #ifdef
__CYGWIN_'s and I'll do whatever I can to help.

We're always trying to tweak things to improve speed in Cygwin and am
open to intelligent suggestions about how we can make things better.
The dance between total linux compatibility and speed is one that we
struggle with all of the time and, sadly, over time, we've probably
sacrificed speed in the name of functionality.  That's probably because
it's easy to fix a problem like "close-on-exec doesn't work for sockets"
and feel good that you've fixed a bug even if you've just added a few
microseconds to fork/exec.

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-26 20:18                                   ` Linus Torvalds
@ 2006-02-26 20:40                                     ` Christopher Faylor
  2006-03-02 14:18                                       ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Christopher Faylor @ 2006-02-26 20:40 UTC (permalink / raw)
  To: git

On Sun, Feb 26, 2006 at 12:18:19PM -0800, Linus Torvalds wrote:
>On Sun, 26 Feb 2006, Christopher Faylor wrote:
>>If the speed of cygwin's fork is an issue then I'd previously suggested
>>using spawn*.  The spawn family of functions were designed to emulate
>>Windows functions of the same name.  They start a new process without
>>the requirement of forking.
>
>I thought that cygwin didn't implement the posix_spawn*() family?

Right.  It just implements the windows version of spawn.  I looked more
closely at the posix_spawn functions after you last suggested it and,
while it would be possible to implement this in cygwin, these functions
are a lot more heavyweight than the windows-like implementation of spawn
that are already in cygwin.  So, they would come with their own
performance penalty.

The cygwin/windows version of spawn is basically like an extended version
of exec*():

pid = spawnlp (P_NOWAIT, "/bin/ls", "ls", "-l", NULL);

will start "/bin/ls" and return a pid which can be used in waitpid.
There is still some overhead to this function but it basically is just a
wrapper around the Windows CreateProcess, which means that it doesn't
go through the annoying overhead of Cygwin's fork.

The posix_spawn stuff is in my todo list but the Windows spawn stuff
could be used now.

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* NT directory traversal speed on 25K files on Cygwin
  2006-02-26 19:55                                 ` Christopher Faylor
  2006-02-26 20:18                                   ` Linus Torvalds
@ 2006-02-26 23:17                                   ` Rutger Nijlunsing
  2006-02-27  1:18                                     ` Christopher Faylor
  2006-02-27  9:19                                     ` Andreas Ericsson
  2006-03-02 14:10                                   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Alex Riesen
  2 siblings, 2 replies; 78+ messages in thread
From: Rutger Nijlunsing @ 2006-02-26 23:17 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
> On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
> >filesystem is slow and locked down, and exec-attribute is NOT really
> >useful even on NTFS (it is somehow related to execute permission and
> >open files.  I still cannot figure out how exactly are they related).
> 
> Again, it's not clear if you're talking about Windows or Cygwin but
> under Cygwin, in the default configuration, the exec attribute means the
> same thing to cygwin as it does to linux.

I don't know about native Windows speed, but comparing NutCracker with
Cygwin on a simple 'find . | wc -l' already gives a clue that looking
at Cygwin to benchmark NT file inspection IO will give a skewed
picture:

##### NutCracker
$ time find . | wc -l

real    0m 1.44s
user    0m 0.45s
sys     0m 0.98s
  25794

##### Cygwin
$ time c:\\cygwin\\bin\\find . | wc -l

real    0m 6.72s
user    0m 1.09s
sys     0m 5.59s
  25794

##### CMD.EXE + DIR /S
C:\PROJECT> c:\cygwin\bin\time cmd /c dir /s >NUL
0.01user 0.01system 0:05.70elapsed 0%CPU (0avgtext+0avgdata 6320maxresident)k
0inputs+0outputs (395major+0minor)pagefaults 0swaps

##### Cygwin 'find -ls' (NutCracker doesn't have a '-ls')
C:\PROJECT> c:\cygwin\bin\time c:\cygwin\bin\find -ls | wc -l
2.79user 7.81system 0:10.60elapsed 100%CPU (0avgtext+0avgdata 14480maxresident)k
  25794


Regards,
Rutger.

-- 
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-26 23:17                                   ` NT directory traversal speed on 25K files on Cygwin Rutger Nijlunsing
@ 2006-02-27  1:18                                     ` Christopher Faylor
  2006-02-27 18:30                                       ` Rutger Nijlunsing
  2006-02-27  9:19                                     ` Andreas Ericsson
  1 sibling, 1 reply; 78+ messages in thread
From: Christopher Faylor @ 2006-02-27  1:18 UTC (permalink / raw)
  To: git

On Mon, Feb 27, 2006 at 12:17:01AM +0100, Rutger Nijlunsing wrote:
>On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
>>On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
>>>filesystem is slow and locked down, and exec-attribute is NOT really
>>>useful even on NTFS (it is somehow related to execute permission and
>>>open files.  I still cannot figure out how exactly are they related).
>>
>>Again, it's not clear if you're talking about Windows or Cygwin but
>>under Cygwin, in the default configuration, the exec attribute means
>>the same thing to cygwin as it does to linux.
>
>I don't know about native Windows speed, but comparing NutCracker with
>Cygwin on a simple 'find .  | wc -l' already gives a clue that looking
>at Cygwin to benchmark NT file inspection IO will give a skewed
>picture:
>
>##### NutCracker $ time find .  | wc -l
>
>real    0m 1.44s
>user    0m 0.45s
>sys     0m 0.98s
>25794
>
>##### Cygwin $ time c:\\cygwin\\bin\\find .  | wc -l
>
>real    0m 6.72s
>user    0m 1.09s
>sys     0m 5.59s
>25794
>
>##### CMD.EXE + DIR /S C:\PROJECT> c:\cygwin\bin\time cmd /c dir /s
>>NUL 0.01user 0.01system 0:05.70elapsed 0%CPU (0avgtext+0avgdata
>6320maxresident)k 0inputs+0outputs (395major+0minor)pagefaults 0swaps
>
>##### Cygwin 'find -ls' (NutCracker doesn't have a '-ls') C:\PROJECT>
>c:\cygwin\bin\time c:\cygwin\bin\find -ls | wc -l 2.79user 7.81system
>0:10.60elapsed 100%CPU (0avgtext+0avgdata 14480maxresident)k 25794

I'm lost.  What does this have to do with the exec attribute?

Or, were you just climbing aboard the "Cygwin sure is slow" bandwagon?

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-26 23:17                                   ` NT directory traversal speed on 25K files on Cygwin Rutger Nijlunsing
  2006-02-27  1:18                                     ` Christopher Faylor
@ 2006-02-27  9:19                                     ` Andreas Ericsson
  2006-02-27 18:45                                       ` Rutger Nijlunsing
  1 sibling, 1 reply; 78+ messages in thread
From: Andreas Ericsson @ 2006-02-27  9:19 UTC (permalink / raw)
  To: git; +Cc: Christopher Faylor, git

Rutger Nijlunsing wrote:
> On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
> 
>>On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
>>
>>>filesystem is slow and locked down, and exec-attribute is NOT really
>>>useful even on NTFS (it is somehow related to execute permission and
>>>open files.  I still cannot figure out how exactly are they related).
>>
>>Again, it's not clear if you're talking about Windows or Cygwin but
>>under Cygwin, in the default configuration, the exec attribute means the
>>same thing to cygwin as it does to linux.
> 
> 
> I don't know about native Windows speed, but comparing NutCracker with
> Cygwin on a simple 'find . | wc -l' already gives a clue that looking
> at Cygwin to benchmark NT file inspection IO will give a skewed
> picture:
> 

Well, naturally. Cygwin is a userland implementation of a sane 
filesystem on top of a less sane one. File IO is bound to be slower when 
one FS is emulated on top of another. I think cygwin users are aware of 
this and simply accept the speed-for-sanity tradeoff. I know I would.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-27  1:18                                     ` Christopher Faylor
@ 2006-02-27 18:30                                       ` Rutger Nijlunsing
  2006-02-27 18:34                                         ` Christopher Faylor
  0 siblings, 1 reply; 78+ messages in thread
From: Rutger Nijlunsing @ 2006-02-27 18:30 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

On Sun, Feb 26, 2006 at 08:18:01PM -0500, Christopher Faylor wrote:
> On Mon, Feb 27, 2006 at 12:17:01AM +0100, Rutger Nijlunsing wrote:
> >On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
> >>On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
> >>>filesystem is slow and locked down, and exec-attribute is NOT really
> >>>useful even on NTFS (it is somehow related to execute permission and
> >>>open files.  I still cannot figure out how exactly are they related).
> >>
> >>Again, it's not clear if you're talking about Windows or Cygwin but
> >>under Cygwin, in the default configuration, the exec attribute means
> >>the same thing to cygwin as it does to linux.
> >
> >I don't know about native Windows speed, but comparing NutCracker with
> >Cygwin on a simple 'find .  | wc -l' already gives a clue that looking
> >at Cygwin to benchmark NT file inspection IO will give a skewed
> >picture:
> >
> >##### NutCracker $ time find .  | wc -l
> >
> >real    0m 1.44s
> >user    0m 0.45s
> >sys     0m 0.98s
> >25794
> >
> >##### Cygwin $ time c:\\cygwin\\bin\\find .  | wc -l
> >
> >real    0m 6.72s
> >user    0m 1.09s
> >sys     0m 5.59s
> >25794
> >
> >##### CMD.EXE + DIR /S C:\PROJECT> c:\cygwin\bin\time cmd /c dir /s
> >>NUL 0.01user 0.01system 0:05.70elapsed 0%CPU (0avgtext+0avgdata
> >6320maxresident)k 0inputs+0outputs (395major+0minor)pagefaults 0swaps
> >
> >##### Cygwin 'find -ls' (NutCracker doesn't have a '-ls') C:\PROJECT>
> >c:\cygwin\bin\time c:\cygwin\bin\find -ls | wc -l 2.79user 7.81system
> >0:10.60elapsed 100%CPU (0avgtext+0avgdata 14480maxresident)k 25794
> 
> I'm lost.  What does this have to do with the exec attribute?
> 
> Or, were you just climbing aboard the "Cygwin sure is slow" bandwagon?

I tried to get on the bandwagon 'NT file IO magnitudes slower => git
magnitudes slower', but missed the parade a week ago. Then another
parade showed up, but I managed to delete most of it with a
misfortunate shift-something in mutt... And then even messed up in
keeping the wrong paragraph... *hmpf*

However, the point I was trying to make was that git might be sped up
by a magnitude (although not all of the magnitudes in comparison to
Linux) by looking at why the file IO is this slow: Windows' file IO is
_not_ the only reason. Using a different/new/better fitted interface
to Cygwin or Win32 for a specific git task might help, although I have
no clue what or how.

-- 
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-27 18:30                                       ` Rutger Nijlunsing
@ 2006-02-27 18:34                                         ` Christopher Faylor
  0 siblings, 0 replies; 78+ messages in thread
From: Christopher Faylor @ 2006-02-27 18:34 UTC (permalink / raw)
  To: git

On Mon, Feb 27, 2006 at 07:30:49PM +0100, Rutger Nijlunsing wrote:
>However, the point I was trying to make was that git might be sped up
>by a magnitude (although not all of the magnitudes in comparison to
>Linux) by looking at why the file IO is this slow: Windows' file IO is
>_not_ the only reason. Using a different/new/better fitted interface
>to Cygwin or Win32 for a specific git task might help, although I have
>no clue what or how.

I'm going to revisit Cygwin's file I/O soon to see if I can figure out
what's adding the overhead and see if it really is inavoidable.  It's
been a while since I've gone through that exercise so it should prove
instructive.

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-27  9:19                                     ` Andreas Ericsson
@ 2006-02-27 18:45                                       ` Rutger Nijlunsing
  2006-03-02 13:40                                         ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Rutger Nijlunsing @ 2006-02-27 18:45 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git, Christopher Faylor, git

On Mon, Feb 27, 2006 at 10:19:09AM +0100, Andreas Ericsson wrote:
> Rutger Nijlunsing wrote:
> >On Sun, Feb 26, 2006 at 02:55:52PM -0500, Christopher Faylor wrote:
> >
> >>On Thu, Feb 23, 2006 at 03:07:07PM +0100, Alex Riesen wrote:
> >>
> >>>filesystem is slow and locked down, and exec-attribute is NOT really
> >>>useful even on NTFS (it is somehow related to execute permission and
> >>>open files.  I still cannot figure out how exactly are they related).
> >>
> >>Again, it's not clear if you're talking about Windows or Cygwin but
> >>under Cygwin, in the default configuration, the exec attribute means the
> >>same thing to cygwin as it does to linux.
> >
> >
> >I don't know about native Windows speed, but comparing NutCracker with
> >Cygwin on a simple 'find . | wc -l' already gives a clue that looking
> >at Cygwin to benchmark NT file inspection IO will give a skewed
> >picture:
> >
> 
> Well, naturally. Cygwin is a userland implementation of a sane 
> filesystem on top of a less sane one. File IO is bound to be slower when 
> one FS is emulated on top of another. I think cygwin users are aware of 
> this and simply accept the speed-for-sanity tradeoff. I know I would.

MKS NutCracker tries to solve the same issues as Cygwin tries to
solve. But maybe less sane, I don't know. But a simple 'find' is
several times faster than a Cygwin 'find'. Yes, very
unscientific. Just as unscientific as 'git is slow on Windows,
therefore Windows IO is slow'.

I'm not saying Cygwin is bad (actually, I'm installing on every
Windows PC I get my hand on ;), but using Cygwin for all file IO
instead of native Windows IO makes git a magnitude slower on Windows
than could-be. So a small portability layer with a function like
'given all filenames with all mtimes' might help, or we could look at
why Cygwin is slower in this case. Alas my Windows profiling skills
aren't that good...

Regards,
Rutger.

-- 
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: NT directory traversal speed on 25K files on Cygwin
  2006-02-27 18:45                                       ` Rutger Nijlunsing
@ 2006-03-02 13:40                                         ` Alex Riesen
  0 siblings, 0 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 13:40 UTC (permalink / raw)
  To: git; +Cc: Andreas Ericsson, Christopher Faylor, git

On 2/27/06, Rutger Nijlunsing <rutger@nospam.com> wrote:
> I'm not saying Cygwin is bad (actually, I'm installing on every
> Windows PC I get my hand on ;), but using Cygwin for all file IO
> instead of native Windows IO makes git a magnitude slower on Windows

By "slow filesystem" I actually meant the native filesystem access.
Cygwin does make it 6 times slower, that's right, and this can be
considered a disaster of course, but not as big as the windows api.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-26 19:55                                 ` Christopher Faylor
  2006-02-26 20:18                                   ` Linus Torvalds
  2006-02-26 23:17                                   ` NT directory traversal speed on 25K files on Cygwin Rutger Nijlunsing
@ 2006-03-02 14:10                                   ` Alex Riesen
  2006-03-02 15:00                                     ` Christopher Faylor
  2 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 14:10 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

Christopher, I'm terribly sorry for the long delays,
but that is something I can't change at this moment.

On 2/26/06, Christopher Faylor <me@cgf.cx> wrote:
> >>>It's not activestate perl actually.  It's only one platform it also
> >>>_has_ to support.  Is it worth supporting Windows?
> >>
> >>With or without cygwin?  With cygwin, I'd say "yes, unless it makes
> >>things terribly difficult to maintain and so long as we don't take
> >>performance hits on unices".  Without cygwin, I'd say "What?  It runs
> >>on windows?".
> >
> >There not much difference with or without cygwin.  The penalties of
> >doing any kind of support for it will pile up (as they started to do
> >with pipes).  Someday we'll have to start dropping features on Windows
> >or restrict them beyond their usefullness.  The fork emulation in
> >cygwin isn't perfect,
>
> If the speed of cygwin's fork is an issue then I'd previously suggested
> using spawn*.  The spawn family of functions were designed to emulate
> Windows functions of the same name.  They start a new process without
> the requirement of forking.

The effort of porting git to spawn-like interface has already started,
so there's no much left to say about the fork's speed...

> >signals do not work reliably (if at all),
>
> I'm not sure if you're mixing cygwin with windows here but if signals do
> not work reliably in Cygwin then that is something that we'd like to
> know about.  Signals *obviously* have to work fairly well for programs
> like ssh, bash, and X to work, however.

That's not enough.
Try interrupting busy processes. Like "git pull", "git clone" or make.

> Native Windows, OTOH, hardly has any signals at all and deals with
> signals in a way that is only vaguely like linux.

That makes the rest of installed system kind of useless in cygwin
environment. After interrupting a build process, which uses java
(don't ask) only make stops. The rest of the process runs happily
away.

Now, I know that windows has no signals at all and nothing which
even closely resembles them. I wont be pressing anyone to
implement them in windows, having the knowledge.
What I'd actually suggest is to drop their implementation entierly,
returning ENOSYS, so that programs are not fooled into believing
that the system will work as expected. It never will.
"Ctrl-C" in windows console is just a shortcut to TerminateProcess,
live with that.

> >filesystem is slow and locked down, and exec-attribute is NOT really
> >useful even on NTFS (it is somehow related to execute permission and
> >open files.  I still cannot figure out how exactly are they related).
>
> Again, it's not clear if you're talking about Windows or Cygwin but
> under Cygwin, in the default configuration, the exec attribute means the
> same thing to cygwin as it does to linux.

I'm talking about git and native windows interaction: I cannot use umask,
because I have to use stupid windows programs, and they always create
"executable" *.c and *.h, and I cannot blindly remove it with something
like "chmod -R -x", because it'd remove it also from executables. The
poor executables lose their _rights_ to be executed (why does cygwin use
windows permissions? They cannot correlate to unix attributes, can they?)
An .bat or .cmd without right to execute it is a pain in my build system
(and no, I'm not allowed to change that damn stupid build system).

Is there any way to tell cygwin that the files it hasn't seen or touched yet
are _not_executables_?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-02-26 20:40                                     ` Christopher Faylor
@ 2006-03-02 14:18                                       ` Alex Riesen
  2006-03-02 15:18                                         ` Mark Wooding
  2006-03-02 15:22                                         ` Christopher Faylor
  0 siblings, 2 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 14:18 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

On 2/26/06, Christopher Faylor <me@cgf.cx> wrote:
> The cygwin/windows version of spawn is basically like an extended version
> of exec*():
>
> pid = spawnlp (P_NOWAIT, "/bin/ls", "ls", "-l", NULL);
>

By the way, is argv worked around?
AFAIK, windows has only one argument, returned by GetCommandLine?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 14:10                                   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Alex Riesen
@ 2006-03-02 15:00                                     ` Christopher Faylor
  2006-03-02 16:10                                       ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Christopher Faylor @ 2006-03-02 15:00 UTC (permalink / raw)
  To: git

On Thu, Mar 02, 2006 at 03:10:30PM +0100, Alex Riesen wrote:
>Christopher, I'm terribly sorry for the long delays,
>but that is something I can't change at this moment.
>
>On 2/26/06, Christopher Faylor <me@cgf.cx> wrote:
>> >>>It's not activestate perl actually.  It's only one platform it also
>> >>>_has_ to support.  Is it worth supporting Windows?
>> >>
>> >>With or without cygwin?  With cygwin, I'd say "yes, unless it makes
>> >>things terribly difficult to maintain and so long as we don't take
>> >>performance hits on unices".  Without cygwin, I'd say "What?  It runs
>> >>on windows?".
>> >
>> >There not much difference with or without cygwin.  The penalties of
>> >doing any kind of support for it will pile up (as they started to do
>> >with pipes).  Someday we'll have to start dropping features on Windows
>> >or restrict them beyond their usefullness.  The fork emulation in
>> >cygwin isn't perfect,
>>
>> If the speed of cygwin's fork is an issue then I'd previously suggested
>> using spawn*.  The spawn family of functions were designed to emulate
>> Windows functions of the same name.  They start a new process without
>> the requirement of forking.
>
>The effort of porting git to spawn-like interface has already started,
>so there's no much left to say about the fork's speed...
>
>> >signals do not work reliably (if at all),
>>
>> I'm not sure if you're mixing cygwin with windows here but if signals do
>> not work reliably in Cygwin then that is something that we'd like to
>> know about.  Signals *obviously* have to work fairly well for programs
>> like ssh, bash, and X to work, however.
>
>That's not enough.
>Try interrupting busy processes. Like "git pull", "git clone" or make.

Are you saying that typing CTRL-C doesn't work when you use "git pull"?
If so, give me a real bug report that I can look into.  I interrupt
"busy" processes on cygwin all of the time so I'm not going to spend a
few hours typing "git pull" on my system only to find out that you are
talking about an environment that uses ActiveState perl on Windows 95
using Netware.

If you are reporting a problem you need to provide details.

>> Native Windows, OTOH, hardly has any signals at all and deals with
>> signals in a way that is only vaguely like linux.
>
>That makes the rest of installed system kind of useless in cygwin
>environment. After interrupting a build process, which uses java
>(don't ask) only make stops. The rest of the process runs happily
>away.

This sounds like a java bug which is entirely unrelated to git.

>Now, I know that windows has no signals at all and nothing which
>even closely resembles them.

Actually, Windows does understand CTRL-C and any native windows console
program should honor CTRL-C in a manner similar to UNIX, i.e., if the
program doesn't trap SIGINT with 'signal()', it will cause the program
to terminate.  There are also other mechanisms for a native windows
program to deal with CTRL-C so this really shouldn't be an issue for
any well-written program.

>I wont be pressing anyone to implement them in windows, having the
>knowledge.  What I'd actually suggest is to drop their implementation
>entierly, returning ENOSYS,

You're not being clear again, but if you are actually promoting the
notion of cygwin not implementing signals then that is a really daft
idea.  Really.  Go to the Cygwin web site and look at all of the
packages which have been ported.  Now think about how they would work if
Cygwin didn't support signals.  bash wouldn't work, openssh, X wouldn't
work.

>so that programs are not fooled into believing that the system will
>work as expected.  It never will.  "Ctrl-C" in windows console is just
>a shortcut to TerminateProcess, live with that.

Let me say it again since it isn't clear that you are getting it.  If
signals in a pure cygwin environment don't work then that is *a bug*.
If you are running pure windows programs in the mix with cygwin programs
then if *they* don't stop when you hit CTRL-C, that is undoubtedly a bug
in that pure windows program.

If you find that a pure windows program terminates when run from a
windows command prompt but keeps running when run by a cygwin program
then that is likely a cygwin problem that can be reported to the cygwin
mailing list.

>>>filesystem is slow and locked down, and exec-attribute is NOT really
>>>useful even on NTFS (it is somehow related to execute permission and
>>>open files.  I still cannot figure out how exactly are they related).
>>
>>Again, it's not clear if you're talking about Windows or Cygwin but
>>under Cygwin, in the default configuration, the exec attribute means
>>the same thing to cygwin as it does to linux.
>
>I'm talking about git and native windows interaction:

I'd suggest that using git with native windows programs should probably
be considered "unsupported" since you seem to be having so much trouble
with it.

>I cannot use umask, because I have to use stupid windows programs, and
>they always create "executable" *.c and *.h, and I cannot blindly
>remove it with something like "chmod -R -x", because it'd remove it
>also from executables.

  find . -name '*.[ch]' | xargs chmod a-x

>The poor executables lose their _rights_ to be executed (why does
>cygwin use windows permissions?  They cannot correlate to unix
>attributes, can they?)

Please read the Cygwin user's guide for a discussion about how file
permissions are implemented.  And, then, when you are outraged about how
unclear that documentation is please send comments and improvements to
the cygwin mailing list.

I don't see why it is appropriate to be discussing how Cygwin implements
UNIX permissions in this mailing list unless the implementation affects
git somehow.

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 14:18                                       ` Alex Riesen
@ 2006-03-02 15:18                                         ` Mark Wooding
  2006-03-02 16:11                                           ` Alex Riesen
  2006-03-02 15:22                                         ` Christopher Faylor
  1 sibling, 1 reply; 78+ messages in thread
From: Mark Wooding @ 2006-03-02 15:18 UTC (permalink / raw)
  To: git

"Alex Riesen" <raa.lkml@gmail.com> wrote:

> AFAIK, windows has only one argument, returned by GetCommandLine?

This is true, but there's a standard quoting convention which (in
particular) Microsoft's C library uses to split the single argument back
into an argv.  The spawn* functions quote; the C library startup stuff
unquotes and splits.

The actual quoting convention is /horrible/.  I had to implement the
darned thing once.  See

  http://sources.redhat.com/ml/cygwin/1999-08/msg00701.html

for the details.

-- [mdw]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 14:18                                       ` Alex Riesen
  2006-03-02 15:18                                         ` Mark Wooding
@ 2006-03-02 15:22                                         ` Christopher Faylor
  2006-03-02 16:20                                           ` Alex Riesen
  1 sibling, 1 reply; 78+ messages in thread
From: Christopher Faylor @ 2006-03-02 15:22 UTC (permalink / raw)
  To: git

On Thu, Mar 02, 2006 at 03:18:44PM +0100, Alex Riesen wrote:
>On 2/26/06, Christopher Faylor <me@cgf.cx> wrote:
>> The cygwin/windows version of spawn is basically like an extended version
>> of exec*():
>>
>> pid = spawnlp (P_NOWAIT, "/bin/ls", "ls", "-l", NULL);
>>
>
>By the way, is argv worked around?
>AFAIK, windows has only one argument, returned by GetCommandLine?

Cygwin passes an argv list between cygwin processes and a quoted command
line to pure windows processes.

cgf

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 15:00                                     ` Christopher Faylor
@ 2006-03-02 16:10                                       ` Alex Riesen
  2006-03-02 17:39                                         ` Andreas Ericsson
  0 siblings, 1 reply; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 16:10 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

On 3/2/06, Christopher Faylor <me@cgf.cx> wrote:
> >That's not enough.
> >Try interrupting busy processes. Like "git pull", "git clone" or make.
>
> Are you saying that typing CTRL-C doesn't work when you use "git pull"?

It does. Almost always. It's the seldom cases when this does not
really work which annoy me that much.

> If so, give me a real bug report that I can look into.  I interrupt
> "busy" processes on cygwin all of the time so I'm not going to spend a
> few hours typing "git pull" on my system only to find out that you are
> talking about an environment that uses ActiveState perl on Windows 95
> using Netware.

well, it is almost exactly the case: WinNT 2K. And I cannot change this.

> If you are reporting a problem you need to provide details.

I am NOT reporting a problem. Everyone knows there are these problems,
it's just almost no one (including me) cares enough about getting anything
to work sanely on windows.

Please, stop assuming that every my complaint is a bug report about
cygwin. It is not. You can use my mails as you please, even as bug reports.
If you ask nicely, I can provide more details maybe. But I am not asking
YOU for anything, and not complaining to YOU about anything.

I _do_not_ like how Cygwin workarounds windows, but I respect the
effort and understand why it happens. Still, I'd prefer it die. I'll try to
keep it moving, but no farther than needed and only when I really have to.

> There are also other mechanisms for a native windows
> program to deal with CTRL-C so this really shouldn't be an issue for
> any well-written program.

In windows you have to do hell of a lot useless typing to write what you
consider a "well-written program".

> >I wont be pressing anyone to implement them in windows, having the
> >knowledge.  What I'd actually suggest is to drop their implementation
> >entierly, returning ENOSYS,
>
> You're not being clear again, but if you are actually promoting the
> notion of cygwin not implementing signals then that is a really daft
> idea.  Really.  Go to the Cygwin web site and look at all of the
> packages which have been ported.  Now think about how they would work if
> Cygwin didn't support signals.  bash wouldn't work, openssh, X wouldn't
> work.

That's right. They are not _ported_. I'm not interested in xterm which
prints page in a minute.

> >so that programs are not fooled into believing that the system will
> >work as expected.  It never will.  "Ctrl-C" in windows console is just
> >a shortcut to TerminateProcess, live with that.
>
> Let me say it again since it isn't clear that you are getting it.  If
> signals in a pure cygwin environment don't work then that is *a bug*.

Whatever you say. I never expected them to, personally.

> If you are running pure windows programs in the mix with cygwin programs
> then if *they* don't stop when you hit CTRL-C, that is undoubtedly a bug
> in that pure windows program.

Maybe. I wouldn't blame that poor windows programmer though: it's hard,
boring and in 99.9999% of starts of that program - dead code.

> If you find that a pure windows program terminates when run from a
> windows command prompt but keeps running when run by a cygwin program
> then that is likely a cygwin problem that can be reported to the cygwin
> mailing list.

gui applications detach from cmd (not from cygwin console),
so that kind of pointless exercise.

> I'd suggest that using git with native windows programs should probably
> be considered "unsupported" since you seem to be having so much trouble
> with it.

I'd suggest calling cygwin ports unsupported.

> >I cannot use umask, because I have to use stupid windows programs, and
> >they always create "executable" *.c and *.h, and I cannot blindly
> >remove it with something like "chmod -R -x", because it'd remove it
> >also from executables.
>
>   find . -name '*.[ch]' | xargs chmod a-x

find . -name '*.[ch]' -o -name '*.[ch]pp' -o -name Makefile -o -name
'*.txt' -o ...ooh! damn it^C -print0| xargs -0 chmod -x
You oversimplifying.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 15:18                                         ` Mark Wooding
@ 2006-03-02 16:11                                           ` Alex Riesen
  0 siblings, 0 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 16:11 UTC (permalink / raw)
  To: Mark Wooding; +Cc: git

On 3/2/06, Mark Wooding <mdw@distorted.org.uk> wrote:
>
> > AFAIK, windows has only one argument, returned by GetCommandLine?
>
> This is true, but there's a standard quoting convention which (in
> particular) Microsoft's C library uses to split the single argument back
> into an argv.  The spawn* functions quote; the C library startup stuff
> unquotes and splits.

Doesn't for programs using WinMain, which is probably 90% of all
windows programs.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 15:22                                         ` Christopher Faylor
@ 2006-03-02 16:20                                           ` Alex Riesen
  0 siblings, 0 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 16:20 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

On 3/2/06, Christopher Faylor <me@cgf.cx> wrote:
> >> The cygwin/windows version of spawn is basically like an extended version
> >> of exec*():
> >>
> >> pid = spawnlp (P_NOWAIT, "/bin/ls", "ls", "-l", NULL);
> >>
> >
> >By the way, is argv worked around?
> >AFAIK, windows has only one argument, returned by GetCommandLine?
>
> Cygwin passes an argv list between cygwin processes and a quoted command
> line to pure windows processes.

What for? They can't use it anyway.

$ notepad '"abc"'

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
@ 2006-03-02 16:44 Christopher Faylor
  2006-03-02 16:55 ` Shawn Pearce
  2006-03-02 17:33 ` Johannes Schindelin
  0 siblings, 2 replies; 78+ messages in thread
From: Christopher Faylor @ 2006-03-02 16:44 UTC (permalink / raw)
  To: git

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)

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 16:44 Christopher Faylor
@ 2006-03-02 16:55 ` Shawn Pearce
  2006-03-02 22:09   ` Alex Riesen
  2006-03-02 17:33 ` Johannes Schindelin
  1 sibling, 1 reply; 78+ messages in thread
From: Shawn Pearce @ 2006-03-02 16:55 UTC (permalink / raw)
  To: git

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.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 16:44 Christopher Faylor
  2006-03-02 16:55 ` Shawn Pearce
@ 2006-03-02 17:33 ` Johannes Schindelin
  1 sibling, 0 replies; 78+ messages in thread
From: Johannes Schindelin @ 2006-03-02 17:33 UTC (permalink / raw)
  To: Christopher Faylor; +Cc: git

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

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 16:10                                       ` Alex Riesen
@ 2006-03-02 17:39                                         ` Andreas Ericsson
  2006-03-02 22:01                                           ` Alex Riesen
  0 siblings, 1 reply; 78+ messages in thread
From: Andreas Ericsson @ 2006-03-02 17:39 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Christopher Faylor, git

Ye gawds, Alex. If you complained this much to your employer you'd get 
to run whatever OS you want.

Alex Riesen wrote:

[ lots of things ]

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 17:39                                         ` Andreas Ericsson
@ 2006-03-02 22:01                                           ` Alex Riesen
  0 siblings, 0 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 22:01 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Christopher Faylor, git

Andreas Ericsson, Thu, Mar 02, 2006 18:39:59 +0100:
> Ye gawds, Alex. If you complained this much to your employer you'd get 
> to run whatever OS you want.

I never stopped. I usually manage to convince them, it just hasn't
happened here yet.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 16:55 ` Shawn Pearce
@ 2006-03-02 22:09   ` Alex Riesen
  2006-03-02 23:27     ` Linus Torvalds
  2006-03-03  0:14     ` Christopher Faylor
  0 siblings, 2 replies; 78+ messages in thread
From: Alex Riesen @ 2006-03-02 22:09 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git

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

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 22:09   ` Alex Riesen
@ 2006-03-02 23:27     ` Linus Torvalds
  2006-03-03  0:34       ` Junio C Hamano
  2006-03-03  0:14     ` Christopher Faylor
  1 sibling, 1 reply; 78+ messages in thread
From: Linus Torvalds @ 2006-03-02 23:27 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Shawn Pearce, git



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("   ...");
		}
	}
}

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 22:09   ` Alex Riesen
  2006-03-02 23:27     ` Linus Torvalds
@ 2006-03-03  0:14     ` Christopher Faylor
  1 sibling, 0 replies; 78+ messages in thread
From: Christopher Faylor @ 2006-03-03  0:14 UTC (permalink / raw)
  To: git

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)

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-02 23:27     ` Linus Torvalds
@ 2006-03-03  0:34       ` Junio C Hamano
  2006-03-03  0:49         ` Linus Torvalds
  0 siblings, 1 reply; 78+ messages in thread
From: Junio C Hamano @ 2006-03-03  0:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

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

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-03  0:34       ` Junio C Hamano
@ 2006-03-03  0:49         ` Linus Torvalds
  2006-03-03  1:25           ` Junio C Hamano
  0 siblings, 1 reply; 78+ messages in thread
From: Linus Torvalds @ 2006-03-03  0:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git



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

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-03  0:49         ` Linus Torvalds
@ 2006-03-03  1:25           ` Junio C Hamano
  2006-03-03  1:52             ` Linus Torvalds
  0 siblings, 1 reply; 78+ messages in thread
From: Junio C Hamano @ 2006-03-03  1:25 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

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?

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6
  2006-03-03  1:25           ` Junio C Hamano
@ 2006-03-03  1:52             ` Linus Torvalds
  0 siblings, 0 replies; 78+ messages in thread
From: Linus Torvalds @ 2006-03-03  1:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git



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

^ permalink raw reply	[flat|nested] 78+ messages in thread

end of thread, other threads:[~2006-03-03  1:53 UTC | newest]

Thread overview: 78+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-20 18:37 Should we support Perl 5.6? Johannes Schindelin
2006-02-20 19:10 ` Eric Wong
2006-02-20 21:01   ` Andreas Ericsson
2006-02-20 21:15     ` Junio C Hamano
2006-02-20 22:05   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Junio C Hamano
2006-02-20 22:12     ` [PATCH] rerere: " Junio C Hamano
2006-02-20 22:12     ` [PATCH] send-email: " Junio C Hamano
2006-02-20 22:12     ` [PATCH] svmimport: " Junio C Hamano
2006-02-20 22:19     ` [PATCH] cvsimport: " Junio C Hamano
2006-02-21 17:30     ` [PATCH] fmt-merge-msg: " Alex Riesen
2006-02-21 20:36       ` Sam Vilain
2006-02-21 21:57         ` Alex Riesen
2006-02-21 22:19           ` Johannes Schindelin
2006-02-21 22:35             ` Eric Wong
2006-02-21 22:38             ` Shawn Pearce
2006-02-21 23:00             ` Martin Langhoff
2006-02-21 22:38           ` Sam Vilain
2006-02-22 16:35             ` Alex Riesen
2006-02-22 19:44               ` Johannes Schindelin
2006-02-22 19:51               ` Sam Vilain
2006-02-22 19:54                 ` Junio C Hamano
2006-02-22 22:00               ` Johannes Schindelin
2006-02-22 22:25                 ` Junio C Hamano
2006-02-23  8:00                 ` Alex Riesen
2006-02-23  8:45                   ` Junio C Hamano
2006-02-23  9:35                     ` Alex Riesen
2006-02-23  9:41                       ` Alex Riesen
2006-02-23  9:48                         ` Andreas Ericsson
2006-02-23 10:10                           ` Alex Riesen
2006-02-23 13:29                             ` Andreas Ericsson
2006-02-23 14:07                               ` Alex Riesen
2006-02-23 14:22                                 ` Andreas Ericsson
2006-02-23 17:13                                 ` Linus Torvalds
2006-02-23 19:32                                   ` Junio C Hamano
2006-02-23 19:38                                     ` Johannes Schindelin
2006-02-23 19:54                                       ` Linus Torvalds
2006-02-23 20:19                                         ` Johannes Schindelin
2006-02-23 19:51                                     ` Linus Torvalds
2006-02-23 20:31                                       ` Sam Vilain
2006-02-24  6:43                                         ` Linus Torvalds
2006-02-23 21:43                                   ` Alex Riesen
2006-02-26 19:55                                 ` Christopher Faylor
2006-02-26 20:18                                   ` Linus Torvalds
2006-02-26 20:40                                     ` Christopher Faylor
2006-03-02 14:18                                       ` Alex Riesen
2006-03-02 15:18                                         ` Mark Wooding
2006-03-02 16:11                                           ` Alex Riesen
2006-03-02 15:22                                         ` Christopher Faylor
2006-03-02 16:20                                           ` Alex Riesen
2006-02-26 23:17                                   ` NT directory traversal speed on 25K files on Cygwin Rutger Nijlunsing
2006-02-27  1:18                                     ` Christopher Faylor
2006-02-27 18:30                                       ` Rutger Nijlunsing
2006-02-27 18:34                                         ` Christopher Faylor
2006-02-27  9:19                                     ` Andreas Ericsson
2006-02-27 18:45                                       ` Rutger Nijlunsing
2006-03-02 13:40                                         ` Alex Riesen
2006-03-02 14:10                                   ` [PATCH] fmt-merge-msg: avoid open "-|" list form for Perl 5.6 Alex Riesen
2006-03-02 15:00                                     ` Christopher Faylor
2006-03-02 16:10                                       ` Alex Riesen
2006-03-02 17:39                                         ` Andreas Ericsson
2006-03-02 22:01                                           ` Alex Riesen
2006-02-26 20:33                               ` Christopher Faylor
2006-02-24 12:02               ` Eric Wong
2006-02-24 13:44                 ` Johannes Schindelin
2006-02-24 16:14                   ` Linus Torvalds
2006-02-21 20:56       ` Eric Wong
2006-02-21 22:04         ` Alex Riesen
     [not found]           ` <1cf1c57a0602211412r1988b14ao435edd29207dc0d0@mail.gmail.com>
2006-02-21 22:13             ` Ron Parker
  -- strict thread matches above, loose matches on Subject: below --
2006-03-02 16:44 Christopher Faylor
2006-03-02 16:55 ` Shawn Pearce
2006-03-02 22:09   ` Alex Riesen
2006-03-02 23:27     ` Linus Torvalds
2006-03-03  0:34       ` Junio C Hamano
2006-03-03  0:49         ` Linus Torvalds
2006-03-03  1:25           ` Junio C Hamano
2006-03-03  1:52             ` Linus Torvalds
2006-03-03  0:14     ` Christopher Faylor
2006-03-02 17:33 ` Johannes Schindelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).