{"thread":{"id":"53109","subject":"git-svn: Skip commit if all items of a commit are ignored by ignore configuration","startedAt":"2020-03-27T08:11:52Z","lastAt":"2020-03-29T23:24:23Z","messageCount":3,"participants":["Lukas Pupka-Lipinski","Eric Wong"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"394156","messageId":"b20ed6ca-1283-fb6d-a00c-94557218e01c@lpl-mind.de","threadId":"53109","inReplyTo":null,"subject":"git-svn: Skip commit if all items of a commit are ignored by ignore configuration","fromName":"Lukas Pupka-Lipinski","fromEmail":"lukas.pupkalipinski@lpl-mind.de","sentAt":"2020-03-27T08:11:41Z","receivedAt":"2020-03-27T08:11:52Z","isPatch":false,"sender":{"key":"lukas.pupkalipinski@lpl-mind.de","avatar":null},"body":"I used the ignore-paths option to ignore a lot of stuff I don’t need. \nThe ignore pattern works well, but it could and up in empty commits. So \njust the message without any modifications / changes. The patch below \nskip a commit if all changes are ignored by the ignore-paths option.\n\n\nSigned-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de>\n\n---\n  perl/Git/SVN/Ra.pm | 32 +++++++++++++++++++++++++++++---\n  1 file changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 56ad9870bc..a5a6c0b774 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -457,13 +457,22 @@ sub gs_fetch_loop_common {\n              $find_trailing_edge = 0;\n          }\n          $SVN::Error::handler = $err_handler;\n-\n+\n          my %exists = map { $_->path => $_ } @$gsv;\n          foreach my $r (sort {$a <=> $b} keys %revs) {\n              my ($paths, $logged) = @{delete $revs{$r}};\n-\n              foreach my $gs ($self->match_globs(\\%exists, $paths,\n                                                 $globs, $r)) {\n+\n+\n+                my $fetcher=Git::SVN::Fetcher->new($gs);\n+\n+                my $skip=$self->is_empty_commit($paths,$fetcher);\n+                if ($skip){\n+                    print \"skip commit $r\\n\";\n+                    next;\n+                }\n+                $fetcher->close_edit();\n                  if ($gs->rev_map_max >= $r) {\n                      next;\n                  }\n@@ -506,6 +517,21 @@ sub gs_fetch_loop_common {\nGit::SVN::gc();\n  }\n\n+sub is_empty_commit{\n+    my ($self, $paths,$fetcher) = @_;\n+    my $path=\"\";\n+    foreach $path (keys %$paths){\n+        unless (defined $path && -d $path ){\n+            my $ignored=$fetcher->is_path_ignored($path);\n+            if (!$ignored){\n+                return 0;\n+            }\n+        }\n+    }\n+    return 1;\n+}\n+\n+\n  sub get_dir_globbed {\n      my ($self, $left, $depth, $r) = @_;\n\n-- \n2.25.1.windows.1\n\n"},{"id":"394157","messageId":"1467670a-a9eb-f230-4ec0-bd6d54411a69@lpl-mind.de","threadId":"53109","inReplyTo":"b20ed6ca-1283-fb6d-a00c-94557218e01c@lpl-mind.de","subject":"git-svn: Skip commit if all items of a commit are ignored by ignore configuration","fromName":"Lukas Pupka-Lipinski","fromEmail":"lukas.pupkalipinski@lpl-mind.de","sentAt":"2020-03-27T08:13:24Z","receivedAt":"2020-03-27T08:13:33Z","isPatch":false,"sender":{"key":"lukas.pupkalipinski@lpl-mind.de","avatar":null},"body":"\nI used the ignore-paths option to ignore a lot of stuff I don’t need. \nThe ignore pattern works well, but it could and up in empty commits. So \njust the message without any modifications / changes. The patch below \nskip a commit if all changes are ignored by the ignore-paths option.\n\n\nSigned-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de>\n\n---\n  perl/Git/SVN/Ra.pm | 32 +++++++++++++++++++++++++++++---\n  1 file changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 56ad9870bc..a5a6c0b774 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -457,13 +457,22 @@ sub gs_fetch_loop_common {\n              $find_trailing_edge = 0;\n          }\n          $SVN::Error::handler = $err_handler;\n-\n+\n          my %exists = map { $_->path => $_ } @$gsv;\n          foreach my $r (sort {$a <=> $b} keys %revs) {\n              my ($paths, $logged) = @{delete $revs{$r}};\n-\n              foreach my $gs ($self->match_globs(\\%exists, $paths,\n                                                 $globs, $r)) {\n+\n+\n+                my $fetcher=Git::SVN::Fetcher->new($gs);\n+\n+                my $skip=$self->is_empty_commit($paths,$fetcher);\n+                if ($skip){\n+                    print \"skip commit $r\\n\";\n+                    next;\n+                }\n+                $fetcher->close_edit();\n                  if ($gs->rev_map_max >= $r) {\n                      next;\n                  }\n@@ -506,6 +517,21 @@ sub gs_fetch_loop_common {\nGit::SVN::gc();\n  }\n\n+sub is_empty_commit{\n+    my ($self, $paths,$fetcher) = @_;\n+    my $path=\"\";\n+    foreach $path (keys %$paths){\n+        unless (defined $path && -d $path ){\n+            my $ignored=$fetcher->is_path_ignored($path);\n+            if (!$ignored){\n+                return 0;\n+            }\n+        }\n+    }\n+    return 1;\n+}\n+\n+\n  sub get_dir_globbed {\n      my ($self, $left, $depth, $r) = @_;\n\n-- \n2.25.1.windows.1\n\n"},{"id":"394301","messageId":"20200329232414.GA27925@dcvr","threadId":"53109","inReplyTo":"1467670a-a9eb-f230-4ec0-bd6d54411a69@lpl-mind.de","subject":"Re: git-svn: Skip commit if all items of a commit are ignored by ignore configuration","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2020-03-29T23:24:14Z","receivedAt":"2020-03-29T23:24:23Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de> wrote:\n> \n> I used the ignore-paths option to ignore a lot of stuff I don’t need. The\n> ignore pattern works well, but it could and up in empty commits. So just the\n> message without any modifications / changes. The patch below skip a commit\n> if all changes are ignored by the ignore-paths option.\n\nHi Lukas, this seems like an incompatible change, but it also\nmatches the intent of the user if they use ignore-paths (which\nAFAIK is a rarely-used feature).\n\nI guess it's OK to make it the default behavior, but maybe\nothers have objections...\n\n> Signed-off-by: Lukas Pupka-Lipinski <lukas.pupkalipinski@lpl-mind.de>\n\nThanks :>  More comments inline...\n\n> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n> index 56ad9870bc..a5a6c0b774 100644\n> --- a/perl/Git/SVN/Ra.pm\n> +++ b/perl/Git/SVN/Ra.pm\n> @@ -457,13 +457,22 @@ sub gs_fetch_loop_common {\n>              $find_trailing_edge = 0;\n>          }\n>          $SVN::Error::handler = $err_handler;\n> -\n> +\n\nThis patch (and your other) are both white-space damaged,\nbut there doesn't need to be a whitespace change, there.\n\nI'm not sure which mail client you use, but\nhttps://git-send-email.io/ documents git-send-email\nas being shipped with Git-for-Windows.\n\nPersonally, I'm fine with attachments if they can go through\nwithout whitespace damage (not speaking for the rest of\ngit@vger).  You can test by emailing yourself and trying\n\"git am\" on it.\n\n>          my %exists = map { $_->path => $_ } @$gsv;\n>          foreach my $r (sort {$a <=> $b} keys %revs) {\n>              my ($paths, $logged) = @{delete $revs{$r}};\n> -\n\nAgain, no extraneous whitespace changes, please.\n\n>              foreach my $gs ($self->match_globs(\\%exists, $paths,\n>                                                 $globs, $r)) {\n> +\n> +\n> +                my $fetcher=Git::SVN::Fetcher->new($gs);\n\nInitializing Git::SVN::Fetcher here and closing it is an\nexpensive operation since it calls git-config(1) many times.\nThis is especially bad for most users who do not use\n--ignore-paths at all.\n\nI know git-svn isn't fast, but I've been hoping to find spare\ntime to make it use fast-import and speed it up, eventually.\n\nInstead, I think it's possible to bail out of Git::SVN::do_fetch\nand still skip the commit without extra network traffic.  I\nsuggest you try that route, instead.\n\n> +\n> +                my $skip=$self->is_empty_commit($paths,$fetcher);\n> +                if ($skip){\n> +                    print \"skip commit $r\\n\";\n> +                    next;\n> +                }\n> +                $fetcher->close_edit();\n>                  if ($gs->rev_map_max >= $r) {\n>                      next;\n>                  }\n\nStyle: put whitespace between operators for readability\nand consistency with the rest of our code:\n\n\tmy $skip = $self->is_empty_commit($paths, $fetcher);\n        if ($skip) {\n\t\tprint \"skip commit $r\\n\";\n\t\t...\n\nWe also use hard tabs for indentation.\n\n> @@ -506,6 +517,21 @@ sub gs_fetch_loop_common {\n> Git::SVN::gc();\n>  }\n> \n> +sub is_empty_commit{\n> +    my ($self, $paths,$fetcher) = @_;\n> +    my $path=\"\";\n> +    foreach $path (keys %$paths){\n> +        unless (defined $path && -d $path ){\n\nThe `-d' check is invalid.  \"git-svn fetch\" never touches the\nworking tree.\n\n> +            my $ignored=$fetcher->is_path_ignored($path);\n> +            if (!$ignored){\n> +                return 0;\n> +            }\n> +        }\n> +    }\n> +    return 1;\n> +}\n> +\n> +\n>  sub get_dir_globbed {\n>      my ($self, $left, $depth, $r) = @_;\n\nSame style comments as before, and a single empty line\nin-between subs is enough.  Thanks.\n"}]}