Merge "ConflictsPredicate: Suppress all checked exceptions"
diff --git a/WORKSPACE b/WORKSPACE
index ca29e49..7aa4dce 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -14,9 +14,9 @@
http_archive(
name = "io_bazel_rules_closure",
- sha256 = "4f2c173ebf95e94d98a0d5cb799e734536eaf3eca280eb15e124f5e5ef8b6e39",
- strip_prefix = "rules_closure-6fd76e645b5c622221c9920f41a4d0bc578a3046",
- urls = ["https://github.com/bazelbuild/rules_closure/archive/6fd76e645b5c622221c9920f41a4d0bc578a3046.tar.gz"],
+ sha256 = "0e6de40666f2ebb2b30dc0339745a274d9999334a249b05a3b1f46462e489adf",
+ strip_prefix = "rules_closure-87d24b1df8b62405de8dd059cb604fd9d4b1e395",
+ urls = ["https://github.com/bazelbuild/rules_closure/archive/87d24b1df8b62405de8dd059cb604fd9d4b1e395.tar.gz"],
)
# File is specific to Polymer and copied from the Closure Github -- should be
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index ded7912..b6df374 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -64,6 +64,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
@@ -360,10 +361,10 @@
private ProblemInfo wrongChangeStatus(PatchSet.Id psId, RevCommit commit) {
String refName = change().getDest().get();
return problem(
- String.format(
+ formatProblemMessage(
"Patch set %d (%s) is merged into destination ref %s (%s), but change"
+ " status is %s",
- psId.get(), commit.name(), refName, tip.name(), change().getStatus()));
+ psId.get(), commit.name(), refName, tip.name()));
}
private void checkMergedBitMatchesStatus(PatchSet.Id psId, RevCommit commit, boolean merged) {
@@ -375,13 +376,24 @@
}
} else if (!merged && change().isMerged()) {
problem(
- String.format(
+ formatProblemMessage(
"Patch set %d (%s) is not merged into"
+ " destination ref %s (%s), but change status is %s",
- currPs.getId().get(), commit.name(), refName, tip.name(), change().getStatus()));
+ currPs.getId().get(), commit.name(), refName, tip.name()));
}
}
+ private String formatProblemMessage(
+ String message, int psId, String commitName, String refName, String tipName) {
+ return String.format(
+ message,
+ psId,
+ commitName,
+ refName,
+ tipName,
+ ChangeUtil.status(change()).toUpperCase(Locale.US));
+ }
+
private void checkExpectMergedAs() {
ObjectId objId = parseObjectId(fix.expectMergedAs, "expected merged commit");
RevCommit commit = parseCommit(objId, "expected merged commit");
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
index 16d3ad1..9d8f3a8 100644
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -43,6 +43,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -436,7 +437,7 @@
checkState(
!cd.change().isMerged(),
"should not call setAllApprovals on %s change",
- cd.change().getStatus());
+ ChangeUtil.status(cd.change()));
// Include a user in the output for this label if either:
// - They are an explicit reviewer.
diff --git a/java/com/google/gerrit/server/change/SetPrivateOp.java b/java/com/google/gerrit/server/change/SetPrivateOp.java
index 2b00a40..017dbec 100644
--- a/java/com/google/gerrit/server/change/SetPrivateOp.java
+++ b/java/com/google/gerrit/server/change/SetPrivateOp.java
@@ -22,6 +22,7 @@
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.PrivateStateChanged;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -83,7 +84,8 @@
}
if (isPrivate && !change.isNew()) {
- throw new BadRequestException("cannot set a non-open change to private");
+ throw new BadRequestException(
+ String.format("cannot set %s change to private", ChangeUtil.status(change)));
}
ChangeNotes notes = ctx.getNotes();
ps = psUtil.get(notes, change.currentPatchSetId());
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 15ebdd1..9698867 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -23,6 +23,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -399,8 +400,7 @@
Change c = notes.getChange();
if (!c.isNew()) {
throw new ResourceConflictException(
- String.format(
- "change %s is %s", c.getChangeId(), c.getStatus().toString().toLowerCase()));
+ String.format("change %s is %s", c.getChangeId(), ChangeUtil.status(c)));
}
// Not allowed to edit if the current patch set is locked.
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 0d427e6..208dafb 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -32,6 +32,7 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -125,20 +126,31 @@
}
public String generate(
- RevCommit original, RevCommit mergeTip, Branch.NameKey dest, String current) {
+ RevCommit original, RevCommit mergeTip, Branch.NameKey dest, String originalMessage) {
requireNonNull(original.getRawBuffer());
if (mergeTip != null) {
requireNonNull(mergeTip.getRawBuffer());
}
- for (ChangeMessageModifier changeMessageModifier : changeMessageModifiers) {
+
+ int count = 0;
+ String current = originalMessage;
+ for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
+ ChangeMessageModifier changeMessageModifier = ext.get();
+ String className = changeMessageModifier.getClass().getName();
current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
- requireNonNull(
- current,
- () ->
- String.format(
- "%s.OnSubmit returned null instead of new commit message",
- changeMessageModifier.getClass().getName()));
+ checkState(
+ current != null,
+ "%s.onSubmit from plugin %s returned null instead of new commit message",
+ className,
+ ext.getPluginName());
+ count++;
+ logger.atFine().log(
+ "Invoked %s from plugin %s, message length now %d",
+ className, ext.getPluginName(), current.length());
}
+ logger.atFine().log(
+ "Invoked %d ChangeMessageModifiers on message with original length %d",
+ count, originalMessage.length());
return current;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 30aa5ae..4cd93e3 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -314,7 +314,7 @@
throw new ResourceConflictException(
String.format(
"Change %s with commit %s is %s",
- change.getChangeId(), base, change.getStatus().asChangeStatus()));
+ change.getChangeId(), base, ChangeUtil.status(change)));
}
private Change.Id insertPatchSet(
diff --git a/java/com/google/gerrit/server/restapi/change/GetRelated.java b/java/com/google/gerrit/server/restapi/change/GetRelated.java
index 9a65165..d3961c1 100644
--- a/java/com/google/gerrit/server/restapi/change/GetRelated.java
+++ b/java/com/google/gerrit/server/restapi/change/GetRelated.java
@@ -27,6 +27,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RevisionResource;
@@ -147,7 +148,7 @@
info._revisionNumber = ps != null ? ps.getPatchSetId() : null;
PatchSet.Id curr = change.currentPatchSetId();
info._currentRevisionNumber = curr != null ? curr.get() : null;
- info.status = change.getStatus().asChangeStatus().toString();
+ info.status = ChangeUtil.status(change);
}
info.commit = new CommitInfo();
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index d9b882d..f41e776 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -53,6 +53,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.change.NotifyResolver;
@@ -386,8 +387,7 @@
if (!cd.change().isNew()) {
if (!(cd.change().isMerged() && allowMerged)) {
commitStatus.problem(
- cd.getId(),
- "Change " + cd.getId() + " is " + cd.change().getStatus().toString().toLowerCase());
+ cd.getId(), "Change " + cd.getId() + " is " + ChangeUtil.status(cd.change()));
}
} else if (cd.change().isWorkInProgress()) {
commitStatus.problem(cd.getId(), "Change " + cd.getId() + " is work in progress");
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 4e48165..d90c4bd 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3962,12 +3962,28 @@
gApi.changes().id(r.getChangeId()).ignore(true);
assertThat(gApi.changes().id(r.getChangeId()).ignored()).isTrue();
+ // New patch set notification is not sent to users ignoring the change
sender.clear();
requestScopeOperations.setApiUser(admin.getId());
- gApi.changes().id(r.getChangeId()).abandon();
+ amendChange(r.getChangeId());
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(new Address(fullname, email));
+ Address address = new Address(fullname, email);
+ assertThat(messages.get(0).rcpt()).containsExactly(address);
+
+ // Review notification is not sent to users ignoring the change
+ sender.clear();
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).rcpt()).containsExactly(address);
+
+ // Abandoned notification is not sent to users ignoring the change
+ sender.clear();
+ gApi.changes().id(r.getChangeId()).abandon();
+ messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).rcpt()).containsExactly(address);
requestScopeOperations.setApiUser(user.getId());
gApi.changes().id(r.getChangeId()).ignore(false);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
index 7ccf9a9c..7973241 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
@@ -81,7 +81,7 @@
}
@Test
- public void setMergedChangePrivate() throws Exception {
+ public void cannotSetMergedChangePrivate() throws Exception {
PushOneCommit.Result result = createChange();
approve(result.getChangeId());
merge(result);
@@ -90,7 +90,20 @@
assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
exception.expect(BadRequestException.class);
- exception.expectMessage("cannot set a non-open change to private");
+ exception.expectMessage("cannot set merged change to private");
+ gApi.changes().id(changeId).setPrivate(true);
+ }
+
+ @Test
+ public void cannotSetAbandonedChangePrivate() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+
+ gApi.changes().id(changeId).abandon();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("cannot set abandoned change to private");
gApi.changes().id(changeId).setPrivate(true);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index b00597c..f5ebfea 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -84,6 +84,8 @@
import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.index.group.StalenessChecker;
import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.testing.Util;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.TestTimeUtil;
@@ -1074,11 +1076,15 @@
private void assertPushToGroupBranch(
Project.NameKey project, String groupRefName, String expectedErrorOnUpdate) throws Exception {
- grant(project, RefNames.REFS_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
- grant(project, RefNames.REFS_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
- grant(project, RefNames.REFS_DELETED_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
- grant(project, RefNames.REFS_DELETED_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
- grant(project, RefNames.REFS_GROUPNAMES, Permission.PUSH, false, REGISTERED_USERS);
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ ProjectConfig cfg = u.getConfig();
+ Util.allow(cfg, Permission.CREATE, REGISTERED_USERS, RefNames.REFS_GROUPS + "*");
+ Util.allow(cfg, Permission.PUSH, REGISTERED_USERS, RefNames.REFS_GROUPS + "*");
+ Util.allow(cfg, Permission.CREATE, REGISTERED_USERS, RefNames.REFS_DELETED_GROUPS + "*");
+ Util.allow(cfg, Permission.PUSH, REGISTERED_USERS, RefNames.REFS_DELETED_GROUPS + "*");
+ Util.allow(cfg, Permission.PUSH, REGISTERED_USERS, RefNames.REFS_GROUPNAMES);
+ u.save();
+ }
TestRepository<InMemoryRepository> repo = cloneProject(project);
@@ -1097,8 +1103,12 @@
}
private void assertCreateGroupBranch(Project.NameKey project) throws Exception {
- grant(project, RefNames.REFS_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
- grant(project, RefNames.REFS_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ ProjectConfig cfg = u.getConfig();
+ Util.allow(cfg, Permission.CREATE, REGISTERED_USERS, RefNames.REFS_GROUPS + "*");
+ Util.allow(cfg, Permission.PUSH, REGISTERED_USERS, RefNames.REFS_GROUPS + "*");
+ u.save();
+ }
TestRepository<InMemoryRepository> repo = cloneProject(project);
PushOneCommit.Result r =
pushFactory
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
index 30a8b6b..4088ebf 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
@@ -32,6 +32,8 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.testing.Util;
import com.google.inject.Inject;
import java.util.List;
import org.eclipse.jgit.lib.RefUpdate;
@@ -60,26 +62,29 @@
privilegedUser = accountCreator.create("privilegedUser", "snowden@nsa.gov", "Ed Snowden");
groupOperations.group(privilegedGroupUuid).forUpdate().addMember(privilegedUser.id).update();
- grant(secretProject, "refs/*", Permission.READ, false, privilegedGroupUuid);
- block(secretProject, "refs/*", Permission.READ, SystemGroupBackend.REGISTERED_USERS);
+ try (ProjectConfigUpdate u = updateProject(secretProject)) {
+ ProjectConfig cfg = u.getConfig();
+ Util.allow(cfg, Permission.READ, privilegedGroupUuid, "refs/*");
+ Util.block(cfg, Permission.READ, SystemGroupBackend.REGISTERED_USERS, "refs/*");
+ u.save();
+ }
- deny(secretRefProject, "refs/*", Permission.READ, SystemGroupBackend.ANONYMOUS_USERS);
- grant(secretRefProject, "refs/heads/secret/*", Permission.READ, false, privilegedGroupUuid);
- block(
- secretRefProject,
- "refs/heads/secret/*",
- Permission.READ,
- SystemGroupBackend.REGISTERED_USERS);
- grant(
- secretRefProject,
- "refs/heads/*",
- Permission.READ,
- false,
- SystemGroupBackend.REGISTERED_USERS);
+ try (ProjectConfigUpdate u = updateProject(secretRefProject)) {
+ ProjectConfig cfg = u.getConfig();
+ Util.deny(cfg, Permission.READ, SystemGroupBackend.ANONYMOUS_USERS, "refs/*");
+ Util.allow(cfg, Permission.READ, privilegedGroupUuid, "refs/heads/secret/*");
+ Util.block(cfg, Permission.READ, SystemGroupBackend.REGISTERED_USERS, "refs/heads/secret/*");
+ Util.allow(cfg, Permission.READ, SystemGroupBackend.REGISTERED_USERS, "refs/heads/*");
+ u.save();
+ }
// Ref permission
- grant(normalProject, "refs/*", Permission.VIEW_PRIVATE_CHANGES, false, privilegedGroupUuid);
- grant(normalProject, "refs/*", Permission.FORGE_SERVER, false, privilegedGroupUuid);
+ try (ProjectConfigUpdate u = updateProject(normalProject)) {
+ ProjectConfig cfg = u.getConfig();
+ Util.allow(cfg, Permission.VIEW_PRIVATE_CHANGES, privilegedGroupUuid, "refs/*");
+ Util.allow(cfg, Permission.FORGE_SERVER, privilegedGroupUuid, "refs/*");
+ u.save();
+ }
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index e8c828a..9538e59 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -881,8 +881,8 @@
exception.expect(ResourceConflictException.class);
exception.expectMessage(
String.format(
- "Change %s with commit %s is %s",
- change2.getChange().getId().get(), input.base, ChangeStatus.ABANDONED));
+ "Change %s with commit %s is abandoned",
+ change2.getChange().getId().get(), input.base));
gApi.changes().id(change1.getChangeId()).current().cherryPick(input);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
index 31813e3..36595ce 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmitByRebase.java
@@ -70,7 +70,8 @@
submitWithRebase(user);
}
- private void submitWithRebase(TestAccount submitter) throws Exception {
+ protected ImmutableList<PushOneCommit.Result> submitWithRebase(TestAccount submitter)
+ throws Exception {
requestScopeOperations.setApiUser(submitter.getId());
RevCommit initialHead = getRemoteHead();
PushOneCommit.Result change = createChange("Change 1", "a.txt", "content");
@@ -97,6 +98,7 @@
headAfterFirstSubmit.name(),
change2.getChangeId(),
headAfterSecondSubmit.name());
+ return ImmutableList.of(change, change2);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index 7eec854..d9a26aa 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -15,25 +15,35 @@
package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
+import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.ChangeMessageModifier;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
-import java.util.List;
+import java.util.ArrayDeque;
+import java.util.Deque;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase {
@Inject private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+ @Inject private DynamicItem<UrlFormatter> urlFormatter;
@Override
protected SubmitType getSubmitType() {
@@ -79,36 +89,86 @@
}
@Test
- @TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
- public void changeMessageOnSubmit() throws Exception {
- PushOneCommit.Result change1 = createChange();
- PushOneCommit.Result change2 = createChange();
+ public void rebaseInvokesChangeMessageModifiers() throws Exception {
+ ChangeMessageModifier modifier1 =
+ (msg, orig, tip, dest) -> msg + "This-change-before-rebase: " + orig.name() + "\n";
+ ChangeMessageModifier modifier2 =
+ (msg, orig, tip, dest) -> msg + "Previous-step-tip: " + tip.name() + "\n";
+ ChangeMessageModifier modifier3 =
+ (msg, orig, tip, dest) -> msg + "Dest: " + dest.getShortName() + "\n";
- RegistrationHandle handle =
- changeMessageModifiers.add(
- "gerrit",
- (newCommitMessage, original, mergeTip, destination) -> {
- List<String> custom = mergeTip.getFooterLines("Custom");
- if (!custom.isEmpty()) {
- newCommitMessage += "Custom-Parent: " + custom.get(0) + "\n";
- }
- return newCommitMessage + "Custom: " + destination.get();
- });
- try {
- // change1 is a fast-forward, but should be rebased in cherry pick style
- // anyway, making change2 not a fast-forward, requiring a rebase.
- approve(change1.getChangeId());
- submit(change2.getChangeId());
- } finally {
- handle.remove();
+ try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2, modifier3)) {
+ ImmutableList<PushOneCommit.Result> changes = submitWithRebase(admin);
+ ChangeData cd1 = changes.get(0).getChange();
+ ChangeData cd2 = changes.get(1).getChange();
+ assertThat(cd2.patchSets()).hasSize(2);
+ String change1CurrentCommit = cd1.currentPatchSet().getRevision().get();
+ String change2Ps1Commit = cd2.patchSet(new PatchSet.Id(cd2.getId(), 1)).getRevision().get();
+
+ assertThat(gApi.changes().id(cd2.getId().get()).revision(2).commit(false).message)
+ .isEqualTo(
+ "Change 2\n\n"
+ + ("Change-Id: " + cd2.change().getKey() + "\n")
+ + ("Reviewed-on: "
+ + urlFormatter.get().getChangeViewUrl(project, cd2.getId()).get()
+ + "\n")
+ + "Reviewed-by: Administrator <admin@example.com>\n"
+ + ("This-change-before-rebase: " + change2Ps1Commit + "\n")
+ + ("Previous-step-tip: " + change1CurrentCommit + "\n")
+ + "Dest: master\n");
}
- // ... but both changes should get custom footers.
- assertThat(getCurrentCommit(change1).getFooterLines("Custom"))
- .containsExactly("refs/heads/master");
- assertThat(getCurrentCommit(change2).getFooterLines("Custom"))
- .containsExactly("refs/heads/master");
- assertThat(getCurrentCommit(change2).getFooterLines("Custom-Parent"))
- .containsExactly("refs/heads/master");
+ }
+
+ @Test
+ public void failingChangeMessageModifierShortCircuits() throws Exception {
+ ChangeMessageModifier modifier1 =
+ (msg, orig, tip, dest) -> {
+ throw new IllegalStateException("boom");
+ };
+ ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
+ try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try {
+ submitWithRebase();
+ assert_().fail("expected ResourceConflictException");
+ } catch (ResourceConflictException e) {
+ Throwable cause = Throwables.getRootCause(e);
+ assertThat(cause).isInstanceOf(RuntimeException.class);
+ assertThat(cause).hasMessageThat().isEqualTo("boom");
+ }
+ }
+ }
+
+ @Test
+ public void changeMessageModifierReturningNullShortCircuits() throws Exception {
+ ChangeMessageModifier modifier1 = (msg, orig, tip, dest) -> null;
+ ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
+ try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try {
+ submitWithRebase();
+ assert_().fail("expected ResourceConflictException");
+ } catch (ResourceConflictException e) {
+ Throwable cause = Throwables.getRootCause(e);
+ assertThat(cause).isInstanceOf(RuntimeException.class);
+ assertThat(cause)
+ .hasMessageThat()
+ .isEqualTo(
+ modifier1.getClass().getName()
+ + ".onSubmit from plugin modifier-1 returned null instead of new commit"
+ + " message");
+ }
+ }
+ }
+
+ private AutoCloseable installChangeMessageModifiers(ChangeMessageModifier... modifiers) {
+ Deque<RegistrationHandle> handles = new ArrayDeque<>(modifiers.length);
+ for (int i = 0; i < modifiers.length; i++) {
+ handles.push(changeMessageModifiers.add("modifier-" + (i + 1), modifiers[i]));
+ }
+ return () -> {
+ while (!handles.isEmpty()) {
+ handles.pop().remove();
+ }
+ };
}
private void assertLatestRevisionHasFooters(PushOneCommit.Result change) throws Exception {
diff --git a/resources/com/google/gerrit/server/commit-msg_test.sh b/resources/com/google/gerrit/server/commit-msg_test.sh
index 44e65e4..d797be3 100755
--- a/resources/com/google/gerrit/server/commit-msg_test.sh
+++ b/resources/com/google/gerrit/server/commit-msg_test.sh
@@ -95,6 +95,21 @@
fi
}
+# Change-Id should not be inserted if gerrit.createChangeId=false
+function test_suppress_changeid {
+ cat << EOF > input
+bla bla
+EOF
+
+ git config gerrit.createChangeId false
+ ${hook} input || fail "failed hook execution"
+ git config --unset gerrit.createChangeId
+ found=$(grep -c '^Change-Id' input || true)
+ if [[ "${found}" != "0" ]]; then
+ fail "got ${found} Change-Ids, want 0"
+ fi
+}
+
# Change-Id goes after existing trailers.
function test_at_end {
cat << EOF > input
diff --git a/resources/com/google/gerrit/server/mail/Abandoned.soy b/resources/com/google/gerrit/server/mail/Abandoned.soy
index 623cfe26..2785ffc 100644
--- a/resources/com/google/gerrit/server/mail/Abandoned.soy
+++ b/resources/com/google/gerrit/server/mail/Abandoned.soy
@@ -19,12 +19,12 @@
/**
* .Abandoned template will determine the contents of the email related to a
* change being abandoned.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Abandoned kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has abandoned this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/AbandonedHtml.soy b/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
index 75d940f..9ad996e 100644
--- a/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/AbandonedHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param coverLetter
- * @param email
- * @param fromName
- */
{template .AbandonedHtml}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>abandoned</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/AddKey.soy b/resources/com/google/gerrit/server/mail/AddKey.soy
index be76aee..8b609cf 100644
--- a/resources/com/google/gerrit/server/mail/AddKey.soy
+++ b/resources/com/google/gerrit/server/mail/AddKey.soy
@@ -19,9 +19,9 @@
/**
* The .AddKey template will determine the contents of the email related to
* adding a new SSH or GPG key to an account.
- * @param email
*/
{template .AddKey kind="text"}
+ {@param email: ?}
One or more new {$email.keyType} keys have been added to Gerrit Code Review at
{sp}{$email.gerritHost}:
diff --git a/resources/com/google/gerrit/server/mail/AddKeyHtml.soy b/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
index 04a0635..ed4f435 100644
--- a/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
+++ b/resources/com/google/gerrit/server/mail/AddKeyHtml.soy
@@ -16,10 +16,8 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- */
{template .AddKeyHtml}
+ {@param email: ?}
<p>
One or more new {$email.keyType} keys have been added to Gerrit Code Review
at {$email.gerritHost}:
diff --git a/resources/com/google/gerrit/server/mail/ChangeFooter.soy b/resources/com/google/gerrit/server/mail/ChangeFooter.soy
index f1d201b..a8170ca 100644
--- a/resources/com/google/gerrit/server/mail/ChangeFooter.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeFooter.soy
@@ -19,9 +19,9 @@
/**
* The .ChangeFooter template will determine the contents of the footer text
* that will be appended to ALL emails related to changes.
- * @param email
*/
{template .ChangeFooter kind="text"}
+ {@param email: ?}
--{sp}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy b/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
index f802366..55e6ef5 100644
--- a/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeFooterHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param change
- * @param email
- */
{template .ChangeFooterHtml}
+ {@param change: ?}
+ {@param email: ?}
{if $email.changeUrl or $email.settingsUrl}
<p>
{if $email.changeUrl}
diff --git a/resources/com/google/gerrit/server/mail/ChangeSubject.soy b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
index 48ec9a2..7fcd213 100644
--- a/resources/com/google/gerrit/server/mail/ChangeSubject.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
@@ -19,13 +19,13 @@
/**
* The .ChangeSubject template will determine the contents of the email subject
* line for ALL emails related to changes.
- * @param branch
- * @param change
- * @param shortProjectName
- * @param instanceAndProjectName
- * @param addInstanceNameInSubject boolean
*/
{template .ChangeSubject kind="text"}
+ {@param branch: ?}
+ {@param change: ?}
+ {@param shortProjectName: ?}
+ {@param instanceAndProjectName: ?}
+ {@param addInstanceNameInSubject: ?} /** boolean */
{if not $addInstanceNameInSubject}
Change in {$shortProjectName}[{$branch.shortName}]: {$change.shortSubject}
{else}
diff --git a/resources/com/google/gerrit/server/mail/Comment.soy b/resources/com/google/gerrit/server/mail/Comment.soy
index f9a11cd..1eb016b 100644
--- a/resources/com/google/gerrit/server/mail/Comment.soy
+++ b/resources/com/google/gerrit/server/mail/Comment.soy
@@ -19,13 +19,13 @@
/**
* The .Comment template will determine the contents of the email related to a
* user submitting comments on changes.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
- * @param commentFiles
*/
{template .Comment kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param commentFiles: ?}
{$fromName} has posted comments on this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/CommentHtml.soy b/resources/com/google/gerrit/server/mail/CommentHtml.soy
index d554258..534cbdb 100644
--- a/resources/com/google/gerrit/server/mail/CommentHtml.soy
+++ b/resources/com/google/gerrit/server/mail/CommentHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param commentFiles
- * @param commentCount
- * @param email
- * @param labels
- * @param patchSet
- * @param patchSetCommentBlocks
- */
{template .CommentHtml}
+ {@param commentFiles: ?}
+ {@param commentCount: ?}
+ {@param email: ?}
+ {@param labels: ?}
+ {@param patchSet: ?}
+ {@param patchSetCommentBlocks: ?}
{let $commentHeaderStyle kind="css"}
margin-bottom: 4px;
{/let}
diff --git a/resources/com/google/gerrit/server/mail/DeleteReviewer.soy b/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
index 065348a..3310249 100644
--- a/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteReviewer.soy
@@ -19,12 +19,12 @@
/**
* The .DeleteReviewer template will determine the contents of the email related
* to removal of a reviewer (and the reviewer's votes) from reviews.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .DeleteReviewer kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has removed{sp}
{for $reviewerName in $email.reviewerNames}
{if not isFirst($reviewerName)},{sp}{/if}
diff --git a/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy b/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
index 0599b52..54720fe 100644
--- a/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteReviewerHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .DeleteReviewerHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName}{sp}
<strong>
diff --git a/resources/com/google/gerrit/server/mail/DeleteVote.soy b/resources/com/google/gerrit/server/mail/DeleteVote.soy
index 724e90d..d869205 100644
--- a/resources/com/google/gerrit/server/mail/DeleteVote.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteVote.soy
@@ -19,11 +19,11 @@
/**
* The .DeleteVote template will determine the contents of the email related
* to removing votes on changes.
- * @param change
- * @param coverLetter
- * @param fromName
*/
{template .DeleteVote kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param fromName: ?}
{$fromName} has removed a vote on this change.{\n}
{\n}
Change subject: {$change.subject}{\n}
diff --git a/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy b/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
index cb8162d..3a82927 100644
--- a/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
+++ b/resources/com/google/gerrit/server/mail/DeleteVoteHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param coverLetter
- * @param email
- * @param fromName
- */
{template .DeleteVoteHtml}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>removed a vote</strong> from this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/Footer.soy b/resources/com/google/gerrit/server/mail/Footer.soy
index e1890a8..7483cd9 100644
--- a/resources/com/google/gerrit/server/mail/Footer.soy
+++ b/resources/com/google/gerrit/server/mail/Footer.soy
@@ -20,9 +20,9 @@
* The .Footer template will determine the contents of the footer text
* appended to the end of all outgoing emails after the ChangeFooter and
* CommentFooter.
- * @param footers
*/
{template .Footer kind="text"}
+ {@param footers: ?}
{for $footer in $footers}
{$footer}{\n}
{/for}
diff --git a/resources/com/google/gerrit/server/mail/FooterHtml.soy b/resources/com/google/gerrit/server/mail/FooterHtml.soy
index 938655c..ce934d3 100644
--- a/resources/com/google/gerrit/server/mail/FooterHtml.soy
+++ b/resources/com/google/gerrit/server/mail/FooterHtml.soy
@@ -16,10 +16,8 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param footers
- */
{template .FooterHtml}
+ {@param footers: ?}
{\n}
{\n}
{for $footer in $footers}
diff --git a/resources/com/google/gerrit/server/mail/Merged.soy b/resources/com/google/gerrit/server/mail/Merged.soy
index 40924e6..04d54c4 100644
--- a/resources/com/google/gerrit/server/mail/Merged.soy
+++ b/resources/com/google/gerrit/server/mail/Merged.soy
@@ -20,11 +20,11 @@
/**
* The .Merged template will determine the contents of the email related to
* a change successfully merged to the head.
- * @param change
- * @param email
- * @param fromName
*/
{template .Merged kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has submitted this change and it was merged.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/MergedHtml.soy b/resources/com/google/gerrit/server/mail/MergedHtml.soy
index b11c5e5..e8c04a5 100644
--- a/resources/com/google/gerrit/server/mail/MergedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/MergedHtml.soy
@@ -16,12 +16,10 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- */
{template .MergedHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>merged</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/NewChange.soy b/resources/com/google/gerrit/server/mail/NewChange.soy
index f11edfe..84a3075 100644
--- a/resources/com/google/gerrit/server/mail/NewChange.soy
+++ b/resources/com/google/gerrit/server/mail/NewChange.soy
@@ -19,13 +19,13 @@
/**
* The .NewChange template will determine the contents of the email related to a
* user submitting a new change for review.
- * @param change
- * @param email
- * @param ownerName
- * @param patchSet
- * @param projectName
*/
{template .NewChange kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param ownerName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
{if $email.reviewerNames}
Hello{sp}
{for $reviewerName in $email.reviewerNames}
diff --git a/resources/com/google/gerrit/server/mail/NewChangeHtml.soy b/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
index 5bce806..9de8707 100644
--- a/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
+++ b/resources/com/google/gerrit/server/mail/NewChangeHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- * @param ownerName
- * @param patchSet
- * @param projectName
- */
{template .NewChangeHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param ownerName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{if $email.reviewerNames}
{$fromName} would like{sp}
diff --git a/resources/com/google/gerrit/server/mail/Private.soy b/resources/com/google/gerrit/server/mail/Private.soy
index bb32a7e9..510f15e 100644
--- a/resources/com/google/gerrit/server/mail/Private.soy
+++ b/resources/com/google/gerrit/server/mail/Private.soy
@@ -22,17 +22,17 @@
/**
* Private template to generate "View Change" buttons.
- * @param email
*/
{template .ViewChangeButton}
+ {@param email: ?}
<a href="{$email.changeUrl}">View Change</a>
{/template}
/**
* Private template to render PRE block with consistent font-sizing.
- * @param content
*/
{template .Pre}
+ {@param content: ?}
{let $preStyle kind="css"}
font-family: monospace,monospace; // Use this to avoid browsers scaling down
// monospace text.
@@ -53,10 +53,9 @@
*
* This mechanism encodes as little structure as possible in order to depend on
* the Soy autoescape mechanism for all of the content.
- *
- * @param content
*/
{template .WikiFormat}
+ {@param content: ?}
{let $blockquoteStyle kind="css"}
border-left: 1px solid #aaa;
margin: 10px 0;
@@ -87,10 +86,8 @@
{/for}
{/template}
-/**
- * @param diffLines
- */
{template .UnifiedDiff}
+ {@param diffLines: ?}
{let $addStyle kind="css"}
color: hsl(120, 100%, 40%);
{/let}
diff --git a/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy b/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
index 2886cc0..ee03de0 100644
--- a/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
+++ b/resources/com/google/gerrit/server/mail/RegisterNewEmail.soy
@@ -19,9 +19,9 @@
/**
* The .RegisterNewEmail template will determine the contents of the email
* related to registering new email accounts.
- * @param email
*/
{template .RegisterNewEmail kind="text"}
+ {@param email: ?}
Welcome to Gerrit Code Review at {$email.gerritHost}.{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy b/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
index 1cb0110..bb84cf1 100644
--- a/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
+++ b/resources/com/google/gerrit/server/mail/ReplacePatchSet.soy
@@ -19,14 +19,14 @@
/**
* The .ReplacePatchSet template will determine the contents of the email
* related to a user submitting a new patchset for a change.
- * @param change
- * @param email
- * @param fromEmail
- * @param fromName
- * @param patchSet
- * @param projectName
*/
{template .ReplacePatchSet kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromEmail: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
{if $email.reviewerNames and $fromEmail == $change.ownerEmail}
Hello{sp}
{for $reviewerName in $email.reviewerNames}
diff --git a/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy b/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
index e618bef..96cba5f 100644
--- a/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
+++ b/resources/com/google/gerrit/server/mail/ReplacePatchSetHtml.soy
@@ -16,15 +16,13 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param change
- * @param email
- * @param fromName
- * @param fromEmail
- * @param patchSet
- * @param projectName
- */
{template .ReplacePatchSetHtml}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param fromEmail: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{$fromName} <strong>uploaded patch set #{$patchSet.patchSetId}</strong>{sp}
to{sp}
diff --git a/resources/com/google/gerrit/server/mail/Restored.soy b/resources/com/google/gerrit/server/mail/Restored.soy
index 4fc6d8c..0ec65b30 100644
--- a/resources/com/google/gerrit/server/mail/Restored.soy
+++ b/resources/com/google/gerrit/server/mail/Restored.soy
@@ -19,12 +19,12 @@
/**
* The .Restored template will determine the contents of the email related to a
* change being restored.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Restored kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has restored this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/RestoredHtml.soy b/resources/com/google/gerrit/server/mail/RestoredHtml.soy
index bb856ac..bcd358f 100644
--- a/resources/com/google/gerrit/server/mail/RestoredHtml.soy
+++ b/resources/com/google/gerrit/server/mail/RestoredHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .RestoredHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} <strong>restored</strong> this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/Reverted.soy b/resources/com/google/gerrit/server/mail/Reverted.soy
index fba8744..32a65c6 100644
--- a/resources/com/google/gerrit/server/mail/Reverted.soy
+++ b/resources/com/google/gerrit/server/mail/Reverted.soy
@@ -19,12 +19,12 @@
/**
* The .Reverted template will determine the contents of the email related
* to a change being reverted.
- * @param change
- * @param coverLetter
- * @param email
- * @param fromName
*/
{template .Reverted kind="text"}
+ {@param change: ?}
+ {@param coverLetter: ?}
+ {@param email: ?}
+ {@param fromName: ?}
{$fromName} has created a revert of this change.
{if $email.changeUrl} ( {$email.changeUrl} ){/if}{\n}
{\n}
diff --git a/resources/com/google/gerrit/server/mail/RevertedHtml.soy b/resources/com/google/gerrit/server/mail/RevertedHtml.soy
index b7b254e..69260ad 100644
--- a/resources/com/google/gerrit/server/mail/RevertedHtml.soy
+++ b/resources/com/google/gerrit/server/mail/RevertedHtml.soy
@@ -16,11 +16,9 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param email
- * @param fromName
- */
{template .RevertedHtml}
+ {@param email: ?}
+ {@param fromName: ?}
<p>
{$fromName} has <strong>created a revert</strong> of this change.
</p>
diff --git a/resources/com/google/gerrit/server/mail/SetAssignee.soy b/resources/com/google/gerrit/server/mail/SetAssignee.soy
index 98290e9..1fdf690 100644
--- a/resources/com/google/gerrit/server/mail/SetAssignee.soy
+++ b/resources/com/google/gerrit/server/mail/SetAssignee.soy
@@ -19,13 +19,13 @@
/**
* The .SetAssignee template will determine the contents of the email related
* to a user being assigned to a change.
- * @param change
- * @param email
- * @param fromName
- * @param patchSet
- * @param projectName
*/
{template .SetAssignee kind="text"}
+ {@param change: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
Hello{sp}
{$email.assigneeName},
diff --git a/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy b/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
index dbd3fae..1826314 100644
--- a/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
+++ b/resources/com/google/gerrit/server/mail/SetAssigneeHtml.soy
@@ -16,14 +16,12 @@
{namespace com.google.gerrit.server.mail.template}
-/**
- * @param diffLines
- * @param email
- * @param fromName
- * @param patchSet
- * @param projectName
- */
{template .SetAssigneeHtml}
+ {@param diffLines: ?}
+ {@param email: ?}
+ {@param fromName: ?}
+ {@param patchSet: ?}
+ {@param projectName: ?}
<p>
{$fromName} has <strong>assigned</strong> a change to{sp}
{$email.assigneeName}.{sp}
diff --git a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
index f39d4ef..2901232 100755
--- a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
+++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
@@ -27,6 +27,11 @@
exit 1
fi
+# Do not create a change id if requested
+if test "false" = "`git config --bool --get gerrit.createChangeId`" ; then
+ exit 0
+fi
+
# $RANDOM will be undefined if not using bash, so don't use set -u
random=$( (whoami ; hostname ; date; cat $1 ; echo $RANDOM) | git hash-object --stdin)
dest="$1.tmp.${random}"
diff --git a/tools/bzl/classpath.bzl b/tools/bzl/classpath.bzl
index f997fcf..0d43be7 100644
--- a/tools/bzl/classpath.bzl
+++ b/tools/bzl/classpath.bzl
@@ -1,14 +1,14 @@
def _classpath_collector(ctx):
- all = depset()
+ all = []
for d in ctx.attr.deps:
if hasattr(d, "java"):
- all += d.java.transitive_runtime_deps
+ all.append(d.java.transitive_runtime_deps)
if hasattr(d.java.compilation_info, "runtime_classpath"):
- all += d.java.compilation_info.runtime_classpath
+ all.append(d.java.compilation_info.runtime_classpath)
elif hasattr(d, "files"):
- all += d.files
+ all.append(d.files)
- as_strs = [c.path for c in all.to_list()]
+ as_strs = [c.path for c in depset(transitive = all).to_list()]
ctx.actions.write(
output = ctx.outputs.runtime,
content = "\n".join(sorted(as_strs)),
diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl
index d315f8f..fcf9f33 100644
--- a/tools/bzl/javadoc.bzl
+++ b/tools/bzl/javadoc.bzl
@@ -17,13 +17,10 @@
def _impl(ctx):
zip_output = ctx.outputs.zip
- transitive_jar_set = depset()
- source_jars = depset()
- for l in ctx.attr.libs:
- source_jars += l.java.source_jars
- transitive_jar_set += l.java.transitive_deps
+ transitive_jars = depset(transitive = [l.java.transitive_deps for l in ctx.attr.libs])
+ source_jars = depset(transitive = [l.java.source_jars for l in ctx.attr.libs])
- transitive_jar_paths = [j.path for j in transitive_jar_set.to_list()]
+ transitive_jar_paths = [j.path for j in transitive_jars.to_list()]
dir = ctx.outputs.zip.path + ".dir"
source = ctx.outputs.zip.path + ".source"
external_docs = ["http://docs.oracle.com/javase/8/docs/api"] + ctx.attr.external_docs
@@ -56,7 +53,7 @@
"(cd %s && zip -Xqr ../%s *)" % (dir, ctx.outputs.zip.basename),
]
ctx.actions.run_shell(
- inputs = transitive_jar_set.to_list() + source_jars.to_list() + ctx.files._jdk,
+ inputs = transitive_jars.to_list() + source_jars.to_list() + ctx.files._jdk,
outputs = [zip_output],
command = " && ".join(cmd),
)
diff --git a/tools/bzl/js.bzl b/tools/bzl/js.bzl
index 19d4436..a7714a1 100644
--- a/tools/bzl/js.bzl
+++ b/tools/bzl/js.bzl
@@ -131,20 +131,20 @@
)
def _bower_component_impl(ctx):
- transitive_zipfiles = depset([ctx.file.zipfile])
- for d in ctx.attr.deps:
- transitive_zipfiles += d.transitive_zipfiles
+ transitive_zipfiles = depset(
+ direct = [ctx.file.zipfile],
+ transitive = [d.transitive_zipfiles for d in ctx.attr.deps],
+ )
- transitive_licenses = depset()
- if ctx.file.license:
- transitive_licenses += depset([ctx.file.license])
+ transitive_licenses = depset(
+ direct = [ctx.file.license],
+ transitive = [d.transitive_licenses for d in ctx.attr.deps],
+ )
- for d in ctx.attr.deps:
- transitive_licenses += d.transitive_licenses
-
- transitive_versions = depset(ctx.files.version_json)
- for d in ctx.attr.deps:
- transitive_versions += d.transitive_versions
+ transitive_versions = depset(
+ direct = ctx.files.version_json,
+ transitive = [d.transitive_versions for d in ctx.attr.deps],
+ )
return struct(
transitive_licenses = transitive_licenses,
@@ -183,12 +183,12 @@
mnemonic = "GenBowerZip",
)
- licenses = depset()
+ licenses = []
if ctx.file.license:
- licenses += depset([ctx.file.license])
+ licenses.append(ctx.file.license)
return struct(
- transitive_licenses = licenses,
+ transitive_licenses = depset(licenses),
transitive_versions = depset(),
transitive_zipfiles = list([ctx.outputs.zip]),
)
@@ -233,15 +233,16 @@
"""A bunch of bower components zipped up."""
zips = depset()
for d in ctx.attr.deps:
- zips += d.transitive_zipfiles
+ files = d.transitive_zipfiles
- versions = depset()
- for d in ctx.attr.deps:
- versions += d.transitive_versions
+ # TODO(davido): Make sure the field always contains a depset
+ if type(files) == "list":
+ files = depset(files)
+ zips = depset(transitive = [zips, files])
- licenses = depset()
- for d in ctx.attr.deps:
- licenses += d.transitive_versions
+ versions = depset(transitive = [d.transitive_versions for d in ctx.attr.deps])
+
+ licenses = depset(transitive = [d.transitive_versions for d in ctx.attr.deps])
out_zip = ctx.outputs.zip
out_versions = ctx.outputs.version_json
@@ -299,11 +300,7 @@
# intermediate artifact if split is wanted.
if ctx.attr.split:
- bundled = ctx.new_file(
- ctx.configuration.genfiles_dir,
- ctx.outputs.html,
- ".bundled.html",
- )
+ bundled = ctx.actions.declare_file(ctx.outputs.html.path + ".bundled.html")
else:
bundled = ctx.outputs.html
destdir = ctx.outputs.html.path + ".dir"
diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl
index 1dd6d7e..a480908 100644
--- a/tools/bzl/pkg_war.bzl
+++ b/tools/bzl/pkg_war.bzl
@@ -75,35 +75,39 @@
]
# Add lib
- transitive_lib_deps = depset()
+ transitive_libs = []
for l in ctx.attr.libs:
if hasattr(l, "java"):
- transitive_lib_deps += l.java.transitive_runtime_deps
+ transitive_libs.append(l.java.transitive_runtime_deps)
elif hasattr(l, "files"):
- transitive_lib_deps += l.files
+ transitive_libs.append(l.files)
+ transitive_lib_deps = depset(transitive = transitive_libs)
for dep in transitive_lib_deps.to_list():
cmd += _add_file(dep, build_output + "/WEB-INF/lib/")
inputs.append(dep)
# Add pgm lib
- transitive_pgmlib_deps = depset()
+ transitive_pgmlibs = []
for l in ctx.attr.pgmlibs:
- transitive_pgmlib_deps += l.java.transitive_runtime_deps
+ transitive_pgmlibs.append(l.java.transitive_runtime_deps)
+ transitive_pgmlib_deps = depset(transitive = transitive_pgmlibs)
for dep in transitive_pgmlib_deps.to_list():
if dep not in inputs:
cmd += _add_file(dep, build_output + "/WEB-INF/pgm-lib/")
inputs.append(dep)
# Add context
- transitive_context_deps = depset()
+ transitive_context_libs = []
if ctx.attr.context:
for jar in ctx.attr.context:
if hasattr(jar, "java"):
- transitive_context_deps += jar.java.transitive_runtime_deps
+ transitive_context_libs.append(jar.java.transitive_runtime_deps)
elif hasattr(jar, "files"):
- transitive_context_deps += jar.files
+ transitive_context_libs.append(jar.files)
+
+ transitive_context_deps = depset(transitive = transitive_context_libs)
for dep in transitive_context_deps.to_list():
cmd += _add_context(dep, build_output)
inputs.append(dep)
