[PATCH] git-svn: add --ignore-paths option for fetching

Signed-off-by: Vitaly "_Vi" Shukela <redacted>
---
 Documentation/git-svn.txt |    4 ++++
 git-svn.perl              |   25 +++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 63d2f5e..4aeb88b 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt

@@ -96,6 +96,10 @@ COMMANDS
 	Store Git commit times in the local timezone instead of UTC.  This
 	makes 'git-log' (even without --date=local) show the same times
 	that `svn log` would in the local timezone.
+--ignore-paths=<regex>;;
+	This allows one to specify regular expression that will
+	cause skipping of all matching paths from checkout from SVN.
+	Example: --ignore-paths='^doc'
 
 This doesn't interfere with interoperating with the Subversion
 repository you cloned from, but if you wish for your local Git

diff --git a/git-svn.perl b/git-svn.perl
index d4cb538..4909b23 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -70,7 +70,8 @@ my ($_stdin, $_help, $_edit,
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
-                    'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache );
+                    'no-auth-cache' => \$Git::SVN::Prompt::_no_auth_cache,
+                    'ignore-paths=s' => \$SVN::Git::Fetcher::ignoreRegex );
 my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
 		'authors-file|A=s' => \$_authors,
 		'repack:i' => \$Git::SVN::_repack,

@@ -3245,6 +3246,15 @@ use warnings;
 use Carp qw/croak/;
 use File::Temp qw/tempfile/;
 use IO::File qw//;
