Merge changes I105f85bd,I4768e35b

* changes:
  Support copying of approvals if new patch set has no code changes
  Support copying of approvals on trivial rebase

Documentation/config-labels.txt

@@ -236,6 +236,30 @@ forward when a new patch set is uploaded. This can be used to enable
 sticky approvals, reducing turn-around for trivial cleanups prior to
 submitting a change.
 
+[[label_copyAllScoresOnTrivialRebase]]
+`label.Label-Name.copyAllScoresOnTrivialRebase`
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If true, all scores for the label are copied forward when a new patch
+set is uploaded that is a trivial rebase. A new patch set is considered
+as trivial rebase if the commit message is the same as in the previous
+patch set and if it has the same code delta as the previous patch set.
+This is the case if the change was rebased onto a different parent.
+This can be used to enable sticky approvals, reducing turn-around for
+trivial rebases prior to submitting a change. Defaults to false.
+
+[[label_copyAllScoresIfNoCodeChange]]
+`label.Label-Name.copyAllScoresIfNoCodeChange`
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If true, all scores for the label are copied forward when a new patch
+set is uploaded that has the same parent commit as the previous patch
+set and the same code delta as the previous patch set. This means only
+the commit message is different. This can be used to enable sticky
+approvals on labels that only depend on the code, reducing turn-around
+if only the commit message is changed prior to submitting a change.
+Defaults to false.
+
 [[label_canOverride]]
 `label.Label-Name.canOverride`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java

