gpush/gpick: introduce infra for indirect properties

rather than having the same (potentially big) value in each Change it
was assigned to, duplicate only a small id.

an arguably cleaner way to approach this would be saving actual group
properties rather than aggregating them on the fly from Change
properties. but when i implemented this, it turned out that we'd have to
be able to resolve conflicts resulting from merging groups in rather
inopportune places: gpick calls assign_series() from deduce_series()
(though a conflict occurring due to this is a rather academical case)
and from _source_map_finish_initial() via analyze_local_branch(). this
means that we'd have to add the property-overriding command line options
to gpick (which is ugly) or we'd have to croak in gpick and tell the
user to use gpush first instead (which is also ugly).

more broadly, group properties aren't a very good fit for gerrit, where
groups aren't even a thing. this is why we don't use them in the first
place.

Change-Id: I555360de0fc08b698213e56045ca408c1ee47851
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

2 files changed, 51 insertions, 0 deletions

diff --git a/bin/git-gpush b/bin/git-gpush
index 7230c5f..56bc5db 100755
--- a/bin/git-gpush
+++ b/bin/git-gpush
@@ -769,6 +769,26 @@ sub aggregate_bool_property($$$$$)
     fail_formatted(\@reports);
 }
 
+sub aggregate_indirect_property($$$)
+{
+    my ($group, $get_key, $get_err) = @_;
+
+    my $changes = $$group{changes};
+    my %key_map = map { $_ => 1 } grep { defined($_) } map { $get_key->() } @$changes;
+    my @pkeys = sort keys %key_map;
+    return if (!@pkeys);
+    # Again a hash, as different keys could still lead to the same value.
+    my %prop_map = map { ($prop_by_key{$_} // "") => 1 } @pkeys;
+    my @props = sort keys %prop_map;
+    return $pkeys[0] if (@props == 1);
+
+    my @reports;
+    report_fixed(\@reports, "Series of ".int(@$changes)." Changes:\n");
+    report_local_changes(\@reports, $changes);
+    report_fixed(\@reports, $get_err->(@props));
+    fail_formatted(\@reports);
+}
+
 # Find _the_ branch the specified commit lives on. This can be the current
 # branch (and other branches are ignored), or _one_ other branch.
 sub branch_for_commit($)
diff --git a/bin/git_gpush.pm b/bin/git_gpush.pm
index 7a23c0b..dce7400 100644
--- a/bin/git_gpush.pm
+++ b/bin/git_gpush.pm
@@ -746,11 +746,30 @@ our %change_by_pushed;  # { sha1 => change-object }
 
 our $next_group = 10000;
 
+my $next_prop = 10000;
+# Indirect Change properties, which are shared to save space.
+# An alternative would be group poperties, but this would require
+# resolving inconsistencies at inopportune times; in particular,
+# we do not want to concern gpick with this at all.
+our %prop_by_key;
+
 our $last_gc = 0;
 
 my $state_lines;
 my $state_updater = ($0 =~ s,^.*/,,r)." @ARGV";
 
+sub new_prop($)
+{
+    my ($val) = @_;
+
+    my $key;
+    if (length($val)) {
+        $key = $next_prop++;
+        $prop_by_key{$key} = $val;
+    }
+    return $key;
+}
+
 # Perform a batch update of refs.
 sub update_refs($$)
 {
@@ -786,7 +805,9 @@ sub save_state(;$$)
     my ($dry, $new) = @_;
 
     print "Saving ".($new ? "new " : "")."state".($dry ? " [DRY]" : "")." ...\n" if ($debug);
+    my %prop_hash;
     my (@lines, @updates);
+    my %ikeys = map { $_ => 1 } ();
     my @fkeys = ('key', 'grp', 'id', 'base', 'src', 'tgt',
                  'topic', 'ver',
                  'nbase', 'ntgt', 'ntopic', 'exclude', 'hide');
@@ -799,6 +820,7 @@ sub save_state(;$$)
     push @lines,
         "next_key $next_key",
         "next_group $next_group",
+        "next_prop $next_prop",
         "last_gc $last_gc",
         "";
     foreach my $key (sort keys %change_by_key) {
@@ -823,10 +845,12 @@ sub save_state(;$$)
                 # We assume that no property ever contains a literal "".
                 $val = '""' if (!length($val));
                 push @lines, "$ky $val";
+                $prop_hash{int($val)} = 1 if (defined($ikeys{$ky}));
             }
         }
         push @lines, "";
     }
+    push @lines, map { $_.'='.$prop_by_key{$_} } sort keys %prop_hash;
     foreach my $ginfo (values %gerrit_info_by_key) {
         my $fetched = $$ginfo{fetched};
         next if (!defined($fetched));
@@ -894,6 +918,11 @@ sub load_state_file(;$)
         if (!length($_)) {
             $inhdr = 0;
             $change = undef;
+        } elsif (/^(\d+)=(.*)/) {
+            my ($key, $val) = (int($1), $2);
+            fail("Bad state file: indirect property with duplicate id at line $line.\n")
+                if (defined($prop_by_key{$key}));
+            $prop_by_key{$key} = $val;
         } elsif (!/^(\w+) (.*)/) {
             fail("Bad state file: Malformed entry at line $line.\n");
         } elsif ($inhdr) {
@@ -901,6 +930,8 @@ sub load_state_file(;$)
                 $next_key = int($2);
             } elsif ($1 eq "next_group") {
                 $next_group = int($2);
+            } elsif ($1 eq "next_prop") {
+                $next_prop = int($2);
             } elsif ($1 eq "last_gc") {
                 $last_gc = int($2);
             } elsif ($new && ($1 eq "verify")) {