+use vars qw/ $ignoreRegex/;
+
+# 0 -- don't ignore, 1 -- ignore
+sub isPathIgnored($) {
+    return 0 unless defined($ignoreRegex);
+    my $path = shift;
+    return 1 if $path =~ m!^$ignoreRegex!o;
+    return 0;
+}
 
 # file baton members: path, mode_a, mode_b, pool, fh, blob, base
 sub new {

@@ -3323,6 +3333,7 @@ sub git_path {
 sub delete_entry {
 	my ($self, $path, $rev, $pb) = @_;
 	return undef if in_dot_git($path);
+	return undef if isPathIgnored($path);
 
 	my $gpath = $self->git_path($path);
 	return undef if ($gpath eq '');

@@ -3353,6 +3364,7 @@ sub open_file {
 	my ($mode, $blob);
 
 	goto out if in_dot_git($path);
+	goto out if isPathIgnored($path);
 
 	my $gpath = $self->git_path($path);
 	($mode, $blob) = (command('ls-tree', $self->{c}, '--', $gpath)

@@ -3372,11 +3384,14 @@ sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
 	my $mode;
 
+	goto out if isPathIgnored($path);
+
 	if (!in_dot_git($path)) {
 		my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
 		delete $self->{empty}->{$dir};
 		$mode = '100644';
 	}
+out:
 	{ path => $path, mode_a => $mode, mode_b => $mode,
 	  pool => SVN::Pool->new, action => 'A' };
 }

@@ -3384,6 +3399,7 @@ sub add_file {
 sub add_directory {
 	my ($self, $path, $cp_path, $cp_rev) = @_;
 	goto out if in_dot_git($path);
+	goto out if isPathIgnored($path);
 	my $gpath = $self->git_path($path);
 	if ($gpath eq '') {
 		my ($ls, $ctx) = command_output_pipe(qw/ls-tree

@@ -3408,6 +3424,7 @@ out:
 sub change_dir_prop {
 	my ($self, $db, $prop, $value) = @_;
 	return undef if in_dot_git($db->{path});
+	return undef if isPathIgnored($db->{path});
 	$self->{dir_prop}->{$db->{path}} ||= {};
 	$self->{dir_prop}->{$db->{path}}->{$prop} = $value;
 	undef;

@@ -3416,6 +3433,7 @@ sub change_dir_prop {
 sub absent_directory {
 	my ($self, $path, $pb) = @_;
 	return undef if in_dot_git($pb->{path});
+	return undef if isPathIgnored($path);
 	$self->{absent_dir}->{$pb->{path}} ||= [];
 	push @{$self->{absent_dir}->{$pb->{path}}}, $path;
 	undef;

@@ -3424,6 +3442,7 @@ sub absent_directory {
 sub absent_file {
 	my ($self, $path, $pb) = @_;
 	return undef if in_dot_git($pb->{path});
+	return undef if isPathIgnored($path);
 	$self->{absent_file}->{$pb->{path}} ||= [];
 	push @{$self->{absent_file}->{$pb->{path}}}, $path;
 	undef;

@@ -3432,6 +3451,7 @@ sub absent_file {
 sub change_file_prop {
 	my ($self, $fb, $prop, $value) = @_;
 	return undef if in_dot_git($fb->{path});
+	return undef if isPathIgnored($fb->{path});
 	if ($prop eq 'svn:executable') {
 		if ($fb->{mode_b} != 120000) {
 			$fb->{mode_b} = defined $value ? 100755 : 100644;

@@ -3448,6 +3468,7 @@ sub change_file_prop {
 sub apply_textdelta {
 	my ($self, $fb, $exp) = @_;
 	return undef if (in_dot_git($fb->{path}));
+	return undef if isPathIgnored($fb->{path});
 	my $fh = $::_repository->temp_acquire('svn_delta');
 	# $fh gets auto-closed() by SVN::TxDelta::apply(),
 	# (but $base does not,) so dup() it for reading in close_file

@@ -3495,7 +3516,7 @@ sub apply_textdelta {
 sub close_file {
 	my ($self, $fb, $exp) = @_;
 	return undef if (in_dot_git($fb->{path}));
-
+	return undef if isPathIgnored($fb->{path});
 	my $hash;
 	my $path = $self->git_path($fb->{path});
 	if (my $fh = $fb->{fh}) {

-- 
1.5.6.5

Vitaly "_Vi" Shukela wrote:

Signed-off-by: Vitaly "_Vi" Shukela <redacted>

This would be a good place to explain why this is useful, and (if
applicable) why you chose to implement it the way you did.

You put the --ignore-paths explanation in the middle of the
--localtime documentation (the last paragraph quoted still talks about
--localtime).

This is the first function in git-svn.perl using camelCase.  Consider
sticking to the current style and spelling it is_path_ignored().

You broke the symmetry here, while all other hunks just add an
equivalent check to the existing in_dot_git().

However, the latter makes me wonder if it would be cleaner to move the
in_dot_git() test to isPathIgnored (er, is_path_ignored) too?


FWIW, I like the feature; it seems a good way to exclude subtrees with
large blobs, and I know several SVN repos that have such a directory.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

It is useful for me, but I think I can be useful for others too.
May be it's better to make in inclusive, i.e. "--only-matching=<regex>"?

This would be a good place to explain why this is useful, and (if
applicable) why you chose to implement it the way you did.
  

OK, fixing them.

You put the --ignore-paths explanation in the middle of the
--localtime documentation (the last paragraph quoted still talks about
--localtime).
  
This is the first function in git-svn.perl using camelCase.  Consider
sticking to the current style and spelling it is_path_ignored().

You broke the symmetry here, while all other hunks just add an
equivalent check to the existing in_dot_git().

However, the latter makes me wonder if it would be cleaner to move the
in_dot_git() test to isPathIgnored (er, is_path_ignored) too?
  

Thomas Rast [off-list ref] wrote:

Vitaly "_Vi" Shukela wrote:

quoted

Signed-off-by: Vitaly "_Vi" Shukela <redacted>

This would be a good place to explain why this is useful, and (if
applicable) why you chose to implement it the way you did.

quoted

--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt

@@ -96,6 +96,10 @@ COMMANDS
 	Store Git commit times in the local timezone instead of UTC.  This
 	makes 'git-log' (even without --date=local) show the same times
 	that `svn log` would in the local timezone.
+--ignore-paths=<regex>;;
+	This allows one to specify regular expression that will
+	cause skipping of all matching paths from checkout from SVN.
+	Example: --ignore-paths='^doc'
 
 This doesn't interfere with interoperating with the Subversion
 repository you cloned from, but if you wish for your local Git

You put the --ignore-paths explanation in the middle of the
--localtime documentation (the last paragraph quoted still talks about
--localtime).

quoted

@@ -3245,6 +3246,15 @@ use warnings;
 use Carp qw/croak/;
 use File::Temp qw/tempfile/;
 use IO::File qw//;
+use vars qw/ $ignoreRegex/;
+
+# 0 -- don't ignore, 1 -- ignore
+sub isPathIgnored($) {
+    return 0 unless defined($ignoreRegex);
+    my $path = shift;
+    return 1 if $path =~ m!^$ignoreRegex!o;
+    return 0;
+}

This is the first function in git-svn.perl using camelCase.  Consider
sticking to the current style and spelling it is_path_ignored().

Also, indentation is always done with tabs in git-svn (and the vast
majority of git as well).

quoted

@@ -3372,11 +3384,14 @@ sub add_file {
 	my ($self, $path, $pb, $cp_path, $cp_rev) = @_;
 	my $mode;
 
+	goto out if isPathIgnored($path);
+
 	if (!in_dot_git($path)) {
 		my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
 		delete $self->{empty}->{$dir};
 		$mode = '100644';
 	}
+out:
 	{ path => $path, mode_a => $mode, mode_b => $mode,
 	  pool => SVN::Pool->new, action => 'A' };
 }

You broke the symmetry here, while all other hunks just add an
equivalent check to the existing in_dot_git().

However, the latter makes me wonder if it would be cleaner to move the
in_dot_git() test to isPathIgnored (er, is_path_ignored) too?

Thanks for the review, Thomas.  I agree with all your suggestions.

Vitaly: thank you for the patch.  Can you also provide a testcase to
ensure this functionality doesn't break during refactorings?  Thanks.

-- 
Eric Wong

Eric Wong wrote:

Thomas Rast [off-list ref] wrote:
  

Also, indentation is always done with tabs in git-svn (and the vast
majority of git as well).

  

Whitespace is invisible for me now, I only checked according to 
Documentation/SubmittingPatches about extra lines adds or removes caused 
just by whitespace difference.

Vitaly: thank you for the patch.  Can you also provide a testcase to
ensure this functionality doesn't break during refactorings?  Thanks.

  

Already done it with testcase. There are two new packs of patches sent 
to git@vger.kernel.org, the first is outdated too, the second is current.
(I don't yet completely know how things should be done).

public_vi [off-list ref] wrote:

Eric Wong wrote:

quoted

Thomas Rast [off-list ref] wrote:
  

Also, indentation is always done with tabs in git-svn (and the vast
majority of git as well).

  

Whitespace is invisible for me now, I only checked according to  
Documentation/SubmittingPatches about extra lines adds or removes caused  
just by whitespace difference.

quoted

Vitaly: thank you for the patch.  Can you also provide a testcase to
ensure this functionality doesn't break during refactorings?  Thanks.

  

Already done it with testcase. There are two new packs of patches sent  
to git@vger.kernel.org, the first is outdated too, the second is current.
(I don't yet completely know how things should be done).

Thanks Vitaly, acked and pushed out with minor fixes to
git://git.bogomips.org/git-svn.git

git-svn: Add test for --ignore-paths parameter
    [ew: replaced 'echo -e' with printf so it works on POSIX shells]
    [ew: added Vitaly to copyright even though it's based on my test]

git-svn: documented --ignore-paths
    [ew: trailing whitespace removed]

git-svn: add --ignore-paths option for fetching
    [ew: replaced 4-space indent with tabs]
    [ew: prefixed $ignore_regex with an underscore to be consistent
         with other globals in git-svn]
    [ew: rearranged functions to minimize diff and removed prototype
         usage to be consistent with the rest of git-svn (and other
         Perl code in git (and they're ugly to me)]

-- 
Eric Wong