git-svn added files to my commit?!

PATCH New Seth Falcon 2 participants 4 messages Jun 15, 2016 2 messages not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Seth Falcon

author

2016-06-15 22:42 UTC #2 lore raw

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

Quoted 70 lines · Eric Wong

Quoted 61 lines · Seth Falcon
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 <[email protected]>
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
Quoted 7 lines
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.

Quoted 4 lines

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?

Quoted 4 lines

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

Quoted 10 lines
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

Eric Wong

2016-06-15 22:42 UTC #3 lore raw

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

Quoted 3 lines · Seth Falcon

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

Signature · 1 line

Eric Wong

Eric Wong

2016-06-15 22:42 UTC #4 lore raw

PATCH 1/1 git-svn: correctly kill keyword expansion without munging EOLs

In reply to Seth Falcon

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 <[email protected]>

---
 git-svn.perl |   45 ++++-----------------------------------------
 1 files changed, 4 insertions(+), 41 deletions(-)

git-svn.perl +4 ~~-41~~

index 6453771..3327ad3 100755
@@ -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";
 	}

Signature · 1 line

1.4.2.rc1.g018f

Eric Wong

2016-06-15 22:42 UTC #5 lore raw

In reply to Seth Falcon

Quoted 8 lines · Seth Falcon

Quoted 4 lines · Eric Wong
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.

Signature · 1 line

Eric Wong