From: Eric Wong Date: Mon, 20 Feb 2006 18:57:28 +0000 (-0800) Subject: git-svn: fix several corner-case and rare bugs with 'commit' X-Git-Tag: v1.3.0-rc1~182 X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=cf52b8f06389189bd32565c5c6adad75ac8a1a62;p=git.git git-svn: fix several corner-case and rare bugs with 'commit' None of these were really show-stoppers (or even triggered) on most of the trees I've tracked. * Node change prevention for identically named nodes. This is a limitation of SVN, but we find the error and exit before it's passed to SVN so we don't dirty our working tree when our commit fails. git-svn will exit with an error code 1 if any of the following conditions are found: 1. a directory is removed and a file of the same name of the removed directory is created 1a. a file has its parent directory removed and the file is takes the name of the removed parent directory:: baz/zzz => baz 2. a file is removed and a directory of the same name of the removed file is created. 2a. a file is moved into a deeper directory that shares the previous name of the file:: dir/$file => dir/file/$file Since SVN cannot handle these cases, the user will have to manually split the commit into several parts. * --rmdir now handles nested/deep removals. If dir/a/b/c/d/e/file is removed, and everything else is in the dir/ hierarchy is otherwise empty, then dir/ will be deleted when file is deleted from svn and --rmdir specified. * Always assert that we have written the tree we want to write on commits. This helped me find several bugs in the symlink handling code (which as been fixed). * Several symlink handling fixes. We now refuse to set permissions on symlinks. We also always unlink a file if we're going to overwrite it. * Apply changes in a pre-determined order, so we always have rename from locations handy before we delete them. Signed-off-by: Eric Wong Signed-off-by: Junio C Hamano --- diff --git a/contrib/git-svn/git-svn b/contrib/git-svn/git-svn index 25c248dee..3a5945490 100755 --- a/contrib/git-svn/git-svn +++ b/contrib/git-svn/git-svn @@ -238,7 +238,11 @@ sub commit { my $svn_current_rev = svn_info('.')->{'Last Changed Rev'}; foreach my $c (@revs) { print "Committing $c\n"; - svn_checkout_tree($svn_current_rev, $c); + my $mods = svn_checkout_tree($svn_current_rev, $c); + if (scalar @$mods == 0) { + print "Skipping, no changes detected\n"; + next; + } $svn_current_rev = svn_commit_tree($svn_current_rev, $c); } print "Done committing ",scalar @revs," revisions to SVN\n"; @@ -267,9 +271,9 @@ sub setup_git_svn { } sub assert_svn_wc_clean { - my ($svn_rev, $commit) = @_; + my ($svn_rev, $treeish) = @_; croak "$svn_rev is not an integer!\n" unless ($svn_rev =~ /^\d+$/); - croak "$commit is not a sha1!\n" unless ($commit =~ /^$sha1$/o); + croak "$treeish is not a sha1!\n" unless ($treeish =~ /^$sha1$/o); my $svn_info = svn_info('.'); if ($svn_rev != $svn_info->{'Last Changed Rev'}) { croak "Expected r$svn_rev, got r", @@ -282,12 +286,42 @@ sub assert_svn_wc_clean { print STDERR $_ foreach @status; croak; } - my ($tree_a) = grep(/^tree $sha1$/o,`git-cat-file commit $commit`); - $tree_a =~ s/^tree //; - chomp $tree_a; - chomp(my $tree_b = `GIT_INDEX_FILE=$GIT_SVN_INDEX git-write-tree`); - if ($tree_a ne $tree_b) { - croak "$svn_rev != $commit, $tree_a != $tree_b\n"; + assert_tree($treeish); +} + +sub assert_tree { + my ($treeish) = @_; + croak "Not a sha1: $treeish\n" unless $treeish =~ /^$sha1$/o; + chomp(my $type = `git-cat-file -t $treeish`); + my $expected; + while ($type eq 'tag') { + chomp(($treeish, $type) = `git-cat-file tag $treeish`); + } + if ($type eq 'commit') { + $expected = (grep /^tree /,`git-cat-file commit $treeish`)[0]; + ($expected) = ($expected =~ /^tree ($sha1)$/); + die "Unable to get tree from $treeish\n" unless $expected; + } elsif ($type eq 'tree') { + $expected = $treeish; + } else { + die "$treeish is a $type, expected tree, tag or commit\n"; + } + + my $old_index = $ENV{GIT_INDEX_FILE}; + my $tmpindex = $GIT_SVN_INDEX.'.assert-tmp'; + if (-e $tmpindex) { + unlink $tmpindex or croak $!; + } + $ENV{GIT_INDEX_FILE} = $tmpindex; + git_addremove(); + chomp(my $tree = `git-write-tree`); + if ($old_index) { + $ENV{GIT_INDEX_FILE} = $old_index; + } else { + delete $ENV{GIT_INDEX_FILE}; + } + if ($tree ne $expected) { + croak "Tree mismatch, Got: $tree, Expected: $expected\n"; } } @@ -298,7 +332,6 @@ sub parse_diff_tree { my @mods; while (<$diff_fh>) { chomp $_; # this gets rid of the trailing "\0" - print $_,"\n"; if ($state eq 'meta' && /^:(\d{6})\s(\d{6})\s $sha1\s($sha1)\s([MTCRAD])\d*$/xo) { push @mods, { mode_a => $1, mode_b => $2, @@ -309,36 +342,44 @@ sub parse_diff_tree { $state = 'file_b'; } } elsif ($state eq 'file_a') { - my $x = $mods[$#mods] or croak __LINE__,": Empty array\n"; + my $x = $mods[$#mods] or croak "Empty array\n"; if ($x->{chg} !~ /^(?:C|R)$/) { - croak __LINE__,": Error parsing $_, $x->{chg}\n"; + croak "Error parsing $_, $x->{chg}\n"; } $x->{file_a} = $_; $state = 'file_b'; } elsif ($state eq 'file_b') { - my $x = $mods[$#mods] or croak __LINE__,": Empty array\n"; + my $x = $mods[$#mods] or croak "Empty array\n"; if (exists $x->{file_a} && $x->{chg} !~ /^(?:C|R)$/) { - croak __LINE__,": Error parsing $_, $x->{chg}\n"; + croak "Error parsing $_, $x->{chg}\n"; } if (!exists $x->{file_a} && $x->{chg} =~ /^(?:C|R)$/) { - croak __LINE__,": Error parsing $_, $x->{chg}\n"; + croak "Error parsing $_, $x->{chg}\n"; } $x->{file_b} = $_; $state = 'meta'; } else { - croak __LINE__,": Error parsing $_\n"; + croak "Error parsing $_\n"; } } close $diff_fh or croak $!; + return \@mods; } sub svn_check_prop_executable { my $m = shift; - if ($m->{mode_b} =~ /755$/ && $m->{mode_a} !~ /755$/) { - sys(qw(svn propset svn:executable 1), $m->{file_b}); + return if -l $m->{file_b}; + if ($m->{mode_b} =~ /755$/) { + chmod((0755 &~ umask),$m->{file_b}) or croak $!; + if ($m->{mode_a} !~ /755$/) { + sys(qw(svn propset svn:executable 1), $m->{file_b}); + } + -x $m->{file_b} or croak "$m->{file_b} is not executable!\n"; } elsif ($m->{mode_b} !~ /755$/ && $m->{mode_a} =~ /755$/) { sys(qw(svn propdel svn:executable), $m->{file_b}); + chmod((0644 &~ umask),$m->{file_b}) or croak $!; + -x $m->{file_b} and croak "$m->{file_b} is executable!\n"; } } @@ -349,84 +390,166 @@ sub svn_ensure_parent_path { sys(qw(svn add -N), $dir_b) unless (-d "$dir_b/.svn"); } +sub precommit_check { + my $mods = shift; + my (%rm_file, %rmdir_check, %added_check); + + my %o = ( D => 0, R => 1, C => 2, A => 3, M => 3, T => 3 ); + foreach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) { + if ($m->{chg} eq 'R') { + if (-d $m->{file_b}) { + err_dir_to_file("$m->{file_a} => $m->{file_b}"); + } + # dir/$file => dir/file/$file + my $dirname = dirname($m->{file_b}); + while ($dirname ne File::Spec->curdir) { + if ($dirname ne $m->{file_a}) { + $dirname = dirname($dirname); + next; + } + err_file_to_dir("$m->{file_a} => $m->{file_b}"); + } + # baz/zzz => baz (baz is a file) + $dirname = dirname($m->{file_a}); + while ($dirname ne File::Spec->curdir) { + if ($dirname ne $m->{file_b}) { + $dirname = dirname($dirname); + next; + } + err_dir_to_file("$m->{file_a} => $m->{file_b}"); + } + } + if ($m->{chg} =~ /^(D|R)$/) { + my $t = $1 eq 'D' ? 'file_b' : 'file_a'; + $rm_file{ $m->{$t} } = 1; + my $dirname = dirname( $m->{$t} ); + my $basename = basename( $m->{$t} ); + $rmdir_check{$dirname}->{$basename} = 1; + } elsif ($m->{chg} =~ /^(?:A|C)$/) { + if (-d $m->{file_b}) { + err_dir_to_file($m->{file_b}); + } + my $dirname = dirname( $m->{file_b} ); + my $basename = basename( $m->{file_b} ); + $added_check{$dirname}->{$basename} = 1; + while ($dirname ne File::Spec->curdir) { + if ($rm_file{$dirname}) { + err_file_to_dir($m->{file_b}); + } + $dirname = dirname $dirname; + } + } + } + return (\%rmdir_check, \%added_check); + + sub err_dir_to_file { + my $file = shift; + print STDERR "Node change from directory to file ", + "is not supported by Subversion: ",$file,"\n"; + exit 1; + } + sub err_file_to_dir { + my $file = shift; + print STDERR "Node change from file to directory ", + "is not supported by Subversion: ",$file,"\n"; + exit 1; + } +} + sub svn_checkout_tree { - my ($svn_rev, $commit) = @_; + my ($svn_rev, $treeish) = @_; my $from = file_to_s("$REV_DIR/$svn_rev"); assert_svn_wc_clean($svn_rev,$from); - print "diff-tree '$from' '$commit'\n"; + print "diff-tree '$from' '$treeish'\n"; my $pid = open my $diff_fh, '-|'; defined $pid or croak $!; if ($pid == 0) { my @diff_tree = qw(git-diff-tree -z -r -C); push @diff_tree, '--find-copies-harder' if $_find_copies_harder; push @diff_tree, "-l$_l" if defined $_l; - exec(@diff_tree, $from, $commit) or croak $!; + exec(@diff_tree, $from, $treeish) or croak $!; } my $mods = parse_diff_tree($diff_fh); unless (@$mods) { # git can do empty commits, SVN doesn't allow it... - return $svn_rev; + return $mods; } - my %rm; - foreach my $m (@$mods) { + my ($rm, $add) = precommit_check($mods); + + my %o = ( D => 1, R => 0, C => -1, A => 3, M => 3, T => 3 ); + foreach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) { if ($m->{chg} eq 'C') { svn_ensure_parent_path( $m->{file_b} ); sys(qw(svn cp), $m->{file_a}, $m->{file_b}); - blob_to_file( $m->{sha1_b}, $m->{file_b}); + apply_mod_line_blob($m); svn_check_prop_executable($m); } elsif ($m->{chg} eq 'D') { - $rm{dirname $m->{file_b}}->{basename $m->{file_b}} = 1; sys(qw(svn rm --force), $m->{file_b}); } elsif ($m->{chg} eq 'R') { svn_ensure_parent_path( $m->{file_b} ); sys(qw(svn mv --force), $m->{file_a}, $m->{file_b}); - blob_to_file( $m->{sha1_b}, $m->{file_b}); + apply_mod_line_blob($m); svn_check_prop_executable($m); - $rm{dirname $m->{file_a}}->{basename $m->{file_a}} = 1; } elsif ($m->{chg} eq 'M') { - if ($m->{mode_b} =~ /^120/ && $m->{mode_a} =~ /^120/) { - unlink $m->{file_b} or croak $!; - blob_to_symlink($m->{sha1_b}, $m->{file_b}); - } else { - blob_to_file($m->{sha1_b}, $m->{file_b}); - } + apply_mod_line_blob($m); svn_check_prop_executable($m); } elsif ($m->{chg} eq 'T') { sys(qw(svn rm --force),$m->{file_b}); - if ($m->{mode_b} =~ /^120/ && $m->{mode_a} =~ /^100/) { - blob_to_symlink($m->{sha1_b}, $m->{file_b}); - } else { - blob_to_file($m->{sha1_b}, $m->{file_b}); - } - svn_check_prop_executable($m); + apply_mod_line_blob($m); sys(qw(svn add --force), $m->{file_b}); + svn_check_prop_executable($m); } elsif ($m->{chg} eq 'A') { svn_ensure_parent_path( $m->{file_b} ); - blob_to_file( $m->{sha1_b}, $m->{file_b}); - if ($m->{mode_b} =~ /755$/) { - chmod 0755, $m->{file_b}; - } + apply_mod_line_blob($m); sys(qw(svn add --force), $m->{file_b}); + svn_check_prop_executable($m); } else { croak "Invalid chg: $m->{chg}\n"; } } - if ($_rmdir) { - my $old_index = $ENV{GIT_INDEX_FILE}; - $ENV{GIT_INDEX_FILE} = $GIT_SVN_INDEX; - foreach my $dir (keys %rm) { - my $files = $rm{$dir}; - my @files; - foreach (safe_qx('svn','ls',$dir)) { - chomp; - push @files, $_ unless $files->{$_}; - } - sys(qw(svn rm),$dir) unless @files; - } - if ($old_index) { - $ENV{GIT_INDEX_FILE} = $old_index; - } else { - delete $ENV{GIT_INDEX_FILE}; + + assert_tree($treeish); + if ($_rmdir) { # remove empty directories + handle_rmdir($rm, $add); + } + assert_tree($treeish); + return $mods; +} + +# svn ls doesn't work with respect to the current working tree, but what's +# in the repository. There's not even an option for it... *sigh* +# (added files don't show up and removed files remain in the ls listing) +sub svn_ls_current { + my ($dir, $rm, $add) = @_; + chomp(my @ls = safe_qx('svn','ls',$dir)); + my @ret = (); + foreach (@ls) { + s#/$##; # trailing slashes are evil + push @ret, $_ unless $rm->{$dir}->{$_}; + } + if (exists $add->{$dir}) { + push @ret, keys %{$add->{$dir}}; + } + return \@ret; +} + +sub handle_rmdir { + my ($rm, $add) = @_; + + foreach my $dir (sort {length $b <=> length $a} keys %$rm) { + my $ls = svn_ls_current($dir, $rm, $add); + next if (scalar @$ls); + sys(qw(svn rm --force),$dir); + + my $dn = dirname $dir; + $rm->{ $dn }->{ basename $dir } = 1; + $ls = svn_ls_current($dn, $rm, $add); + while (scalar @$ls == 0 && $dn ne File::Spec->curdir) { + sys(qw(svn rm --force),$dn); + $dir = basename $dn; + $dn = dirname $dn; + $rm->{ $dn }->{ $dir } = 1; + $ls = svn_ls_current($dn, $rm, $add); } } } @@ -692,10 +815,23 @@ sub git_commit { return $commit; } +sub apply_mod_line_blob { + my $m = shift; + if ($m->{mode_b} =~ /^120/) { + blob_to_symlink($m->{sha1_b}, $m->{file_b}); + } else { + blob_to_file($m->{sha1_b}, $m->{file_b}); + } +} + sub blob_to_symlink { my ($blob, $link) = @_; defined $link or croak "\$link not defined!\n"; croak "Not a sha1: $blob\n" unless $blob =~ /^$sha1$/o; + if (-l $link || -f _) { + unlink $link or croak $!; + } + my $dest = `git-cat-file blob $blob`; # no newline, so no chomp symlink $dest, $link or croak $!; } @@ -704,6 +840,10 @@ sub blob_to_file { my ($blob, $file) = @_; defined $file or croak "\$file not defined!\n"; croak "Not a sha1: $blob\n" unless $blob =~ /^$sha1$/o; + if (-l $file || -f _) { + unlink $file or croak $!; + } + open my $blob_fh, '>', $file or croak "$!: $file\n"; my $pid = fork; defined $pid or croak $!;