opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.14' into stable-2.15

Browse Source 
* stable-2.14:
  ReceiveCommits: Fix NEW_PATCHSET regex pattern
  CreateChangeIT#createNewChangeSignedOffByFooter: Remove unnecessary assume
  CommitValidators: Always check for Change-Id in subject line
  AbstractPushForReview: Factor repeated code to setRequireChangeId method
  Bazel: Use rules_closure from HEAD

Change-Id: I0dd77ea57201e8a3cdc50df21e0039b672ea9b14
This commit is contained in:
David Pursehouse
2018-02-05 18:47:33 +09:00
parent 773d679b02 6131772d65
commit 79f8561402
6 changed files with 48 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
WORKSPACE
View File

@@ -6,9 +6,9 @@ load("//plugins:external_plugin_deps.bzl", "external_plugin_deps")
http_archive(
name = "io_bazel_rules_closure",
sha256 = "25f5399f18d8bf9ce435f85c6bbf671ec4820bc4396b3022cc5dc4bc66303609",
strip_prefix = "rules_closure-0.4.2",
url = "https://bazel-mirror.storage.googleapis.com/github.com/bazelbuild/rules_closure/archive/0.4.2.tar.gz", # 2017-08-29
sha256 = "6691c58a2cd30a86776dd9bb34898b041e37136f2dc7e24cadaeaf599c95c657",
strip_prefix = "rules_closure-08039ba8ca59f64248bb3b6ae016460fe9c9914f",
url = "https://github.com/bazelbuild/rules_closure/archive/08039ba8ca59f64248bb3b6ae016460fe9c9914f.tar.gz",
)
# File is specific to Polymer and copied from the Closure Github -- should be

9
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -849,6 +849,15 @@ public abstract class AbstractDaemonTest {
}
}
protected void setRequireChangeId(InheritableBoolean value) throws Exception {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
ProjectConfig config = ProjectConfig.read(md);
config.getProject().setRequireChangeID(value);
config.commit(md);
projectCache.evict(config.getProject());
}
}
protected void deny(String ref, String permission, AccountGroup.UUID id) throws Exception {
deny(project, ref, permission, id);
}

25
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@@ -1226,9 +1226,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "missing Change-Id in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewOk(testRepo);
}
@@ -1252,9 +1250,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n");
pushForReviewRejected(testRepo, "multiple Change-Id lines in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "multiple Change-Id lines in commit message footer");
}
@@ -1273,9 +1269,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
createCommit(testRepo, "Message with invalid Change-Id\n\nChange-Id: X\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
}
@@ -1299,12 +1293,19 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
+ "Change-Id: I0000000000000000000000000000000000000000\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
ProjectConfig config = projectCache.checkedGet(project).getConfig();
config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
saveProjectConfig(project, config);
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "invalid Change-Id line format in commit message footer");
}
@Test
public void pushWithChangeIdInSubjectLine() throws Exception {
createCommit(testRepo, "Change-Id: I1234000000000000000000000000000000000000");
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in commit message footer");
setRequireChangeId(InheritableBoolean.FALSE);
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in commit message footer");
}
@Test
public void pushCommitWithSameChangeIdAsPredecessorChange() throws Exception {
PushOneCommit push =

11
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -42,6 +42,7 @@ import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -96,6 +97,16 @@ public class CreateChangeIT extends AbstractDaemonTest {
assertCreateFails(ci, BadRequestException.class, "unsupported change status");
}
@Test
public void createEmptyChange_InvalidChangeId() throws Exception {
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject\n\nChange-Id: I0000000000000000000000000000000000000000";
assertCreateFails(
ci,
ResourceConflictException.class,
"invalid Change-Id line format in commit message footer");
}
@Test
public void createNewChange() throws Exception {
assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));

16
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
View File

@@ -76,7 +76,7 @@ public class CommitValidators {
private static final Logger log = LoggerFactory.getLogger(CommitValidators.class);
public static final Pattern NEW_PATCHSET_PATTERN =
Pattern.compile("^" + REFS_CHANGES + "(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$");
Pattern.compile("^" + REFS_CHANGES + "(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/[1-9][0-9]*)?$");
@Singleton
public static class Factory {
@@ -251,15 +251,13 @@ public class CommitValidators {
String sha1 = commit.abbreviate(RevId.ABBREV_LEN).name();
if (idList.isEmpty()) {
String shortMsg = commit.getShortMessage();
if (shortMsg.startsWith(CHANGE_ID_PREFIX)
&& CHANGE_ID.matcher(shortMsg.substring(CHANGE_ID_PREFIX.length()).trim()).matches()) {
String errMsg = String.format(MISSING_SUBJECT_MSG, sha1);
throw new CommitValidationException(errMsg);
}
if (projectState.isRequireChangeID()) {
String shortMsg = commit.getShortMessage();
if (shortMsg.startsWith(CHANGE_ID_PREFIX)
&& CHANGE_ID
.matcher(shortMsg.substring(CHANGE_ID_PREFIX.length()).trim())
.matches()) {
String errMsg = String.format(MISSING_SUBJECT_MSG, sha1);
throw new CommitValidationException(errMsg);
}
String errMsg = String.format(MISSING_CHANGE_ID_MSG, sha1);
messages.add(getMissingChangeIdErrorMsg(errMsg, commit));
throw new CommitValidationException(errMsg, messages);

6
polygerrit-ui/app/rules.bzl
View File

@@ -31,7 +31,11 @@ def polygerrit_bundle(name, srcs, outs, app):
convention = "GOOGLE",
# TODO(davido): Clean up these issues: http://paste.openstack.org/show/608548
# and remove this supression
suppress = ["JSC_UNUSED_LOCAL_ASSIGNMENT"],
suppress = [
"JSC_JSDOC_MISSING_TYPE_WARNING",
"JSC_UNNECESSARY_ESCAPE",
"JSC_UNUSED_LOCAL_ASSIGNMENT",
],
deps = [
"//lib/polymer_externs:polymer_closure",
"@io_bazel_rules_closure//closure/library",