[PATCH 1/3] git-cvsexportcommit.perl tabified

[PATCH 1/3] git-cvsexportcommit.perl tabified

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
    This patch attempts to tabify this code.
    Please be gentle, this is my second time
    submitting a patch.

 git-cvsexportcommit.perl |  284 +++++++++++++++++++++++-----------------------
 1 files changed, 141 insertions(+), 143 deletions(-)

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index a33fa8d..b330c3d 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -12,7 +12,7 @@ use Data::Dumper;
 use File::Basename qw(basename dirname);
 
 unless ($ENV{GIT_DIR} && -r $ENV{GIT_DIR}){
-    die "GIT_DIR is not defined or is unreadable";
+	die "GIT_DIR is not defined or is unreadable";
 }
 
 our ($opt_h, $opt_P, $opt_p, $opt_v, $opt_c, $opt_f, $opt_a, $opt_m, $opt_d, $opt_u);
@@ -25,15 +25,13 @@ die "Need at least one commit identifier!" unless @ARGV;
 
 my @cvs;
 if ($opt_d) {
-	@cvs = ('cvs', '-d', $opt_d);
+		@cvs = ('cvs', '-d', $opt_d);
 } else {
-	@cvs = ('cvs');
+		@cvs = ('cvs');
 }
 
 # setup a tempdir
