chiark / gitweb /
git-debrebase: Rewrite get_differs in terms of trees_diff_walk
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Sun, 19 Aug 2018 16:49:17 +0000 (17:49 +0100)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Sun, 19 Aug 2018 16:52:27 +0000 (17:52 +0100)
As noted, git-diff-tree is quite slow.  We can do much better, by
stepping across the two trees ourselves.

There should be no particularly significant change: the answers should
be the same for reasonable inputs.  Unreasonable inputs (particularly,
executable files etc.) may be handled better, even.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
debian/changelog
git-debrebase

index 58b934e..40a880e 100644 (file)
@@ -8,6 +8,7 @@ dgit (6.7~) unstable; urgency=medium
   * dgit(1): Encourage --overwrite rather than --overwrite=version.
   * test suite: unset GIT_EDITOR, so it works if user has that set.
   * git-debrebase: Improve laundry performance.  Closes:#905995.
+  * git-debrebase: Improve analysis performance.
 
  --
 
index 57bfa98..61507ea 100755 (executable)
@@ -249,70 +249,75 @@ sub trees_diff_walk ($$$;$) {
 
 sub get_differs ($$) {
     my ($x,$y) = @_;
-    # This resembles quiltify_trees_differ, in dgit, a bit.
+    # This does a similar job to quiltify_trees_differ, in dgit, a bit.
     # But we don't care about modes, or dpkg-source-unrepresentable
     # changes, and we don't need the plethora of different modes.
     # Conversely we need to distinguish different kinds of changes to
     # debian/ and debian/patches/.
+    # Also, here we have, and want to use, trees_diff_walk, because
+    # we may be calling this an awful lot and we want it to be fast.
 
     my $differs = 0;
+    my @debian_info;
 
-    my $rundiff = sub {
-       my ($opts, $limits, $fn) = @_;
-       my @cmd = (@git, qw(diff-tree -z --no-renames));
-       push @cmd, @$opts;
-       push @cmd, "$_:" foreach $x, $y;
-       push @cmd, '--', @$limits;
-       my $diffs = cmdoutput @cmd;
-       foreach (split /\0/, $diffs) { $fn->(); }
-    };
+    no warnings qw(exiting);
 
-    $rundiff->([qw(--name-only)], [], sub {
-        $differs |= $_ eq 'debian' ? DS_DEB : D_UPS;
-    });
+    my $plain = sub { $_[0] =~ m{^(100|0*)644 blob }s; };
 
-    if ($differs & DS_DEB) {
-       $differs &= ~DS_DEB;
-       $rundiff->([qw(--name-only -r)], [qw(debian)], sub {
-            $differs |=
-               m{^debian/patches/}      ? D_PAT_OTH  :
-               $_ eq 'debian/changelog' ? D_DEB_CLOG :
-                                          D_DEB_OTH;
-       });
-       die "mysterious debian changes $x..$y"
-           unless $differs & (D_PAT_OTH|DS_DEB);
-    }
-
-    if ($differs & D_PAT_OTH) {
-       my $mode;
-       $differs &= ~D_PAT_OTH;
-       my $pat_oth = sub {
-           $differs |= D_PAT_OTH;
-           no warnings qw(exiting);  last;
-       };
-       $rundiff->([qw(--name-status -r)], [qw(debian/patches/)], sub {
-            no warnings qw(exiting);
-            if (!defined $mode) {
-               $mode = $_;  next;
+    trees_diff_walk "$x:", "$y:", sub {
+       my ($n,$ix,$iy) = @_;
+
+       # analyse difference at the toplevel
+
+       if ($n ne 'debian/') {
+           $differs |= D_UPS;
+           next;
+       }
+       if ($n eq 'debian') {
+           # one side has a non-tree for ./debian !
+           $differs |= D_DEB_OTH;
+           next;
+       }
+
+       my $xd = $ix && "$x:debian";
+       my $yd = $iy && "$y:debian";
+       trees_diff_walk $xd, $yd, sub {
+           my ($n,$ix,$iy) = @_;
+           
+           # analyse difference in debian/
+
+           if ($n eq 'changelog' && (!$ix || $plain->($ix))
+                                  &&          $plain->($iy) ) {
+               $differs |= D_DEB_CLOG;
+               next;
            }
-           die unless s{^debian/patches/}{};
-           my $ok;
-           if ($mode eq 'A' && !m/\.series$/s) {
-               $ok = 1;
-           } elsif ($mode eq 'M' && $_ eq 'series') {
-               my $x_s = (git_cat_file "$x:debian/patches/series", 'blob');
-               my $y_s = (git_cat_file "$y:debian/patches/series", 'blob');
-               chomp $x_s;  $x_s .= "\n";
-               $ok = $x_s eq substr($y_s, 0, length $x_s);
-           } else {
-               # nope
+           if ($n ne 'patches/') {
+               $differs |= D_DEB_OTH;
+               next;
            }
-           $mode = undef;
-           $differs |= $ok ? D_PAT_ADD : D_PAT_OTH;
-        });
-       die "mysterious debian/patches changes $x..$y"
-           unless $differs & (D_PAT_ADD|D_PAT_OTH);
-    }
+
+           my $xp = $ix && "$xd/patches";
+           my $yp = $iy && "$yd/patches";
+           trees_diff_walk $xp, $yp, sub {
+               my ($n,$ix,$iy) = @_;
+
+               # analyse difference in debian/patches
+
+               my $ok;
+               if ($n !~ m/\.series$/s && !$ix && $plain->($iy)) {
+                   $ok = 1;
+               } elsif ($n eq 'series' && $plain->($ix) && $plain->($iy)) {
+                   my $x_s = (git_cat_file "$xp/series", 'blob');
+                   my $y_s = (git_cat_file "$yp/series", 'blob');
+                   chomp $x_s;  $x_s .= "\n";
+                   $ok = $x_s eq substr($y_s, 0, length $x_s);
+               } else {
+                   # nope
+               }
+               $differs |= $ok ? D_PAT_ADD : D_PAT_OTH;
+           };    
+       };
+    };
 
     printdebug sprintf "get_differs %s %s = %#x\n", $x, $y, $differs;