@@ -97,6 +97,8 @@ public class LabelType {
   protected String functionName;
   protected boolean copyMinScore;
   protected boolean copyMaxScore;
+  protected boolean copyAllScoresOnTrivialRebase;
+  protected boolean copyAllScoresIfNoCodeChange;
 
   protected List<LabelValue> values;
   protected short maxNegative;
@@ -205,6 +207,22 @@ public class LabelType {
     this.copyMaxScore = copyMaxScore;
   }
 
+  public boolean isCopyAllScoresOnTrivialRebase() {
+    return copyAllScoresOnTrivialRebase;
+  }
+
+  public void setCopyAllScoresOnTrivialRebase(boolean copyAllScoresOnTrivialRebase) {
+    this.copyAllScoresOnTrivialRebase = copyAllScoresOnTrivialRebase;
+  }
+
+  public boolean isCopyAllScoresIfNoCodeChange() {
+    return copyAllScoresIfNoCodeChange;
+  }
+
+  public void setCopyAllScoresIfNoCodeChange(boolean copyAllScoresIfNoCodeChange) {
+    this.copyAllScoresIfNoCodeChange = copyAllScoresIfNoCodeChange;
+  }
+
   public boolean isMaxNegative(PatchSetApproval ca) {
     return maxNegative == ca.getValue();
   }

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.change.PatchSetInserter.ChangeKind;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 
@@ -69,10 +70,10 @@ public class ApprovalsUtil {
    * @throws OrmException
    */
   public static void copyLabels(ReviewDb db, LabelTypes labelTypes,
-      PatchSet.Id source, PatchSet.Id dest) throws OrmException {
+      PatchSet.Id source, PatchSet dest, ChangeKind changeKind) throws OrmException {
     Iterable<PatchSetApproval> sourceApprovals =
         db.patchSetApprovals().byPatchSet(source);
-    copyLabels(db, labelTypes, sourceApprovals, source, dest);
+    copyLabels(db, labelTypes, sourceApprovals, source, dest, changeKind);
   }
 
   /**
@@ -82,7 +83,7 @@ public class ApprovalsUtil {
    */
   public static void copyLabels(ReviewDb db, LabelTypes labelTypes,
       Iterable<PatchSetApproval> sourceApprovals, PatchSet.Id source,
-      PatchSet.Id dest) throws OrmException {
+      PatchSet dest, ChangeKind changeKind) throws OrmException {
     List<PatchSetApproval> copied = Lists.newArrayList();
     for (PatchSetApproval a : sourceApprovals) {
       if (source.equals(a.getPatchSetId())) {
@@ -90,9 +91,15 @@ public class ApprovalsUtil {
         if (type == null) {
           continue;
         } else if (type.isCopyMinScore() && type.isMaxNegative(a)) {
-          copied.add(new PatchSetApproval(dest, a));
+          copied.add(new PatchSetApproval(dest.getId(), a));
         } else if (type.isCopyMaxScore() && type.isMaxPositive(a)) {
-          copied.add(new PatchSetApproval(dest, a));
+          copied.add(new PatchSetApproval(dest.getId(), a));
+        } else if (type.isCopyAllScoresOnTrivialRebase()
+            && ChangeKind.TRIVIAL_REBASE.equals(changeKind)) {
+          copied.add(new PatchSetApproval(dest.getId(), a));
+        } else if (type.isCopyAllScoresIfNoCodeChange()
+            && ChangeKind.NO_CODE_CHANGE.equals(changeKind)) {
+          copied.add(new PatchSetApproval(dest.getId(), a));
         }
       }
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java

@@ -33,12 +33,14 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.validators.CommitValidationException;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.RefControl;
 import com.google.gerrit.server.ssh.NoSshInfo;
 import com.google.gerrit.server.ssh.SshInfo;
@@ -50,6 +52,7 @@ import com.google.inject.assistedinject.Assisted;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.merge.ThreeWayMerger;
 import org.eclipse.jgit.revwalk.FooterLine;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -81,6 +84,10 @@ public class PatchSetInserter {
     GERRIT, RECEIVE_COMMITS, NONE;
   }
 
+  public static enum ChangeKind {
+    REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE;
+  }
+
   private final ChangeHooks hooks;
   private final TrackingFooters trackingFooters;
   private final PatchSetInfoFactory patchSetInfoFactory;
@@ -90,6 +97,7 @@ public class PatchSetInserter {
   private final CommitValidators.Factory commitValidatorsFactory;
   private final ChangeIndexer indexer;
   private final ReplacePatchSetSender.Factory replacePatchSetFactory;
+  private final MergeUtil.Factory mergeUtilFactory;
 
   private final Repository git;
   private final RevWalk revWalk;
@@ -115,6 +123,7 @@ public class PatchSetInserter {
       CommitValidators.Factory commitValidatorsFactory,
       ChangeIndexer indexer,
       ReplacePatchSetSender.Factory replacePatchSetFactory,
+      MergeUtil.Factory mergeUtilFactory,
       @Assisted Repository git,
       @Assisted RevWalk revWalk,
       @Assisted RefControl refControl,
@@ -130,6 +139,7 @@ public class PatchSetInserter {
     this.commitValidatorsFactory = commitValidatorsFactory;
     this.indexer = indexer;
     this.replacePatchSetFactory = replacePatchSetFactory;
+    this.mergeUtilFactory = mergeUtilFactory;
 
     this.git = git;
     this.revWalk = revWalk;
@@ -265,8 +275,16 @@ public class PatchSetInserter {
       }
 
       if (copyLabels) {
+        PatchSet priorPatchSet = db.patchSets().get(currentPatchSetId);
+        ObjectId priorCommitId = ObjectId.fromString(priorPatchSet.getRevision().get());
+        RevCommit priorCommit = revWalk.parseCommit(priorCommitId);
+        ProjectState projectState =
+            refControl.getProjectControl().getProjectState();
+        ChangeKind changeKind =
+            getChangeKind(mergeUtilFactory, projectState, git, priorCommit, commit);
+
         ApprovalsUtil.copyLabels(db, refControl.getProjectControl()
-            .getLabelTypes(), currentPatchSetId, updatedChange.currentPatchSetId());
+            .getLabelTypes(), currentPatchSetId, patchSet, changeKind);
       }
 
       final List<FooterLine> footerLines = commit.getFooterLines();
@@ -346,6 +364,48 @@ public class PatchSetInserter {
     }
   }
 
+  public static ChangeKind getChangeKind(MergeUtil.Factory mergeUtilFactory, ProjectState project,
+      Repository git, RevCommit prior, RevCommit next) {
+    if (!next.getFullMessage().equals(prior.getFullMessage())) {
+      if (next.getTree() == prior.getTree()
+          && prior.getParent(0).equals(next.getParent(0))) {
+        return ChangeKind.NO_CODE_CHANGE;
+      } else {
+        return ChangeKind.REWORK;
+      }
+    }
+
+    if (prior.getParentCount() != 1 || next.getParentCount() != 1) {
+      // Trivial rebases done by machine only work well on 1 parent.
+      return ChangeKind.REWORK;
+    }
+
+    if (next.getTree() == prior.getTree() &&
+       prior.getParent(0).equals(next.getParent(0))) {
+      return ChangeKind.TRIVIAL_REBASE;
+    }
+
+    // A trivial rebase can be detected by looking for the next commit
+    // having the same tree as would exist when the prior commit is
+    // cherry-picked onto the next commit's new first parent.
+    try {
+      MergeUtil mergeUtil = mergeUtilFactory.create(project);
+      ThreeWayMerger merger =
+          mergeUtil.newThreeWayMerger(git, mergeUtil.createDryRunInserter());
+      merger.setBase(prior.getParent(0));
+      if (merger.merge(next.getParent(0), prior)
+          && merger.getResultTreeId().equals(next.getTree())) {
+        return ChangeKind.TRIVIAL_REBASE;
+      } else {
+        return ChangeKind.REWORK;
+      }
+    } catch (IOException err) {
+      log.warn("Cannot check trivial rebase of new patch set " + next.name()
+          + " in " + project.getProject().getName(), err);
+      return ChangeKind.REWORK;
+    }
+  }
+
   public class ChangeModifiedException extends InvalidChangeOperationException {
     private static final long serialVersionUID = 1L;
 

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java

@@ -486,15 +486,10 @@ public class MergeUtil {
 
   public ObjectInserter createDryRunInserter() {
     return new ObjectInserter() {
-      private final MutableObjectId buf = new MutableObjectId();
-      private static final int LAST_BYTE = Constants.OBJECT_ID_LENGTH - 1;
-
       @Override
       public ObjectId insert(int objectType, long length, InputStream in)
           throws IOException {
-        // create non-existing dummy ID
+        return idFor(objectType, length, in);
-        buf.setByte(LAST_BYTE, buf.getByte(LAST_BYTE) + 1);
-        return buf.copy();
       }
 
       @Override

gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java

@@ -128,6 +128,8 @@ public class ProjectConfig extends VersionedMetaData {
   private static final String KEY_FUNCTION = "function";
   private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
   private static final String KEY_COPY_MAX_SCORE = "copyMaxScore";
+  private static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
+  private static final String KEY_COPY_ALL_SCORES_IF_NO_CHANGE = "copyAllScoresIfNoCodeChange";
   private static final String KEY_VALUE = "value";
   private static final String KEY_CAN_OVERRIDE = "canOverride";
   private static final String KEY_Branch = "branch";
@@ -657,6 +659,10 @@ public class ProjectConfig extends VersionedMetaData {
           rc.getBoolean(LABEL, name, KEY_COPY_MIN_SCORE, false));
       label.setCopyMaxScore(
           rc.getBoolean(LABEL, name, KEY_COPY_MAX_SCORE, false));
+      label.setCopyAllScoresOnTrivialRebase(
+          rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, false));
+      label.setCopyAllScoresIfNoCodeChange(
+          rc.getBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, false));
       label.setCanOverride(
           rc.getBoolean(LABEL, name, KEY_CAN_OVERRIDE, true));
       label.setRefPatterns(getStringListOrNull(rc, LABEL, name, KEY_Branch));
@@ -1006,6 +1012,16 @@ public class ProjectConfig extends VersionedMetaData {
       } else {
         rc.unset(LABEL, name, KEY_COPY_MAX_SCORE);
       }
+      if (label.isCopyAllScoresOnTrivialRebase()) {
+        rc.setBoolean(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE, true);
+      } else {
+        rc.unset(LABEL, name, KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE);
+      }
+      if (label.isCopyAllScoresIfNoCodeChange()) {
+        rc.setBoolean(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE, true);
+      } else {
+        rc.unset(LABEL, name, KEY_COPY_ALL_SCORES_IF_NO_CHANGE);
+      }
       if (!label.canOverride()) {
         rc.setBoolean(LABEL, name, KEY_CAN_OVERRIDE, false);
       } else {

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -66,6 +66,8 @@ import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.change.PatchSetInserter.ChangeKind;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.change.Submit;
 import com.google.gerrit.server.config.AllProjectsName;
@@ -293,6 +295,7 @@ public class ReceiveCommits {
   private final SubmoduleOp.Factory subOpFactory;
   private final Provider<Submit> submitProvider;
   private final MergeQueue mergeQueue;
+  private final MergeUtil.Factory mergeUtilFactory;
 
   private final List<CommitValidationMessage> messages = new ArrayList<CommitValidationMessage>();
   private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@@ -336,7 +339,8 @@ public class ReceiveCommits {
       @Assisted final Repository repo,
       final SubmoduleOp.Factory subOpFactory,
       final Provider<Submit> submitProvider,
-      final MergeQueue mergeQueue) throws IOException {
+      final MergeQueue mergeQueue,
+      final MergeUtil.Factory mergeUtilFactory) throws IOException {
     this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
     this.db = db;
     this.schemaFactory = schemaFactory;
@@ -375,6 +379,7 @@ public class ReceiveCommits {
     this.subOpFactory = subOpFactory;
     this.submitProvider = submitProvider;
     this.mergeQueue = mergeQueue;
+    this.mergeUtilFactory = mergeUtilFactory;
 
     this.messageSender = new ReceivePackMessageSender();
 
@@ -1607,6 +1612,7 @@ public class ReceiveCommits {
     ChangeMessage msg;
     String mergedIntoRef;
     boolean skip;
+    ChangeKind changeKind;
     private PatchSet.Id priorPatchSet;
 
     ReplaceRequest(final Change.Id toChange, final RevCommit newCommit,
@@ -1615,6 +1621,7 @@ public class ReceiveCommits {
       this.newCommit = newCommit;
       this.inputCommand = cmd;
       this.checkMergedInto = checkMergedInto;
+      this.changeKind = ChangeKind.REWORK;
 
       revisions = HashBiMap.create();
       for (Ref ref : refs(toChange)) {
@@ -1715,6 +1722,10 @@ public class ReceiveCommits {
         }
       }
 
+      changeKind =
+          PatchSetInserter.getChangeKind(mergeUtilFactory,
+              projectControl.getProjectState(), repo, priorCommit, newCommit);
+
       PatchSet.Id id =
           ChangeUtil.nextPatchSetId(allRefs, change.currentPatchSetId());
       newPatchSet = new PatchSet(id);
@@ -1793,7 +1804,7 @@ public class ReceiveCommits {
         final MailRecipients oldRecipients = getRecipientsFromApprovals(
             oldChangeApprovals);
         ApprovalsUtil.copyLabels(db, labelTypes, oldChangeApprovals,
-            priorPatchSet, newPatchSet.getId());
+            priorPatchSet, newPatchSet, changeKind);
         approvalsUtil.addReviewers(db, labelTypes, change, newPatchSet, info,
             recipients.getReviewers(), oldRecipients.getAll());
         recipients.add(oldRecipients);