-our ($tmpdir, $tmpdirname) = tempdir('git-cvsapplycommit-XXXXXX',
-				     TMPDIR => 1,
-				     CLEANUP => 1);
+our ($tmpdir, $tmpdirname) = tempdir('git-cvsapplycommit-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 
 # resolve target commit
 my $commit;
@@ -41,18 +39,18 @@ $commit = pop @ARGV;
 $commit = safe_pipe_capture('git-rev-parse', '--verify', "$commit^0");
 chomp $commit;
 if ($?) {
-    die "The commit reference $commit did not resolve!";
+	die "The commit reference $commit did not resolve!";
 }
 
 # resolve what parent we want
 my $parent;
 if (@ARGV) {
-    $parent = pop @ARGV;
-    $parent =  safe_pipe_capture('git-rev-parse', '--verify', "$parent^0");
-    chomp $parent;
-    if ($?) {
-	die "The parent reference did not resolve!";
-    }
+	$parent = pop @ARGV;
+	$parent =  safe_pipe_capture('git-rev-parse', '--verify', "$parent^0");
+	chomp $parent;
+	if ($?) {
+		die "The parent reference did not resolve!";
+	}
 }
 
 # find parents from the commit itself
@@ -65,44 +63,44 @@ my $title;
 my $msg = '';
 
 foreach my $line (@commit) {
-    chomp $line;
-    if ($stage eq 'headers' && $line eq '') {
-	$stage = 'msg';
-	next;
-    }
-
-    if ($stage eq 'headers') {
-	if ($line =~ m/^parent (\w{40})$/) { # found a parent
-	    push @parents, $1;
-	} elsif ($line =~ m/^author (.+) \d+ [-+]\d+$/) {
-	    $author = $1;
-	} elsif ($line =~ m/^committer (.+) \d+ [-+]\d+$/) {
-	    $committer = $1;
+	chomp $line;
+	if ($stage eq 'headers' && $line eq '') {
+		$stage = 'msg';
+		next;
 	}
-    } else {
-	$msg .= $line . "\n";
-	unless ($title) {
-	    $title = $line;
+
+	if ($stage eq 'headers') {
+		if ($line =~ m/^parent (\w{40})$/) { # found a parent
+			push @parents, $1;
+		} elsif ($line =~ m/^author (.+) \d+ [-+]\d+$/) {
+			$author = $1;
+		} elsif ($line =~ m/^committer (.+) \d+ [-+]\d+$/) {
+			$committer = $1;
+		}
+	} else {
+		$msg .= $line . "\n";
+		unless ($title) {
+			$title = $line;
+		}
 	}
-    }
 }
 
 if ($parent) {
-    my $found;
-    # double check that it's a valid parent
-    foreach my $p (@parents) {
-	if ($p eq $parent) {
-	    $found = 1;
-	    last;
-	}; # found it
-    }
-    die "Did not find $parent in the parents for this commit!" if !$found and !$opt_P;
+	my $found;
+	# double check that it's a valid parent
+	foreach my $p (@parents) {
+		if ($p eq $parent) {
+			$found = 1;
+			last;
+		}; # found it
+	}
+	die "Did not find $parent in the parents for this commit!" if !$found and !$opt_P;
 } else { # we don't have a parent from the cmdline...
-    if (@parents == 1) { # it's safe to get it from the commit
-	$parent = $parents[0];
-    } else { # or perhaps not!
-	die "This commit has more than one parent -- please name the parent you want to use explicitly";
-    }
+	if (@parents == 1) { # it's safe to get it from the commit
+		$parent = $parents[0];
+	} else { # or perhaps not!
+		die "This commit has more than one parent -- please name the parent you want to use explicitly";
+	}
 }
 
 $opt_v && print "Applying to CVS commit $commit from parent $parent\n";
@@ -110,14 +108,14 @@ $opt_v && print "Applying to CVS commit $commit from parent $parent\n";
 # grab the commit message
 open(MSG, ">.msg") or die "Cannot open .msg for writing";
 if ($opt_m) {
-    print MSG $opt_m;
+	print MSG $opt_m;
 }
 print MSG $msg;
 if ($opt_a) {
-    print MSG "\n\nAuthor: $author\n";
-    if ($author ne $committer) {
-	print MSG "Committer: $committer\n";
-    }
+	print MSG "\n\nAuthor: $author\n";
+	if ($author ne $committer) {
+		print MSG "Committer: $committer\n";
+	}
 }
 close MSG;
 
@@ -153,70 +151,70 @@ map { s/\\([0-7]{3})/sprintf('%c',oct $1)/eg } @bfiles,@files;
 my $dirty;
 my @dirs;
 foreach my $p (@afiles) {
-    my $path = dirname $p;
-    while (!-d $path and ! grep { $_ eq $path } @dirs) {
-	unshift @dirs, $path;
-	$path = dirname $path;
-    }
+	my $path = dirname $p;
+	while (!-d $path and ! grep { $_ eq $path } @dirs) {
+		unshift @dirs, $path;
+		$path = dirname $path;
+	}
 }
 
 # ... check dirs,
 foreach my $d (@dirs) {
-    if (-e $d) {
-	$dirty = 1;
-	warn "$d exists and is not a directory!\n";
-    }
+	if (-e $d) {
+		$dirty = 1;
+		warn "$d exists and is not a directory!\n";
+	}
 }
 
 # ... query status of all files that we have a directory for and parse output of 'cvs status' to %cvsstat.
 my @canstatusfiles;
 foreach my $f (@files) {
-    my $path = dirname $f;
-    next if (grep { $_ eq $path } @dirs);
-    push @canstatusfiles, $f;
+	my $path = dirname $f;
+	next if (grep { $_ eq $path } @dirs);
+	push @canstatusfiles, $f;
 }
 
 my %cvsstat;
 if (@canstatusfiles) {
-    if ($opt_u) {
-      my @updated = safe_pipe_capture(@cvs, 'update', @canstatusfiles);
-      print @updated;
-    }
-    my @cvsoutput;
-    @cvsoutput= safe_pipe_capture(@cvs, 'status', @canstatusfiles);
-    my $matchcount = 0;
-    foreach my $l (@cvsoutput) {
-        chomp $l;
-        if ( $l =~ /^File:/ and  $l =~ /Status: (.*)$/ ) {
-            $cvsstat{$canstatusfiles[$matchcount]} = $1;
-            $matchcount++;
-        }
-    }
+	if ($opt_u) {
+	  my @updated = safe_pipe_capture(@cvs, 'update', @canstatusfiles);
+	  print @updated;
+	}
+	my @cvsoutput;
+	@cvsoutput= safe_pipe_capture(@cvs, 'status', @canstatusfiles);
+	my $matchcount = 0;
+	foreach my $l (@cvsoutput) {
+		chomp $l;
+		if ( $l =~ /^File:/ and  $l =~ /Status: (.*)$/ ) {
+			$cvsstat{$canstatusfiles[$matchcount]} = $1;
+			$matchcount++;
+		}
+	}
 }
 
 # ... validate new files,
 foreach my $f (@afiles) {
-    if (defined ($cvsstat{$f}) and $cvsstat{$f} ne "Unknown") {
-	$dirty = 1;
-	warn "File $f is already known in your CVS checkout -- perhaps it has been added by another user. Or this may indicate that it exists on a different branch. If this is the case, use -f to force the merge.\n";
-	warn "Status was: $cvsstat{$f}\n";
-    }
+	if (defined ($cvsstat{$f}) and $cvsstat{$f} ne "Unknown") {
+		$dirty = 1;
+		warn "File $f is already known in your CVS checkout -- perhaps it has been added by another user. Or this may indicate that it exists on a different branch. If this is the case, use -f to force the merge.\n";
+		warn "Status was: $cvsstat{$f}\n";
+	}
 }
 # ... validate known files.
 foreach my $f (@files) {
-    next if grep { $_ eq $f } @afiles;
-    # TODO:we need to handle removed in cvs
-    unless (defined ($cvsstat{$f}) and $cvsstat{$f} eq "Up-to-date") {
-	$dirty = 1;
-	warn "File $f not up to date but has status '$cvsstat{$f}' in your CVS checkout!\n";
-    }
+	next if grep { $_ eq $f } @afiles;
+	# TODO:we need to handle removed in cvs
+	unless (defined ($cvsstat{$f}) and $cvsstat{$f} eq "Up-to-date") {
+		$dirty = 1;
+		warn "File $f not up to date but has status '$cvsstat{$f}' in your CVS checkout!\n";
+	}
 }
 if ($dirty) {
-    if ($opt_f) {	warn "The tree is not clean -- forced merge\n";
-	$dirty = 0;
-    } else {
-	die "Exiting: your CVS tree is not clean for this merge.";
-    }
+	if ($opt_f) {		warn "The tree is not clean -- forced merge\n";
+		$dirty = 0;
+	} else {
+		die "Exiting: your CVS tree is not clean for this merge.";
+	}
 }
 
 print "Applying\n";
@@ -225,30 +223,30 @@ print "Applying\n";
 print "Patch applied successfully. Adding new files and directories to CVS\n";
 my $dirtypatch = 0;
 foreach my $d (@dirs) {
-    if (system(@cvs,'add',$d)) {
-	$dirtypatch = 1;
-	warn "Failed to cvs add directory $d -- you may need to do it manually";
-    }
+	if (system(@cvs,'add',$d)) {
+		$dirtypatch = 1;
+		warn "Failed to cvs add directory $d -- you may need to do it manually";
+	}
 }
 
 foreach my $f (@afiles) {
-    if (grep { $_ eq $f } @bfiles) {
-      system(@cvs, 'add','-kb',$f);
-    } else {
-      system(@cvs, 'add', $f);
-    }
-    if ($?) {
-	$dirtypatch = 1;
-	warn "Failed to cvs add $f -- you may need to do it manually";
-    }
+	if (grep { $_ eq $f } @bfiles) {
+		system(@cvs, 'add','-kb',$f);
+	} else {
+		system(@cvs, 'add', $f);
+	}
+	if ($?) {
+		$dirtypatch = 1;
+		warn "Failed to cvs add $f -- you may need to do it manually";
+	}
 }
 
 foreach my $f (@dfiles) {
-    system(@cvs, 'rm', '-f', $f);
-    if ($?) {
-	$dirtypatch = 1;
-	warn "Failed to cvs rm -f $f -- you may need to do it manually";
-    }
+	system(@cvs, 'rm', '-f', $f);
+	if ($?) {
+		$dirtypatch = 1;
+		warn "Failed to cvs rm -f $f -- you may need to do it manually";
+	}
 }
 
 print "Commit to CVS\n";
@@ -257,25 +255,25 @@ my @commitfiles = map { unless (m/\s/) { '\''.$_.'\''; } else { $_; }; } (@files
 my $cmd = join(' ', @cvs)." commit -F .msg @commitfiles";
 
 if ($dirtypatch) {
-    print "NOTE: One or more hunks failed to apply cleanly.\n";
-    print "You'll need to apply the patch in .cvsexportcommit.diff manually\n";
-    print "using a patch program. After applying the patch and resolving the\n";
-    print "problems you may commit using:";
-    print "\n    $cmd\n\n";
-    exit(1);
+	print "NOTE: One or more hunks failed to apply cleanly.\n";
+	print "You'll need to apply the patch in .cvsexportcommit.diff manually\n";
+	print "using a patch program. After applying the patch and resolving the\n";
+	print "problems you may commit using:";
+	print "\n	$cmd\n\n";
+	exit(1);
 }
 
 if ($opt_c) {
-    print "Autocommit\n  $cmd\n";
-    print safe_pipe_capture(@cvs, 'commit', '-F', '.msg', @files);
-    if ($?) {
-	die "Exiting: The commit did not succeed";
-    }
-    print "Committed successfully to CVS\n";
-    # clean up
-    unlink(".msg");
+	print "Autocommit\n  $cmd\n";
+	print safe_pipe_capture(@cvs, 'commit', '-F', '.msg', @files);
+	if ($?) {
+		die "Exiting: The commit did not succeed";
+	}
+	print "Committed successfully to CVS\n";
+	# clean up
+	unlink(".msg");
 } else {
-    print "Ready for you to commit, just run:\n\n   $cmd\n";
+	print "Ready for you to commit, just run:\n\n   $cmd\n";
 }
 
 # clean up
@@ -297,25 +295,25 @@ END
 # to work around shell problems with weird characters in arguments
 # if the exec returns non-zero we die
 sub safe_pipe_capture {
-    my @output;
-    if (my $pid = open my $child, '-|') {
-	@output = (<$child>);
-	close $child or die join(' ',@_).": $! $?";
-    } else {
-	exec(@_) or die "$! $?"; # exec() can fail the executable can't be found
-    }
-    return wantarray ? @output : join('',@output);
+	my @output;
+	if (my $pid = open my $child, '-|') {
+		@output = (<$child>);
+		close $child or die join(' ',@_).": $! $?";
+	} else {
+		exec(@_) or die "$! $?"; # exec() can fail the executable can't be found
+	}
+	return wantarray ? @output : join('',@output);
 }
 
 sub safe_pipe_capture_blob {
-    my $output;
-    if (my $pid = open my $child, '-|') {
-        local $/;
-	undef $/;
-	$output = (<$child>);
-	close $child or die join(' ',@_).": $! $?";
-    } else {
-	exec(@_) or die "$! $?"; # exec() can fail the executable can't be found
-    }
-    return $output;
+	my $output;
+	if (my $pid = open my $child, '-|') {
+		local $/;
+		undef $/;
+		$output = (<$child>);
+		close $child or die join(' ',@_).": $! $?";
+	} else {
+		exec(@_) or die "$! $?"; # exec() can fail the executable can't be found
+	}
+	return $output;
 }
-- 
1.5.3.4.207.g6d7480-dirty

[PATCH 2/3] git-cvsexportcommit.perl tmpdir removed

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
    This is perhaps a duplicate of another patch sent in,
    but it applies to the tabified version.
    
 git-cvsexportcommit.perl |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index b330c3d..869c8b5 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -30,9 +30,6 @@ if ($opt_d) {
 		@cvs = ('cvs');
 }
 
-# setup a tempdir
-our ($tmpdir, $tmpdirname) = tempdir('git-cvsapplycommit-XXXXXX', TMPDIR => 1, CLEANUP => 1);
-
 # resolve target commit
 my $commit;
 $commit = pop @ARGV;
-- 
1.5.3.4.207.g6d7480-dirty

[PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Michael Witten]

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
 git-cvsexportcommit.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 869c8b5..22c3d2f 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -129,7 +129,7 @@ my $context = $opt_p ? '' : '-C1';
 print "Checking if patch will apply\n";
 
 my @stat;
-open APPLY, "GIT_DIR= git-apply $context --binary --summary --numstat<.cvsexportcommit.diff|" || die "cannot patch";
+open APPLY, "GIT_DIR= git-apply $context --summary --numstat<.cvsexportcommit.diff|" || die "cannot patch";
 @stat=<APPLY>;
 close APPLY || die "Cannot patch";
 my (@bfiles,@files,@afiles,@dfiles);
@@ -215,7 +215,7 @@ if ($dirty) {
 }
 
 print "Applying\n";
-`GIT_DIR= git-apply $context --binary --summary --numstat --apply <.cvsexportcommit.diff` || die "cannot patch";
+`GIT_DIR= git-apply $context --summary --numstat --apply <.cvsexportcommit.diff` || die "cannot patch";
 
 print "Patch applied successfully. Adding new files and directories to CVS\n";
 my $dirtypatch = 0;
-- 
1.5.3.4.207.g6d7480-dirty

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Johannes Schindelin]

Hi,

does --binary hurt?

Ciao,
Dscho

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Michael Witten]

On 16 Oct 2007, at 9:04:34 AM, Johannes Schindelin wrote:

> Hi,
>
> does --binary hurt?

It's a no op according to the documentation.

In my experience, the healthier the trees are,
the healthier the forest is.

Michael Witten

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Robin Rosenberg]

tisdag 16 oktober 2007 skrev Michael Witten:
> 
> On 16 Oct 2007, at 9:04:34 AM, Johannes Schindelin wrote:
> 
> > Hi,
> >
> > does --binary hurt?
> 
> It's a no op according to the documentation.
> 
> In my experience, the healthier the trees are,
> the healthier the forest is.
> 
> Michael Witten

So all this series does is... making it harder to follow the history?

Ack for removing the --binary, the rest is just noise, and the remove tempdir
patch should be attributed to Johannes since he wrote the patch (ack to that one
too, which he wrote and submitted exactly two minutes ahead of me, just enough
for me not to submit it.).

-- robin
 

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Michael Witten]

