Allow a Link trailer to be used as an alternative change ID trailer
In some projects it may be desirable for the trailer to contain a link to
the Gerrit review page so that it is convenient to access the review page
starting from the commit message. The Link trailer is a standard trailer
used for inserting links in the commit message, which has been adopted by
the Linux kernel among other projects. For example, the Linux kernel has a
policy about linking to the mailing list archive with Link trailers:
https://www.kernel.org/doc/html/latest/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org
This change makes Gerrit interoperate well with the Link trailer. Specifically,
it teaches the Gerrit server to recognize trailers of the form:
Link: https://gerrit-review.googlesource.com/id/I78e884a944cedb5144f661a057e4829c8f84e933
as well as the existing Change-Id trailer, teaches the server to recognize
/id/ as a search prefix for change IDs and modifies the commit-msg hook
to optionally add a Link trailer (using a server URL provided in the
property gerrit.reviewUrl) instead of the Change-Id trailer.
Change-Id: I78e884a944cedb5144f661a057e4829c8f84e933
diff --git a/Documentation/cmd-hook-commit-msg.txt b/Documentation/cmd-hook-commit-msg.txt
index 2b6d7af..e547822 100644
--- a/Documentation/cmd-hook-commit-msg.txt
+++ b/Documentation/cmd-hook-commit-msg.txt
@@ -56,6 +56,26 @@
The `Change-Id` will not be added if `gerrit.createChangeId` is set
to `false` in the git config.
+If `gerrit.reviewUrl` is set to the base URL of the Gerrit server that
+changes are uploaded to (e.g. `https://gerrit-review.googlesource.com/`)
+in the git config, then instead of adding a `Change-Id` trailer, a `Link`
+trailer will be inserted that will look like this:
+
+----
+Improve foo widget by attaching a bar.
+
+We want a bar, because it improves the foo by providing more
+wizbangery to the dowhatimeanery.
+
+Link: https://gerrit-review.googlesource.com/id/Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
+Signed-off-by: A. U. Thor <author@example.com>
+----
+
+This link will become a valid link to the review page once the change is
+uploaded to the Gerrit server. Newer versions of the Gerrit server will read
+the change identifier out of the appropriate `Link` trailer and treat it in
+the same way as the change identifier in a `Change-Id` trailer.
+
== OBTAINING
To obtain the `commit-msg` script use `scp`, `wget` or `curl` to download
diff --git a/Documentation/concept-changes.txt b/Documentation/concept-changes.txt
index 1d275b4..762fb43 100644
--- a/Documentation/concept-changes.txt
+++ b/Documentation/concept-changes.txt
@@ -203,6 +203,34 @@
method uses git's link:cmd-hook-commit-msg.html[commit-msg hook]
to automatically add the Change-Id to each new commit.
+== The Link footer
+
+Gerrit also supports the Link footer as an alternative to the Change-Id
+footer. A Link footer looks like this:
+
+....
+ Link: https://gerrit-review.googlesource.com/id/Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
+....
+
+The advantage of this style of footer is that it usually acts
+as a link directly to the change's review page, provided that
+the change has been uploaded to Gerrit. Projects such as the
+link:https://www.kernel.org/doc/html/latest/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org[Linux kernel]
+have a convention of adding links to commit messages using the
+Link footer.
+
+If multiple changes have been uploaded to Gerrit with the same
+change ID, for example if a change has been cherry-picked to multiple
+branches, the link will take the user to a list of changes.
+
+The base URL in the footer is required to match the server's base URL.
+If the URL does not match, the server will not recognize the footer
+as a change ID footer.
+
+The link:cmd-hook-commit-msg.html[commit-msg hook] can be configured
+to insert Link footers instead of Change-Id footers by setting the
+property `gerrit.reviewUrl` to the base URL of the Gerrit server.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/java/com/google/gerrit/common/FooterConstants.java b/java/com/google/gerrit/common/FooterConstants.java
index 3ec809c..656d850 100644
--- a/java/com/google/gerrit/common/FooterConstants.java
+++ b/java/com/google/gerrit/common/FooterConstants.java
@@ -20,6 +20,9 @@
/** The change ID as used to track patch sets. */
public static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
+ /** Link is an alternative footer that may be used to track patch sets. */
+ public static final FooterKey LINK = new FooterKey("Link");
+
/** The footer telling us who reviewed the change. */
public static final FooterKey REVIEWED_BY = new FooterKey("Reviewed-by");
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 414a120..4b2c8a9 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -69,6 +69,7 @@
ImmutableList.of(
"/",
"/c/*",
+ "/id/*",
"/p/*",
"/q/*",
"/x/*",
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index a166d97..eea1052 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -19,17 +19,24 @@
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
+import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
import java.util.Random;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
@Singleton
public class ChangeUtil {
@@ -106,5 +113,29 @@
return c != null ? c.getStatus().name().toLowerCase() : "deleted";
}
+ private static final Pattern LINK_CHANGE_ID_PATTERN = Pattern.compile("I[0-9a-f]{40}");
+
+ public static List<String> getChangeIdsFromFooter(RevCommit c, UrlFormatter urlFormatter) {
+ List<String> changeIds = c.getFooterLines(FooterConstants.CHANGE_ID);
+ Optional<String> webUrl = urlFormatter.getWebUrl();
+ if (!webUrl.isPresent()) {
+ return changeIds;
+ }
+
+ String prefix = webUrl.get() + "id/";
+ for (String link : c.getFooterLines(FooterConstants.LINK)) {
+ if (!link.startsWith(prefix)) {
+ continue;
+ }
+ String changeId = link.substring(prefix.length());
+ Matcher m = LINK_CHANGE_ID_PATTERN.matcher(changeId);
+ if (m.matches()) {
+ changeIds.add(changeId);
+ }
+ }
+
+ return changeIds;
+ }
+
private ChangeUtil() {}
}
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index f49a21e..a086cb1 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -28,7 +28,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
@@ -41,17 +40,20 @@
import com.google.gerrit.entities.SubmissionId;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ReviewerAdder.InternalAddReviewerInput;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
import com.google.gerrit.server.config.SendEmailExecutor;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
@@ -110,6 +112,7 @@
private final CommentAdded commentAdded;
private final ReviewerAdder reviewerAdder;
private final MessageIdGenerator messageIdGenerator;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private final Change.Id changeId;
private final PatchSet.Id psId;
@@ -159,6 +162,7 @@
RevisionCreated revisionCreated,
ReviewerAdder reviewerAdder,
MessageIdGenerator messageIdGenerator,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted Change.Id changeId,
@Assisted ObjectId commitId,
@Assisted String refName) {
@@ -175,6 +179,7 @@
this.commentAdded = commentAdded;
this.reviewerAdder = reviewerAdder;
this.messageIdGenerator = messageIdGenerator;
+ this.urlFormatter = urlFormatter;
this.changeId = changeId;
this.psId = PatchSet.id(changeId, INITIAL_PATCH_SET_ID);
@@ -191,7 +196,7 @@
public Change createChange(Context ctx) throws IOException {
change =
new Change(
- getChangeKey(ctx.getRevWalk(), commitId),
+ getChangeKey(ctx.getRevWalk()),
changeId,
ctx.getAccountId(),
BranchNameKey.create(ctx.getProject(), refName),
@@ -206,10 +211,10 @@
return change;
}
- private static Change.Key getChangeKey(RevWalk rw, ObjectId id) throws IOException {
- RevCommit commit = rw.parseCommit(id);
+ private Change.Key getChangeKey(RevWalk rw) throws IOException {
+ RevCommit commit = rw.parseCommit(commitId);
rw.parseBody(commit);
- List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
if (!idList.isEmpty()) {
return Change.key(idList.get(idList.size() - 1).trim());
}
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 61616c0..7d0bda1 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -28,7 +28,6 @@
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -38,12 +37,14 @@
import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.common.ProblemInfo;
import com.google.gerrit.extensions.common.ProblemInfo.Status;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.PatchSetState;
@@ -115,6 +116,7 @@
private final Provider<CurrentUser> user;
private final Provider<PersonIdent> serverIdent;
private final RetryHelper retryHelper;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private BatchUpdate.Factory updateFactory;
private FixInput fix;
@@ -141,7 +143,8 @@
PatchSetInserter.Factory patchSetInserterFactory,
PatchSetUtil psUtil,
Provider<CurrentUser> user,
- RetryHelper retryHelper) {
+ RetryHelper retryHelper,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.accounts = accounts;
this.accountPatchReviewStore = accountPatchReviewStore;
this.notesFactory = notesFactory;
@@ -152,6 +155,7 @@
this.retryHelper = retryHelper;
this.serverIdent = serverIdent;
this.user = user;
+ this.urlFormatter = urlFormatter;
reset();
}
@@ -456,7 +460,8 @@
// No patch set for this commit; insert one.
rw.parseBody(commit);
String changeId =
- Iterables.getFirst(commit.getFooterLines(FooterConstants.CHANGE_ID), null);
+ Iterables.getFirst(
+ ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get()), null);
// Missing Change-Id footer is ok, but mismatched is not.
if (changeId != null && !changeId.equals(change().getKey().get())) {
problem(
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 7518a14..8666f26 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -48,6 +48,7 @@
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
@@ -531,7 +532,7 @@
msgbuf.append('\n');
}
- if (!contains(footers, FooterConstants.CHANGE_ID, c.getKey().get())) {
+ if (ChangeUtil.getChangeIdsFromFooter(n, urlFormatter.get()).isEmpty()) {
msgbuf.append(FooterConstants.CHANGE_ID.getName());
msgbuf.append(": ");
msgbuf.append(c.getKey().get());
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 22772e7..bd70de1 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -60,7 +60,6 @@
import com.google.common.collect.SortedSetMultimap;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
@@ -117,6 +116,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.BanCommit;
@@ -347,6 +347,7 @@
private final ProjectConfig.Factory projectConfigFactory;
private final SetPrivateOp.Factory setPrivateOpFactory;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
+ private final DynamicItem<UrlFormatter> urlFormatter;
// Assisted injected fields.
private final ProjectState projectState;
@@ -427,6 +428,7 @@
TagCache tagCache,
SetPrivateOp.Factory setPrivateOpFactory,
ReplyAttentionSetUpdates replyAttentionSetUpdates,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted ReceivePack rp,
@@ -475,6 +477,7 @@
this.projectConfigFactory = projectConfigFactory;
this.setPrivateOpFactory = setPrivateOpFactory;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
+ this.urlFormatter = urlFormatter;
// Assisted injected fields.
this.projectState = projectState;
@@ -2092,7 +2095,7 @@
} catch (IOException e) {
throw new StorageException("Can't parse commit", e);
}
- List<String> idList = create.commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(create.commit, urlFormatter.get());
if (idList.isEmpty()) {
messages.add(
@@ -2184,7 +2187,7 @@
}
}
- List<String> idList = c.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(c, urlFormatter.get());
if (!idList.isEmpty()) {
pending.put(c, lookupByChangeKey(c, Change.key(idList.get(idList.size() - 1).trim())));
} else {
@@ -3323,7 +3326,8 @@
}
}
- for (String changeId : c.getFooterLines(FooterConstants.CHANGE_ID)) {
+ for (String changeId :
+ ChangeUtil.getChangeIdsFromFooter(c, urlFormatter.get())) {
if (byKey == null) {
byKey =
retryHelper
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index cdec2cd..ce62d7a 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -16,7 +16,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.server.change.ReviewerAdder.newAddReviewerInputFromCommitIdentity;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromReviewers;
@@ -41,11 +40,13 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.AddReviewersOp;
@@ -56,6 +57,7 @@
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
import com.google.gerrit.server.config.SendEmailExecutor;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
import com.google.gerrit.server.git.MergedByPushOp;
@@ -131,6 +133,7 @@
private final ReviewerAdder reviewerAdder;
private final Change change;
private final MessageIdGenerator messageIdGenerator;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private final ProjectState projectState;
private final BranchNameKey dest;
@@ -175,6 +178,7 @@
ReviewerAdder reviewerAdder,
Change change,
MessageIdGenerator messageIdGenerator,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted ProjectState projectState,
@Assisted BranchNameKey dest,
@Assisted boolean checkMergedInto,
@@ -202,6 +206,7 @@
this.reviewerAdder = reviewerAdder;
this.change = change;
this.messageIdGenerator = messageIdGenerator;
+ this.urlFormatter = urlFormatter;
this.projectState = projectState;
this.dest = dest;
@@ -483,7 +488,7 @@
change.setStatus(Change.Status.NEW);
change.setCurrentPatchSet(info);
- List<String> idList = commit.getFooterLines(CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
change.setKey(Change.key(idList.get(idList.size() - 1).trim()));
}
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 923ba68..c67df8b 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -34,6 +34,7 @@
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
@@ -302,7 +303,7 @@
}
RevCommit commit = receiveEvent.commit;
List<CommitValidationMessage> messages = new ArrayList<>();
- List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter);
if (idList.isEmpty()) {
String shortMsg = commit.getShortMessage();
diff --git a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
index 7ed2491..812d98d 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.project;
-import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.index.query.Predicate.and;
import static com.google.gerrit.index.query.Predicate.or;
import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open;
@@ -36,13 +35,16 @@
import com.google.gerrit.extensions.api.projects.CheckProjectResultInfo.AutoCloseableChangesCheckResult;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
@@ -75,17 +77,20 @@
private final RetryHelper retryHelper;
private final ChangeJson.Factory changeJsonFactory;
private final IndexConfig indexConfig;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
ProjectsConsistencyChecker(
GitRepositoryManager repoManager,
RetryHelper retryHelper,
ChangeJson.Factory changeJsonFactory,
- IndexConfig indexConfig) {
+ IndexConfig indexConfig,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.repoManager = repoManager;
this.retryHelper = retryHelper;
this.changeJsonFactory = changeJsonFactory;
this.indexConfig = indexConfig;
+ this.urlFormatter = urlFormatter;
}
public CheckProjectResultInfo check(Project.NameKey projectName, CheckProjectInput input)
@@ -172,7 +177,7 @@
mergedSha1s.add(commitId);
// Consider all Change-Id lines since this is what ReceiveCommits#autoCloseChanges does.
- List<String> changeIds = commit.getFooterLines(CHANGE_ID);
+ List<String> changeIds = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
// Number of predicates that we need to add for this commit, 1 per Change-Id plus one for
// the commit.
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 44dc6e1..5fab976 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -20,7 +20,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
@@ -29,6 +28,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -43,6 +43,7 @@
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.SetCherryPickOp;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -112,6 +113,7 @@
private final ApprovalsUtil approvalsUtil;
private final NotifyResolver notifyResolver;
private final BatchUpdate.Factory batchUpdateFactory;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
CherryPickChange(
@@ -128,7 +130,8 @@
ProjectCache projectCache,
ApprovalsUtil approvalsUtil,
NotifyResolver notifyResolver,
- BatchUpdate.Factory batchUpdateFactory) {
+ BatchUpdate.Factory batchUpdateFactory,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.seq = seq;
this.queryProvider = queryProvider;
this.gitManager = gitManager;
@@ -143,6 +146,7 @@
this.approvalsUtil = approvalsUtil;
this.notifyResolver = notifyResolver;
this.batchUpdateFactory = batchUpdateFactory;
+ this.urlFormatter = urlFormatter;
}
/**
@@ -312,7 +316,8 @@
input.allowConflicts);
Change.Key changeKey;
- final List<String> idList = cherryPickCommit.getFooterLines(FooterConstants.CHANGE_ID);
+ final List<String> idList =
+ ChangeUtil.getChangeIdsFromFooter(cherryPickCommit, urlFormatter.get());
if (!idList.isEmpty()) {
final String idStr = idList.get(idList.size() - 1).trim();
changeKey = Change.key(idStr);
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 2e9d21a..190deb5 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.CommitMessageInput;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -34,6 +35,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -73,6 +75,7 @@
private final PatchSetUtil psUtil;
private final NotifyResolver notifyResolver;
private final ProjectCache projectCache;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
PutMessage(
@@ -84,7 +87,8 @@
@GerritPersonIdent PersonIdent gerritIdent,
PatchSetUtil psUtil,
NotifyResolver notifyResolver,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.updateFactory = updateFactory;
this.repositoryManager = repositoryManager;
this.userProvider = userProvider;
@@ -94,6 +98,7 @@
this.psUtil = psUtil;
this.notifyResolver = notifyResolver;
this.projectCache = projectCache;
+ this.urlFormatter = urlFormatter;
}
@Override
@@ -200,7 +205,7 @@
}
}
- private static String ensureChangeIdIsCorrect(
+ private String ensureChangeIdIsCorrect(
boolean requireChangeId, String currentChangeId, String newCommitMessage)
throws ResourceConflictException, BadRequestException {
RevCommit revCommit =
@@ -210,7 +215,7 @@
// Check that the commit message without footers is not empty
CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
- List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> changeIdFooters = ChangeUtil.getChangeIdsFromFooter(revCommit, urlFormatter.get());
if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
throw new ResourceConflictException("wrong Change-Id footer");
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 6fb444f..e994d03 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -1557,6 +1557,27 @@
}
@Test
+ public void pushWithLinkFooter() throws Exception {
+ String changeId = "I0123456789abcdef0123456789abcdef01234567";
+ String url = cfg.getString("gerrit", null, "canonicalWebUrl");
+ if (!url.endsWith("/")) {
+ url += "/";
+ }
+ RevCommit c = createCommit(testRepo, "test commit\n\nLink: " + url + "id/" + changeId);
+ pushForReviewOk(testRepo);
+
+ List<ChangeMessageInfo> messages = getMessages(changeId);
+ assertThat(messages.get(0).message).isEqualTo("Uploaded patch set 1.");
+ }
+
+ @Test
+ public void pushWithWrongHostLinkFooter() throws Exception {
+ String changeId = "I0123456789abcdef0123456789abcdef01234567";
+ RevCommit c = createCommit(testRepo, "test commit\n\nLink: https://wronghost/id/" + changeId);
+ pushForReviewRejected(testRepo, "missing Change-Id in message footer");
+ }
+
+ @Test
public void pushWithChangeIdAboveFooterWithCreateNewChangeForAllNotInTarget() throws Exception {
enableCreateNewChangeForAllNotInTarget();
testPushWithChangeIdAboveFooter();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 1094809..38f7c06 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -78,7 +78,8 @@
MISMATCH: 'mismatch',
MISSING: 'missing',
};
-const CHANGE_ID_REGEX_PATTERN = /^Change-Id\:\s(I[0-9a-f]{8,40})/gm;
+const CHANGE_ID_REGEX_PATTERN =
+ /^(Change-Id\:\s|Link:.*\/id\/)(I[0-9a-f]{8,40})/gm;
const MIN_LINES_FOR_COMMIT_COLLAPSE = 30;
const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -1316,7 +1317,7 @@
let changeIdArr;
while (changeIdArr = CHANGE_ID_REGEX_PATTERN.exec(commitMessage)) {
- changeId = changeIdArr[1];
+ changeId = changeIdArr[2];
}
if (changeId) {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 3ffd819..5261426 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -160,6 +160,8 @@
*/
QUERY_LEGACY_SUFFIX: /^\/q\/.+,n,z$/,
+ CHANGE_ID_QUERY: /^\/id\/(I[0-9a-f]{40})$/,
+
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
CHANGE_LEGACY: /^\/c\/(\d+)\/?(((-?\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
@@ -1018,6 +1020,8 @@
this._mapRoute(RoutePattern.QUERY, '_handleQueryRoute');
+ this._mapRoute(RoutePattern.CHANGE_ID_QUERY, '_handleChangeIdQueryRoute');
+
this._mapRoute(RoutePattern.DIFF_LEGACY_LINENUM, '_handleLegacyLinenum');
this._mapRoute(
@@ -1510,6 +1514,16 @@
});
}
+ _handleChangeIdQueryRoute(data: PageContextWithQueryMap) {
+ // TODO(pcc): This will need to indicate that this was a change ID query if
+ // standard queries gain the ability to search places like commit messages
+ // for change IDs.
+ this._setParams({
+ view: GerritNav.View.SEARCH,
+ query: data.params[0],
+ });
+ }
+
_handleQueryLegacySuffixRoute(ctx: PageContextWithQueryMap) {
this._redirect(ctx.path.replace(LEGACY_QUERY_SUFFIX_PATTERN, ''));
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index a931aac..5ee58c9 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -182,6 +182,7 @@
'_handleBranchListFilterOffsetRoute',
'_handleBranchListFilterRoute',
'_handleBranchListOffsetRoute',
+ '_handleChangeIdQueryRoute',
'_handleChangeNumberLegacyRoute',
'_handleChangeRoute',
'_handleCommentRoute',
@@ -739,6 +740,14 @@
});
});
+ test('_handleChangeIdQueryRoute', () => {
+ const data = {params: ['I0123456789abcdef0123456789abcdef01234567']};
+ assertDataToParams(data, '_handleChangeIdQueryRoute', {
+ view: GerritNav.View.SEARCH,
+ query: 'I0123456789abcdef0123456789abcdef01234567',
+ });
+ });
+
suite('_handleRegisterRoute', () => {
test('happy path', () => {
const ctx = {params: ['/foo/bar']};
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index 2b788a1..12b3516 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -482,7 +482,7 @@
// Any path prefixes that should resolve to index.html.
var (
- fePaths = []string{"/q/", "/c/", "/p/", "/x/", "/dashboard/", "/admin/", "/settings/"}
+ fePaths = []string{"/q/", "/c/", "/id/", "/p/", "/x/", "/dashboard/", "/admin/", "/settings/"}
issueNumRE = regexp.MustCompile(`^\/\d+\/?$`)
)
diff --git a/resources/com/google/gerrit/server/commit-msg_test.sh b/resources/com/google/gerrit/server/commit-msg_test.sh
index d797be3..4f1a3f7 100755
--- a/resources/com/google/gerrit/server/commit-msg_test.sh
+++ b/resources/com/google/gerrit/server/commit-msg_test.sh
@@ -110,6 +110,25 @@
fi
}
+# gerrit.reviewUrl causes us to create Link instead of Change-Id.
+function test_link {
+ cat << EOF > input
+bla bla
+EOF
+
+ git config gerrit.reviewUrl https://myhost/
+ ${hook} input || fail "failed hook execution"
+ git config --unset gerrit.reviewUrl
+ found=$(grep -c '^Change-Id' input || true)
+ if [[ "${found}" != "0" ]]; then
+ fail "got ${found} Change-Ids, want 0"
+ fi
+ found=$(grep -c '^Link: https://myhost/id/I' input || true)
+ if [[ "${found}" != "1" ]]; then
+ fail "got ${found} Link footers, want 1"
+ fi
+}
+
# Change-Id goes after existing trailers.
function test_at_end {
cat << EOF > input
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 2901232..3a40d22 100755
--- a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
+++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
@@ -48,12 +48,23 @@
exit 1
fi
-# Avoid the --in-place option which only appeared in Git 2.8
-# Avoid the --if-exists option which only appeared in Git 2.15
-if ! git -c trailer.ifexists=doNothing interpret-trailers \
- --trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then
- echo "cannot insert change-id line in $1"
- exit 1
+reviewurl="$(git config --get gerrit.reviewUrl)"
+if test -n "${reviewurl}" ; then
+ if ! git interpret-trailers --parse < "$1" | grep -q '^Link:.*/id/I[0-9a-f]\{40\}$' ; then
+ if ! git interpret-trailers \
+ --trailer "Link: ${reviewurl%/}/id/I${random}" < "$1" > "${dest}" ; then
+ echo "cannot insert link footer in $1"
+ exit 1
+ fi
+ fi
+else
+ # Avoid the --in-place option which only appeared in Git 2.8
+ # Avoid the --if-exists option which only appeared in Git 2.15
+ if ! git -c trailer.ifexists=doNothing interpret-trailers \
+ --trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then
+ echo "cannot insert change-id line in $1"
+ exit 1
+ fi
fi
if ! mv "${dest}" "$1" ; then
