| Message ID | 1467670a-a9eb-f230-4ec0-bd6d54411a69@lpl-mind.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-svn: Skip commit if all items of a commit are ignored by ignore configuration | expand |
Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de> wrote: > > I used the ignore-paths option to ignore a lot of stuff I don’t need. The > ignore pattern works well, but it could and up in empty commits. So just the > message without any modifications / changes. The patch below skip a commit > if all changes are ignored by the ignore-paths option. Hi Lukas, this seems like an incompatible change, but it also matches the intent of the user if they use ignore-paths (which AFAIK is a rarely-used feature). I guess it's OK to make it the default behavior, but maybe others have objections... > Signed-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de> Thanks :> More comments inline... > diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm > index 56ad9870bc..a5a6c0b774 100644 > --- a/perl/Git/SVN/Ra.pm > +++ b/perl/Git/SVN/Ra.pm > @@ -457,13 +457,22 @@ sub gs_fetch_loop_common { > $find_trailing_edge = 0; > } > $SVN::Error::handler = $err_handler; > - > + This patch (and your other) are both white-space damaged, but there doesn't need to be a whitespace change, there. I'm not sure which mail client you use, but https://git-send-email.io/ documents git-send-email as being shipped with Git-for-Windows. Personally, I'm fine with attachments if they can go through without whitespace damage (not speaking for the rest of git@vger). You can test by emailing yourself and trying "git am" on it. > my %exists = map { $_->path => $_ } @$gsv; > foreach my $r (sort {$a <=> $b} keys %revs) { > my ($paths, $logged) = @{delete $revs{$r}}; > - Again, no extraneous whitespace changes, please. > foreach my $gs ($self->match_globs(\%exists, $paths, > $globs, $r)) { > + > + > + my $fetcher=Git::SVN::Fetcher->new($gs); Initializing Git::SVN::Fetcher here and closing it is an expensive operation since it calls git-config(1) many times. This is especially bad for most users who do not use --ignore-paths at all. I know git-svn isn't fast, but I've been hoping to find spare time to make it use fast-import and speed it up, eventually. Instead, I think it's possible to bail out of Git::SVN::do_fetch and still skip the commit without extra network traffic. I suggest you try that route, instead. > + > + my $skip=$self->is_empty_commit($paths,$fetcher); > + if ($skip){ > + print "skip commit $r\n"; > + next; > + } > + $fetcher->close_edit(); > if ($gs->rev_map_max >= $r) { > next; > } Style: put whitespace between operators for readability and consistency with the rest of our code: my $skip = $self->is_empty_commit($paths, $fetcher); if ($skip) { print "skip commit $r\n"; ... We also use hard tabs for indentation. > @@ -506,6 +517,21 @@ sub gs_fetch_loop_common { > Git::SVN::gc(); > } > > +sub is_empty_commit{ > + my ($self, $paths,$fetcher) = @_; > + my $path=""; > + foreach $path (keys %$paths){ > + unless (defined $path && -d $path ){ The `-d' check is invalid. "git-svn fetch" never touches the working tree. > + my $ignored=$fetcher->is_path_ignored($path); > + if (!$ignored){ > + return 0; > + } > + } > + } > + return 1; > +} > + > + > sub get_dir_globbed { > my ($self, $left, $depth, $r) = @_; Same style comments as before, and a single empty line in-between subs is enough. Thanks.
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 56ad9870bc..a5a6c0b774 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -457,13 +457,22 @@ sub gs_fetch_loop_common { $find_trailing_edge = 0; } $SVN::Error::handler = $err_handler; - + my %exists = map { $_->path => $_ } @$gsv; foreach my $r (sort {$a <=> $b} keys %revs) { my ($paths, $logged) = @{delete $revs{$r}}; - foreach my $gs ($self->match_globs(\%exists, $paths, $globs, $r)) { + + + my $fetcher=Git::SVN::Fetcher->new($gs); + + my $skip=$self->is_empty_commit($paths,$fetcher); + if ($skip){ + print "skip commit $r\n"; + next; + } + $fetcher->close_edit(); if ($gs->rev_map_max >= $r) { next; } @@ -506,6 +517,21 @@ sub gs_fetch_loop_common { Git::SVN::gc(); } +sub is_empty_commit{ + my ($self, $paths,$fetcher) = @_; + my $path=""; + foreach $path (keys %$paths){ + unless (defined $path && -d $path ){ + my $ignored=$fetcher->is_path_ignored($path); + if (!$ignored){ + return 0; + } + } + } + return 1; +} + + sub get_dir_globbed { my ($self, $left, $depth, $r) = @_;
I used the ignore-paths option to ignore a lot of stuff I don’t need. The ignore pattern works well, but it could and up in empty commits. So just the message without any modifications / changes. The patch below skip a commit if all changes are ignored by the ignore-paths option. Signed-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de> --- perl/Git/SVN/Ra.pm | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)