gpush/gpick: try to preserve group ids

a subsequent commit will require the gids to stay stable.

it also improves the optics, making debugging somewhat easier:
  git diff refs/gpush/state@{1}..refs/gpush/state
now returns something that actually looks sane in most cases.

if we actually versioned the state file, this would also save space, but
as there is no need for versioning beyond what the reflog provides, this
is kinda moot.

Change-Id: If222c4763527bcfe3629bc181339176de095db39
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

2 files changed, 37 insertions, 5 deletions

diff --git a/bin/git-gpush b/bin/git-gpush
index c4aa762..53299b9 100755
--- a/bin/git-gpush
+++ b/bin/git-gpush
@@ -1300,13 +1300,12 @@ sub define_series($)
 {
     my ($commits) = @_;
 
-    # A new group key is used if a series is being (re-)assigned;
-    # for simplicity, we do not bother preserving keys.
     # Stealing Changes from existing series could dissolve these series
     # entirely. This implementation does not do this for simplicity's
     # sake, thus losing some expressiveness - but that might have been
     # more annoying than helpful anyway.
-    return (changes_from_commits($commits), $next_group++);
+    my $changes = changes_from_commits($commits);
+    return ($changes, obtain_gid($changes));
 }
 
 sub determine_series($)
diff --git a/bin/git_gpush.pm b/bin/git_gpush.pm
index 62cb362..8895d4b 100644
--- a/bin/git_gpush.pm
+++ b/bin/git_gpush.pm
@@ -850,7 +850,7 @@ our %changes_by_id;  # { gerrit-id => [ change-object, ... ] }
 # Note that other local branches may have gpick'd the same commits.
 our %change_by_pushed;  # { sha1 => change-object }
 
-our $next_group = 10000;
+my $next_group = 10000;
 
 my $next_prop = 10000;
 # Indirect Change properties, which are shared to save space.
@@ -1599,11 +1599,44 @@ sub parse_local_rev($$)
 # smart series #
 ################
 
+sub obtain_gid($)
+{
+    my ($changes) = @_;
+
+    # First collect the group ids appearing within the set.
+    my %gid_hash;
+    foreach my $change (@$changes) {
+        my $gid = $$change{grp};
+        $gid_hash{$gid} = 1 if (defined($gid));
+    }
+    if (%gid_hash) {
+        # Then collect all Changes with these group ids ...
+        my %orphans;
+        foreach my $change (values %change_by_key) {
+            my $gid = $$change{grp};
+            $orphans{$gid}{$$change{key}} = $change
+                if (defined($gid) && defined($gid_hash{$gid}));
+        }
+        # ... and subtract the ones we're currently grouping.
+        foreach my $change (@$changes) {
+            my $gid = $$change{grp};
+            delete $orphans{$gid}{$$change{key}}
+                if (defined($gid));
+        }
+        foreach my $gid (sort keys %gid_hash) {
+            # If there are no orphans, we are enlarging the group without
+            # dropping any Changes. Then we can recycle the id.
+            return $gid if (!%{$orphans{$gid}});
+        }
+    }
+    return $next_group++;
+}
+
 sub assign_series($)
 {
     my ($changes) = @_;
 
-    my $gid = $next_group++;
+    my $gid = obtain_gid($changes);
     $$_{grp} = $gid foreach (@$changes);
 }