On 16 Oct 2007, at 5:20:14 PM, Robin Rosenberg wrote:

> So all this series does is... making it harder to follow the history?

If you follow the history solely on patches.


> Ack for removing the --binary, the rest is just noise

I think fixing the tabs is more important than removing --binary.

It's clear the the entropy of tabulation increases over time;
the tab patch acts as a buffer to reconstruct a clean signal.

Michael Witten

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Johannes Schindelin]

Hi,

On Tue, 16 Oct 2007, Michael Witten wrote:

> On 16 Oct 2007, at 5:20:14 PM, Robin Rosenberg wrote:
> 
> > So all this series does is... making it harder to follow the history?
> 
> If you follow the history solely on patches.
> 
> 
> > Ack for removing the --binary, the rest is just noise
> 
> I think fixing the tabs is more important than removing --binary.
> 
> It's clear the the entropy of tabulation increases over time;
> the tab patch acts as a buffer to reconstruct a clean signal.

Git.  The most advanced TCM - Tab Code Manager.  It's not about source, 
it's about tabs.

;-)

Ciao,
Dscho

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Michael Witten]

On 16 Oct 2007, at 6:49:17 PM, Johannes Schindelin wrote:

>> I think fixing the tabs is more important than removing --binary.
>>
>> It's clear the the entropy of tabulation increases over time;
>> the tab patch acts as a buffer to reconstruct a clean signal.
>
> Git.  The most advanced TCM - Tab Code Manager.  It's not about  
> source,
> it's about tabs.
>
> ;-)

