* [PATCH]v2 GitSVN: Multi line support of ignore-path, include-paths and skiping of empty commits
@ 2020-05-02 9:29 Lukas Pupka-Lipinski
2020-06-24 3:16 ` Eric Wong
0 siblings, 1 reply; 2+ messages in thread
From: Lukas Pupka-Lipinski @ 2020-05-02 9:29 UTC (permalink / raw)
To: Eric Wong; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 4659 bytes --]
Hi Eric,
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. In
order to use this feature I includes the option to read configuration
for ignore-path, include-paths in several lines. So that the user is not
limited by the max. char. per line definition.
In Addition this patch includes the optimizations which are mansion from
your side.
Signed-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de>
---
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 4b28b87784..fa87687306 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1188,6 +1188,22 @@ sub find_parent_branch {
return undef;
}
+############################################################
+
+=item do_fetch()
+
+Fetch an Commit and returns a log entry
+
+Input: $path - array of strings (Paths) in a commit
+ $rev - Revision number
+
+Output: $log_entry if successfull
+ null if skipped
+ (die) on fetch error
+
+=cut
+
+############################################################
sub do_fetch {
my ($self, $paths, $rev) = @_;
my $ed;
@@ -1212,6 +1228,11 @@ sub do_fetch {
}
$ed = Git::SVN::Fetcher->new($self);
}
+ my $skip = $ed->is_empty_commit($paths);
+ if ($skip){
+ print "skip commit $rev\n";
+ return;
+ }
unless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {
die "SVN connection failed somewhere...\n";
}
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 64e900a0e9..c414aa879f 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -31,15 +31,17 @@ sub new {
# override options set in an [svn-remote "..."] section
$repo_id = $git_svn->{repo_id};
my $k = "svn-remote.$repo_id.ignore-paths";
- my $v = eval { command_oneline('config', '--get', $k) };
- $self->{ignore_regex} = $v;
+ my @config = eval { command('config', '--get-all', $k) };
+ chomp(@config); # Replace all \n\r on the end
+ $self->{ignore_regex} = join('', @config);
$k = "svn-remote.$repo_id.include-paths";
- $v = eval { command_oneline('config', '--get', $k) };
- $self->{include_regex} = $v;
+ @config = eval { command_oneline('config', '--get-all', $k) };
+ chomp(@config); # Replace all \n\r on the end
+ $self->{include_regex} = join('', @config);
$k = "svn-remote.$repo_id.preserve-empty-dirs";
- $v = eval { command_oneline('config', '--get', '--bool', $k) };
+ my $v = eval { command_oneline('config', '--get', '--bool', $k) };
if ($v && $v eq 'true') {
$_preserve_empty_dirs = 1;
$k = "svn-remote.$repo_id.placeholder-filename";
@@ -137,6 +139,37 @@ sub is_path_ignored {
return 0;
}
+############################################################
+
+=item is_empty_commit()
+
+Return 1 if all given $paths are ignored, so that this commit end up in
an empty commit
+
+Input: $path - array of strings (Paths) in a commit
+
+Output: { 1 if true, 0 if false }
+
+=cut
+
+############################################################
+sub is_empty_commit{
+ my ($self, $paths) = @_;
+ my $path = "";
+ unless (defined($self->{include_regex})){
+ return 0;
+ }
+
+ foreach $path (keys %$paths){
+ unless (defined $path && -d $path ){
+ my $ignored = $self->is_path_ignored($path);
+ if (!$ignored){
+ return 0;
+ }
+ }
+ }
+ return 1;
+}
+
sub set_path_strip {
my ($self, $path) = @_;
$self->{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path;
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 56ad9870bc..63be69dc12 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -475,6 +475,8 @@ sub gs_fetch_loop_common {
my $log_entry = $gs->do_fetch($paths, $r);
if ($log_entry) {
$gs->do_git_commit($log_entry);
+ }else{
+ next;
}
$Git::SVN::INDEX_FILES{$gs->{index}} = 1;
}
[-- Attachment #2: Git-SVN-EmptyCommits.diff --]
[-- Type: text/plain, Size: 3290 bytes --]
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 4b28b87784..fa87687306 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1188,6 +1188,22 @@ sub find_parent_branch {
return undef;
}
+############################################################
+
+=item do_fetch()
+
+Fetch an Commit and returns a log entry
+
+Input: $path - array of strings (Paths) in a commit
+ $rev - Revision number
+
+Output: $log_entry if successfull
+ null if skipped
+ (die) on fetch error
+
+=cut
+
+############################################################
sub do_fetch {
my ($self, $paths, $rev) = @_;
my $ed;
@@ -1212,6 +1228,11 @@ sub do_fetch {
}
$ed = Git::SVN::Fetcher->new($self);
}
+ my $skip = $ed->is_empty_commit($paths);
+ if ($skip){
+ print "skip commit $rev\n";
+ return;
+ }
unless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {
die "SVN connection failed somewhere...\n";
}
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 64e900a0e9..c414aa879f 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -31,15 +31,17 @@ sub new {
# override options set in an [svn-remote "..."] section
$repo_id = $git_svn->{repo_id};
my $k = "svn-remote.$repo_id.ignore-paths";
- my $v = eval { command_oneline('config', '--get', $k) };
- $self->{ignore_regex} = $v;
+ my @config = eval { command('config', '--get-all', $k) };
+ chomp(@config); # Replace all \n\r on the end
+ $self->{ignore_regex} = join('', @config);
$k = "svn-remote.$repo_id.include-paths";
- $v = eval { command_oneline('config', '--get', $k) };
- $self->{include_regex} = $v;
+ @config = eval { command_oneline('config', '--get-all', $k) };
+ chomp(@config); # Replace all \n\r on the end
+ $self->{include_regex} = join('', @config);
$k = "svn-remote.$repo_id.preserve-empty-dirs";
- $v = eval { command_oneline('config', '--get', '--bool', $k) };
+ my $v = eval { command_oneline('config', '--get', '--bool', $k) };
if ($v && $v eq 'true') {
$_preserve_empty_dirs = 1;
$k = "svn-remote.$repo_id.placeholder-filename";
@@ -137,6 +139,37 @@ sub is_path_ignored {
return 0;
}
+############################################################
+
+=item is_empty_commit()
+
+Return 1 if all given $paths are ignored, so that this commit end up in an empty commit
+
+Input: $path - array of strings (Paths) in a commit
+
+Output: { 1 if true, 0 if false }
+
+=cut
+
+############################################################
+sub is_empty_commit{
+ my ($self, $paths) = @_;
+ my $path = "";
+ unless (defined($self->{include_regex})){
+ return 0;
+ }
+
+ foreach $path (keys %$paths){
+ unless (defined $path && -d $path ){
+ my $ignored = $self->is_path_ignored($path);
+ if (!$ignored){
+ return 0;
+ }
+ }
+ }
+ return 1;
+}
+
sub set_path_strip {
my ($self, $path) = @_;
$self->{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path;
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 56ad9870bc..63be69dc12 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -475,6 +475,8 @@ sub gs_fetch_loop_common {
my $log_entry = $gs->do_fetch($paths, $r);
if ($log_entry) {
$gs->do_git_commit($log_entry);
+ }else{
+ next;
}
$Git::SVN::INDEX_FILES{$gs->{index}} = 1;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH]v2 GitSVN: Multi line support of ignore-path, include-paths and skiping of empty commits
2020-05-02 9:29 [PATCH]v2 GitSVN: Multi line support of ignore-path, include-paths and skiping of empty commits Lukas Pupka-Lipinski
@ 2020-06-24 3:16 ` Eric Wong
0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2020-06-24 3:16 UTC (permalink / raw)
To: Lukas Pupka-Lipinski; +Cc: git
Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de> wrote:
> Hi Eric,
>
> 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. In order to use this
> feature I includes the option to read configuration for ignore-path,
> include-paths in several lines. So that the user is not limited by the max.
> char. per line definition.
> In Addition this patch includes the optimizations which are mansion from
> your side.
Hi Lukas, sorry for the delay.
The above is not a proper commit message. Please see
Documentation/SubmittingPatches and git.git history
for some examples. I or someone else here can help you
write it if you need help.
Comments inline.
> Signed-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de>
I'm skipping the whitespace damaged inline version and
reviewing the the attachment.
> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> index 4b28b87784..fa87687306 100644
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
> @@ -1188,6 +1188,22 @@ sub find_parent_branch {
> return undef;
> }
>
> +############################################################
> +
> +=item do_fetch()
> +
> +Fetch an Commit and returns a log entry
> +
> +Input: $path - array of strings (Paths) in a commit
> + $rev - Revision number
> +
> +Output: $log_entry if successfull
> + null if skipped
> + (die) on fetch error
> +
> +=cut
> +
> +############################################################
Documentation is good, but the above takes up more than half my
terminal. A single line is more than sufficient.
(I have poor eyesight and need very big fonts).
> sub do_fetch {
> my ($self, $paths, $rev) = @_;
> my $ed;
> @@ -1212,6 +1228,11 @@ sub do_fetch {
> }
> $ed = Git::SVN::Fetcher->new($self);
> }
> + my $skip = $ed->is_empty_commit($paths);
> + if ($skip){
> + print "skip commit $rev\n";
I think "skipping" is a better word to use, here (I'm not an
English expert, either). To be unambiguous, we prefix SVN
revision numbers with "r" (just like SVN does):
print "skipping commit r$rev\n";
> + return;
> + }
> unless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {
> die "SVN connection failed somewhere...\n";
> }
> diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
> index 64e900a0e9..c414aa879f 100644
> --- a/perl/Git/SVN/Fetcher.pm
> +++ b/perl/Git/SVN/Fetcher.pm
> @@ -31,15 +31,17 @@ sub new {
> # override options set in an [svn-remote "..."] section
> $repo_id = $git_svn->{repo_id};
> my $k = "svn-remote.$repo_id.ignore-paths";
> - my $v = eval { command_oneline('config', '--get', $k) };
> - $self->{ignore_regex} = $v;
> + my @config = eval { command('config', '--get-all', $k) };
> + chomp(@config); # Replace all \n\r on the end
> + $self->{ignore_regex} = join('', @config);
That join() doesn't make a correct regexp with multiple values
in @config. I think you would need the following to make
a proper regexp:
$self->{ignore_regex} = '(?:'.join('|', @config).')';
(totally untested, please test with multiple values)
> $k = "svn-remote.$repo_id.include-paths";
> - $v = eval { command_oneline('config', '--get', $k) };
> - $self->{include_regex} = $v;
> + @config = eval { command_oneline('config', '--get-all', $k) };
> + chomp(@config); # Replace all \n\r on the end
> + $self->{include_regex} = join('', @config);
Same comment as above w.r.t multi-value @config
> $k = "svn-remote.$repo_id.preserve-empty-dirs";
> - $v = eval { command_oneline('config', '--get', '--bool', $k) };
> + my $v = eval { command_oneline('config', '--get', '--bool', $k) };
> if ($v && $v eq 'true') {
> $_preserve_empty_dirs = 1;
> $k = "svn-remote.$repo_id.placeholder-filename";
> @@ -137,6 +139,37 @@ sub is_path_ignored {
> return 0;
> }
>
> +############################################################
> +
> +=item is_empty_commit()
> +
> +Return 1 if all given $paths are ignored, so that this commit end up in an empty commit
> +
> +Input: $path - array of strings (Paths) in a commit
> +
> +Output: { 1 if true, 0 if false }
> +
> +=cut
> +
> +############################################################
Same comment w.r.t. excessive screen space for comments.
> +sub is_empty_commit{
> + my ($self, $paths) = @_;
> + my $path = "";
Don't need to initialize $path, here...
> + unless (defined($self->{include_regex})){
> + return 0;
> + }
> +
> + foreach $path (keys %$paths){
Instead, localize variables within the loop:
foreach my $path (keys %$paths) {
Also, both our Perl and C code prefer a space in-between "){",
so ") {", everywhere, and no need to have " )" when ")" will
do (same goes for the rest of the codebase).
> + unless (defined $path && -d $path ){
It's been a while since I looked at this code; but
Git::SVN::Fetcher should NEVER be comparing paths
from SVN vs paths from the filesystem via `-d' or
similar stat() ops.
In other words, "git svn fetch" never touches the
git working tree, it should work on bare git repos.
Can the just next 4 lines work w/o the unless wrapping it?
> + my $ignored = $self->is_path_ignored($path);
> + if (!$ignored){
> + return 0;
> + }
> + }
> + }
> + return 1;
> +}
> +
> sub set_path_strip {
> my ($self, $path) = @_;
> $self->{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path;
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 56ad9870bc..63be69dc12 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -475,6 +475,8 @@ sub gs_fetch_loop_common {
> my $log_entry = $gs->do_fetch($paths, $r);
> if ($log_entry) {
> $gs->do_git_commit($log_entry);
> + }else{
Again, in both C and Perl, we write "} else {"
> + next;
> }
> $Git::SVN::INDEX_FILES{$gs->{index}} = 1;
> }
Thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-06-24 3:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-02 9:29 [PATCH]v2 GitSVN: Multi line support of ignore-path, include-paths and skiping of empty commits Lukas Pupka-Lipinski
2020-06-24 3:16 ` Eric Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).