Re: git-svn added files to my commit?!

[cc'ing the list in case others are seeing similar, etc]

Eric Wong [off-list ref] writes:

Seth Falcon [off-list ref] wrote:

quoted

Hi Eric,

I just used git-svn commit to commit a change back to an svn
repository and a few files made it into the commit that didn't appear
in the git diff output.  This resulted in a "dirty" commit to the svn
repository.  Perhaps it is user error, but I would appreciate it if
you could take a quick look.

Here is the log for my git-svn branch:

   $ git log --stat remotes/git-svn
   commit 6a376fa49bb5e9917c6272d61c6ee0769674da34
   Merge: c40e1e6... e4a5813...
   Author: sethf <sethf@bc3139a8-67e5-0310-9ffc-ced21a209358>
   Date:   Sat Aug 5 18:02:01 2006 +0000
   
       Fixed bug in certain vectorized uses of removeEdge
       
       Thanks to Denise Scholtens for reporting.
       
       
       git-svn-id: https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph@190
   commit e4a581328e194d545eb2e6a0e61f55c6d8ae56b8
   Author: Seth Falcon [off-list ref]
   Date:   Sat Aug 5 11:00:59 2006 -0700
   
       Fixed bug in certain vectorized uses of removeEdge
       
       Thanks to Denise Scholtens for reporting.
   
    DESCRIPTION                    |    2 +-
    R/graphNEL.R                   |   23 ++++++++++++++++-------
    inst/unitTests/graphNEL_test.R |   40 +++++++++++++++++++++++++++++++++++++++-
    3 files changed, 56 insertions(+), 9 deletions(-)

git-svn log has:

   ------------------------------------------------------------------------
   r19045 | sethf | 2006-08-05 11:02:01 -0700 (Sat, 05 Aug 2006) | 4 lines
   
   Fixed bug in certain vectorized uses of removeEdge
   
   Thanks to Denise Scholtens for reporting.

Ok, looks good.  But in an updated svn working copy, I get:

svn log -v:
    ------------------------------------------------------------------------
    r19045 | sethf | 2006-08-05 11:02:01 -0700 (Sat, 05 Aug 2006) | 4 lines
    Changed paths:
       M /trunk/madman/Rpacks/graph/DESCRIPTION
       M /trunk/madman/Rpacks/graph/R/graphNEL.R
       M /trunk/madman/Rpacks/graph/inst/GXL/complexExample.gxl
       M /trunk/madman/Rpacks/graph/inst/GXL/simplExample.gxl.www
       M /trunk/madman/Rpacks/graph/inst/GXL/simpleExample.gxl
       M /trunk/madman/Rpacks/graph/inst/doc/GraphClass.tex
       M /trunk/madman/Rpacks/graph/inst/doc/GraphDesign.tex
       M /trunk/madman/Rpacks/graph/inst/unitTests/graphNEL_test.R
    
    Fixed bug in certain vectorized uses of removeEdge
    
    Thanks to Denise Scholtens for reporting.

Here is where you can see that a number of files "snuck" into the
commit.  When I looked at the diffs, it seemed to be line ending
changes only, but I was very surprised because 'git diff
remotes/git-svn..master' didn't show these, so I wasn't expecting them
to be part of the commit.

That's very strange, git diff not showing anything suspicious.  The
modified files have eol-style:native set on them, so it could be an
issue with git-svn's eol-style handling.  From the info below, I assume
you're _not_ using the SVN:: libraries, correct?

Right, I haven't installed any Perl libs out of laziness.  What are
the advantages of using the SVN:: libs?

Looks like a bug, might as well cc the ml as it's a public repo.  I had
issues accessing it with the SVN:: libraries, though (probably a
permissions handling issue with git-svn) and had to use the
command-line.

The repos is public, but you need --username=readonly
--password=readonly.

I think this is related to git-svn eol handling.  I tried the
following experiment:

Create a fresh git-svn repos along with a vanilla svn working copy.
Then diff.  I get differences in eol and I think there should be no
differences.

 ## create svn working copy
 svn co \
   https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph graph-svn
 cd graph-svn
 svn update -r18986

 ## create git-svn repos
 cd ~/
 mkdir graph-git
 cd graph-git
 git-svn init https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/graph
 git-svn fetch -r18300:18986
 git checkout master
 git diff remotes/git-svn..master  ## no output, as expected

 diff -ur inst/doc ~/graph-svn/inst/doc ## lots of output, unexpected

quoted

git-svn --version
git-svn version 1.4.2.rc3.g7e18e

git --version
git version 1.4.2.rc3.g7e18e

svn --version
svn, version 1.3.1 (r19032)

OSX 10.4.7

+ seth

Sorry for the delayed response; and furthermore, I've barely slept the
past few days, so don't expect full coherency.

Seth Falcon [off-list ref] wrote:

[cc'ing the list in case others are seeing similar, etc]

I think this is related to git-svn eol handling.

It is... git-svn is issuing 'svn revert' before running 'svn up' which
means it's changing newlines between git commits.

I can't remember exactly why manual eol handling was needed in git-svn,
but I think it had to do with disabling keyword expansion and copying
from the original text-base files.  Of course, the 'svn revert' command
is there for a reason, too (looking at the comment in git-svn).

Perhaps getting rid of keyword killing and accepting expansions with the
command-line client is the simplest and safest way to go.

I've been using the SVN:: libraries exclusively for a while now and all
seems well, save for initial checkouts or big commits which are
significantly slower in some cases (I think using SVN::Delta for
downloads should fix this).

-- 
Eric Wong

This bugfix applies to users of the svn command-line client only.

We no longer muck with newlines when killing keyword expansion.
This tended to generate unintended diffs in commits because svn
revert -R would destroy the manual EOL changes we were doing. Of
course, we didn't need the EOL munging in the first place, as
svn seems to do it for us even in the text-base files.

Now we set the mtime and atime the files changed by keyword
expansion killing to avoid triggering a change on svn revert,
which svn still seems to want to do.

Thanks to Seth Falcon for reporting this bug.

Signed-off-by: Eric Wong <redacted>
---
 git-svn.perl |   45 ++++-----------------------------------------
 1 files changed, 4 insertions(+), 41 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 6453771..3327ad3 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -31,6 +31,7 @@ use File::Basename qw/dirname basename/;
 use File::Path qw/mkpath/;
 use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev pass_through/;
 use File::Spec qw//;
+use File::Copy qw/copy/;
 use POSIX qw/strftime/;
 use IPC::Open3;
 use Memoize;

@@ -77,9 +78,6 @@ my %cmt_opts = ( 'edit|e' => \$_edit,
 		'copy-similarity|C=i'=> \$_cp_similarity
 );
 
-# yes, 'native' sets "\n".  Patches to fix this for non-*nix systems welcome:
-my %EOL = ( CR => "\015", LF => "\012", CRLF => "\015\012", native => "\012" );
-
 my %cmd = (
 	fetch => [ \&fetch, "Download new revisions from SVN",
 			{ 'revision|r=s' => \$_revision, %fc_opts } ],

@@ -1760,43 +1758,6 @@ sub svn_info {
 
 sub sys { system(@_) == 0 or croak $? }
 
-sub eol_cp {
-	my ($from, $to) = @_;
-	my $es = svn_propget_base('svn:eol-style', $to);
-	open my $rfd, '<', $from or croak $!;
-	binmode $rfd or croak $!;
-	open my $wfd, '>', $to or croak $!;
-	binmode $wfd or croak $!;
-	eol_cp_fd($rfd, $wfd, $es);
-	close $rfd or croak $!;
-	close $wfd or croak $!;
-}
-
-sub eol_cp_fd {
-	my ($rfd, $wfd, $es) = @_;
-	my $eol = defined $es ? $EOL{$es} : undef;
-	my $buf;
-	use bytes;
-	while (1) {
-		my ($r, $w, $t);
-		defined($r = sysread($rfd, $buf, 4096)) or croak $!;
-		return unless $r;
-		if ($eol) {
-			if ($buf =~ /\015$/) {
-				my $c;
-				defined($r = sysread($rfd,$c,1)) or croak $!;
-				$buf .= $c if $r > 0;
-			}
-			$buf =~ s/(?:\015\012|\015|\012)/$eol/gs;
-			$r = length($buf);
-		}
-		for ($w = 0; $w < $r; $w += $t) {
-			$t = syswrite($wfd, $buf, $r - $w, $w) or croak $!;
-		}
-	}
-	no bytes;
-}
-
 sub do_update_index {
 	my ($z_cmd, $cmd, $no_text_base) = @_;
 

@@ -1824,9 +1785,11 @@ sub do_update_index {
 						'text-base',"$f.svn-base");
 				$tb =~ s#^/##;
 			}
+			my @s = stat($x);
 			unlink $x or croak $!;
-			eol_cp($tb, $x);
+			copy($tb, $x);
 			chmod(($mode &~ umask), $x) or croak $!;
+			utime $s[8], $s[9], $x;
 		}
 		print $ui $x,"\0";
 	}

-- 
1.4.2.rc1.g018f

Seth Falcon [off-list ref] wrote:

Eric Wong [off-list ref] writes:

quoted

Looks like a bug, might as well cc the ml as it's a public repo.  I had
issues accessing it with the SVN:: libraries, though (probably a
permissions handling issue with git-svn) and had to use the
command-line.

The repos is public, but you need --username=readonly
--password=readonly.

I've fixed one bug wrt to having limited permissions to a repository
when using the SVN:: libraries, but I can't seem to $ra->get_log() to
play nicely with limited read permissions.  Interesting, I'll be playing
with this sometime this weekend if I can maintain a stable internet
connection.

-- 
Eric Wong