I don't know whether to laugh or be miffed.

I think I'll choose to laugh. ;-D

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Shawn O. Pearce]

Michael Witten <mfwitten@MIT.EDU> wrote:
> On 16 Oct 2007, at 5:20:14 PM, Robin Rosenberg wrote:
> 
> >So all this series does is... making it harder to follow the history?
> 
> If you follow the history solely on patches.

`git-blame -w` can probably punch through the indentation change
just fine to find the older history.  But it does make `git log -p`
damn ugly to read at this point in history.  And if you forget the
-w to git-blame, well, then you are really in for some fun for a
few minutes.  Lets not mention pickaxe noticing strings removed+added
in this patch either.

> >Ack for removing the --binary, the rest is just noise
> 
> I think fixing the tabs is more important than removing --binary.
> 
> It's clear the the entropy of tabulation increases over time;
> the tab patch acts as a buffer to reconstruct a clean signal.

Sorry, but I have to say I agree with Robin here.  The tab patch
is large, ugly, and provides relatively little value in comparsion.

The first rule of git development typically is "any change is bad".
Because anything that is already written can be assumed to already
be tested and in use by someone.  Breaking that is bad, as then
they have a bad experience with git.

There needs to be a really good reason behind a change, like the
existing code is already not functioning according to its documented
behavior due to a corner case input.  Such things should be changed.

I'm not going to apply the indentation change patch to my tree.
You can try to resubmit it through Junio after he's back online
and accepting patches.

-- 
Shawn.

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Michael Witten]

On 16 Oct 2007, at 9:11:48 PM, Shawn O. Pearce wrote:

> Sorry, but I have to say I agree with Robin here.  The tab patch
> is large, ugly, and provides relatively little value in comparsion.
>
> The first rule of git development typically is "any change is bad".
> Because anything that is already written can be assumed to already
> be tested and in use by someone.  Breaking that is bad, as then
> they have a bad experience with git.

OK. I understand.

This was really preparation for a more substantial addition
that I hope to make.

I'll just have to change one line at a time with every substantial
patch!

mfwitten

Re: [PATCH 2/3] git-cvsexportcommit.perl tmpdir removed

[Shawn O. Pearce]

Michael Witten <mfwitten@mit.edu> wrote:
> Signed-off-by: Michael Witten <mfwitten@mit.edu>
> ---
>     This is perhaps a duplicate of another patch sent in,
>     but it applies to the tabified version.

Yea, I don't know if you know this but I do already have this in my
master branch.  Same change but it came from Johannes Schindelin.
I just happened to come across his patch first in my inbox last
night when I was processing through what I could.
     
-- 
Shawn.

Re: [PATCH 3/3] git-cvsexportcommit.perl: git-apply no longer needs --binary

[Robin Rosenberg]

onsdag 17 oktober 2007 skrev Michael Witten:
> 
> On 16 Oct 2007, at 9:11:48 PM, Shawn O. Pearce wrote:
> 
> > Sorry, but I have to say I agree with Robin here.  The tab patch
> > is large, ugly, and provides relatively little value in comparsion.
> >
> > The first rule of git development typically is "any change is bad".
> > Because anything that is already written can be assumed to already
> > be tested and in use by someone.  Breaking that is bad, as then
> > they have a bad experience with git.
> 
> OK. I understand.
> 
> This was really preparation for a more substantial addition
> that I hope to make.

That might warrant fixing the tabs, *then*. It is nice to know that this 
debate did not scare you awya from submitting patches. 

> I'll just have to change one line at a time with every substantial
> patch!

Exactly :)
I wrote some more in a commet on the On tabs and spaces thread.

-- robin