Revert "Remove requireChangeId."
This reverts
* commit ec79eb968296c5900b483c87439901a3ce8862df (I2c407bdded0,
"Remove requireChangeId.")
* commit 80beb1cac22c5e1607c6235dc6f85ecd86d21327 (I409106ce "Remove
fields/methods that are unused since requireChangeId has been
removed")
Reason for revert: This uncovered internal users at Google.
In particular, PutMessage, which is a commonly used REST API endpoint,
requires a Change-Id to be included if requireChangeId=false.
Change-Id: I626dbb4d53fbcbf5983b0a5e79ceadbee8a42db9
diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt
index 08c26d4..71af331 100644
--- a/Documentation/config-project-config.txt
+++ b/Documentation/config-project-config.txt
@@ -157,6 +157,32 @@
did what, especially with patches. Default is `INHERIT`, which means that this
property is inherited from the parent project.
+[[receive.requireChangeId]]receive.requireChangeId::
++
+The `Require Change-Id in commit message` option defines whether a
+link:user-changeid.html[Change-Id] in the commit message is required
+for pushing a commit for review. If this option is set, trying to push
+a commit for review that doesn't contain a Change-Id in the commit
+message fails with link:error-missing-changeid.html[missing Change-Id
+in commit message footer].
+
+It is recommended to set this option and use a
+link:user-changeid.html#create[commit-msg hook] (or other client side
+tooling like EGit) to automatically generate Change-Id's for new
+commits. This way the Change-Id is automatically in place when changes
+are reworked or rebased and uploading new patch sets gets easy.
+
+If this option is not set, commits can be uploaded without a Change-Id,
+but then users have to remember to copy the assigned Change-Id from the
+change screen and insert it manually into the commit message when they
+want to upload a second patch set.
+
+Default is `INHERIT`, which means that this property is inherited from
+the parent project. The global default for new hosts is `true`
+
+This option is deprecated and future releases will behave as if this
+is always `true`.
+
[[receive.maxObjectSizeLimit]]receive.maxObjectSizeLimit::
+
Maximum allowed Git object size that receive-pack will accept. If an object
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 1544aae..f69c4ae 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -824,6 +824,11 @@
"configured_value": "INHERIT",
"inherited_value": false
},
+ "require_change_id": {
+ "value": false,
+ "configured_value": "FALSE",
+ "inherited_value": true
+ },
"max_object_size_limit": {
"value": "15m",
"configured_value": "15m",
@@ -882,6 +887,7 @@
"enable_signed_push": "INHERIT",
"require_signed_push": "INHERIT",
"reject_implicit_merges": "INHERIT",
+ "require_change_id": "TRUE",
"max_object_size_limit": "10m",
"submit_type": "REBASE_IF_NECESSARY",
"state": "ACTIVE"
@@ -919,6 +925,11 @@
"configured_value": "INHERIT",
"inherited_value": false
},
+ "require_change_id": {
+ "value": true,
+ "configured_value": "TRUE",
+ "inherited_value": true
+ },
"enable_signed_push": {
"value": true,
"configured_value": "INHERIT",
@@ -3083,6 +3094,12 @@
|`create_new_change_for_all_not_in_target` |optional|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
a new change is created for every commit not in target branch.
+|`require_change_id` |optional|
+link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether a
+valid link:user-changeid.html[Change-Id] footer in any commit uploaded
+for review is required. This does not apply to commits pushed directly
+to a branch or tag. This property is deprecated and will be removed in
+a future release.
|`enable_signed_push`|optional, not set if signed push is disabled|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
signed push validation is enabled on the project.
@@ -3163,6 +3180,14 @@
branch. +
Can be `TRUE`, `FALSE` or `INHERIT`. +
If not set, this setting is not updated.
+|`require_change_id` |optional|
+Whether a valid link:user-changeid.html[Change-Id] footer in any commit
+uploaded for review is required. This does not apply to commits pushed
+directly to a branch or tag. +
+Can be `TRUE`, `FALSE` or `INHERIT`. +
+If not set, this setting is not updated.
+This property is deprecated and will be removed in
+a future release.
|`reject_implicit_merges` |optional|
Whether a check for implicit merges will be performed when changes
are pushed for review. +
@@ -3528,6 +3553,11 @@
Whether content merge should be enabled for the project (`TRUE`,
`FALSE`, `INHERIT`). +
`FALSE`, if the `submit_type` is `FAST_FORWARD_ONLY`.
+|`require_change_id` |`INHERIT` if not set|
+Whether the usage of Change-Ids is required for the project (`TRUE`,
+`FALSE`, `INHERIT`).
+This property is deprecated and will be removed in
+a future release.
|`enable_signed_push` |`INHERIT` if not set|
Whether signed push validation is enabled on the project (`TRUE`,
`FALSE`, `INHERIT`).
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 8818ade..197a6a3 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -888,6 +888,15 @@
}
}
+ protected void setRequireChangeId(InheritableBoolean value) throws Exception {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ ProjectConfig config = projectConfigFactory.read(md);
+ config.getProject().setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, value);
+ config.commit(md);
+ projectCache.evict(config.getProject());
+ }
+ }
+
protected PushOneCommit.Result pushTo(String ref) throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
return push.to(ref);
diff --git a/java/com/google/gerrit/acceptance/TestProjectInput.java b/java/com/google/gerrit/acceptance/TestProjectInput.java
index 7deb88a..0a3686b 100644
--- a/java/com/google/gerrit/acceptance/TestProjectInput.java
+++ b/java/com/google/gerrit/acceptance/TestProjectInput.java
@@ -43,6 +43,8 @@
InheritableBoolean useContentMerge() default InheritableBoolean.INHERIT;
+ InheritableBoolean requireChangeId() default InheritableBoolean.INHERIT;
+
InheritableBoolean rejectEmptyCommit() default InheritableBoolean.INHERIT;
InheritableBoolean enableSignedPush() default InheritableBoolean.INHERIT;
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
index eddfb09..fb2a0fe 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
@@ -29,6 +29,7 @@
public InheritedBooleanInfo useContentMerge;
public InheritedBooleanInfo useSignedOffBy;
public InheritedBooleanInfo createNewChangeForAllNotInTarget;
+ public InheritedBooleanInfo requireChangeId;
public InheritedBooleanInfo enableSignedPush;
public InheritedBooleanInfo requireSignedPush;
public InheritedBooleanInfo rejectImplicitMerges;
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInput.java b/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
index 44a5258..1a6d77b 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
@@ -25,6 +25,7 @@
public InheritableBoolean useContentMerge;
public InheritableBoolean useSignedOffBy;
public InheritableBoolean createNewChangeForAllNotInTarget;
+ public InheritableBoolean requireChangeId;
public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush;
public InheritableBoolean rejectImplicitMerges;
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectInput.java b/java/com/google/gerrit/extensions/api/projects/ProjectInput.java
index 2dec2b9..e61d316 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectInput.java
@@ -31,6 +31,7 @@
public InheritableBoolean useContributorAgreements;
public InheritableBoolean useSignedOffBy;
public InheritableBoolean useContentMerge;
+ public InheritableBoolean requireChangeId;
public InheritableBoolean createNewChangeForAllNotInTarget;
public InheritableBoolean rejectEmptyCommit;
public InheritableBoolean enableSignedPush;
diff --git a/java/com/google/gerrit/reviewdb/client/BooleanProjectConfig.java b/java/com/google/gerrit/reviewdb/client/BooleanProjectConfig.java
index bea5abe..a70d254 100644
--- a/java/com/google/gerrit/reviewdb/client/BooleanProjectConfig.java
+++ b/java/com/google/gerrit/reviewdb/client/BooleanProjectConfig.java
@@ -32,6 +32,7 @@
USE_CONTRIBUTOR_AGREEMENTS("receive", "requireContributorAgreement"),
USE_SIGNED_OFF_BY("receive", "requireSignedOffBy"),
USE_CONTENT_MERGE("submit", "mergeContent"),
+ REQUIRE_CHANGE_ID("receive", "requireChangeId"),
CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET("receive", "createNewChangeForAllNotInTarget"),
ENABLE_SIGNED_PUSH("receive", "enableSignedPush"),
REQUIRE_SIGNED_PUSH("receive", "requireSignedPush"),
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 02a24f7..a9a1a5d 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -146,7 +146,12 @@
new CommitterUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
- user, urlFormatter.get(), installCommitMsgHookCommand, sshInfo, change),
+ projectState,
+ user,
+ urlFormatter.get(),
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators, skipValidation),
@@ -173,7 +178,12 @@
new AuthorUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.project())),
new ChangeIdValidator(
- user, urlFormatter.get(), installCommitMsgHookCommand, sshInfo, change),
+ projectState,
+ user,
+ urlFormatter.get(),
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -247,6 +257,7 @@
private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN);
+ private final ProjectState projectState;
private final UrlFormatter urlFormatter;
private final String installCommitMsgHookCommand;
private final SshInfo sshInfo;
@@ -254,11 +265,13 @@
private final Change change;
public ChangeIdValidator(
+ ProjectState projectState,
IdentifiedUser user,
UrlFormatter urlFormatter,
String installCommitMsgHookCommand,
SshInfo sshInfo,
Change change) {
+ this.projectState = projectState;
this.urlFormatter = urlFormatter;
this.installCommitMsgHookCommand = installCommitMsgHookCommand;
this.sshInfo = sshInfo;
@@ -294,8 +307,10 @@
Type.ERROR));
throw new CommitValidationException(CHANGE_ID_ABOVE_FOOTER_MSG, messages);
}
- messages.add(getMissingChangeIdErrorMsg(MISSING_CHANGE_ID_MSG));
- throw new CommitValidationException(MISSING_CHANGE_ID_MSG, messages);
+ if (projectState.is(BooleanProjectConfig.REQUIRE_CHANGE_ID)) {
+ messages.add(getMissingChangeIdErrorMsg(MISSING_CHANGE_ID_MSG));
+ throw new CommitValidationException(MISSING_CHANGE_ID_MSG, messages);
+ }
} else if (idList.size() > 1) {
throw new CommitValidationException(MULTIPLE_CHANGE_ID_MSG, messages);
} else {
diff --git a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
index 1328f6b..79eccbb 100644
--- a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
+++ b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
@@ -39,6 +39,9 @@
BooleanProjectConfig.USE_CONTENT_MERGE,
new Mapper(i -> i.useContentMerge, (i, v) -> i.useContentMerge = v))
.put(
+ BooleanProjectConfig.REQUIRE_CHANGE_ID,
+ new Mapper(i -> i.requireChangeId, (i, v) -> i.requireChangeId = v))
+ .put(
BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
new Mapper(
i -> i.createNewChangeForAllNotInTarget,
diff --git a/java/com/google/gerrit/server/project/CreateProjectArgs.java b/java/com/google/gerrit/server/project/CreateProjectArgs.java
index 9ae3b2c..7405df1 100644
--- a/java/com/google/gerrit/server/project/CreateProjectArgs.java
+++ b/java/com/google/gerrit/server/project/CreateProjectArgs.java
@@ -33,6 +33,7 @@
public List<String> branch;
public InheritableBoolean contentMerge;
public InheritableBoolean newChangeForAllNotInTarget;
+ public InheritableBoolean changeIdRequired;
public InheritableBoolean rejectEmptyCommit;
public InheritableBoolean enableSignedPush;
public InheritableBoolean requireSignedPush;
@@ -43,6 +44,7 @@
contributorAgreements = InheritableBoolean.INHERIT;
signedOffBy = InheritableBoolean.INHERIT;
contentMerge = InheritableBoolean.INHERIT;
+ changeIdRequired = InheritableBoolean.INHERIT;
newChangeForAllNotInTarget = InheritableBoolean.INHERIT;
enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT;
diff --git a/java/com/google/gerrit/server/project/ProjectCreator.java b/java/com/google/gerrit/server/project/ProjectCreator.java
index d468b44..b50b046 100644
--- a/java/com/google/gerrit/server/project/ProjectCreator.java
+++ b/java/com/google/gerrit/server/project/ProjectCreator.java
@@ -152,6 +152,7 @@
newProject.setBooleanConfig(
BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
args.newChangeForAllNotInTarget);
+ newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 5c36e60..db02418 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -22,6 +22,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
@@ -110,7 +111,10 @@
String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
ensureCanEditCommitMessage(resource.getNotes());
- ensureChangeIdIsCorrect(resource.getChange().getKey().get(), sanitizedCommitMessage);
+ ensureChangeIdIsCorrect(
+ projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+ resource.getChange().getKey().get(),
+ sanitizedCommitMessage);
try (Repository repository = repositoryManager.openRepository(resource.getProject());
RevWalk revWalk = new RevWalk(repository);
@@ -189,7 +193,8 @@
}
}
- private static void ensureChangeIdIsCorrect(String currentChangeId, String newCommitMessage)
+ private static void ensureChangeIdIsCorrect(
+ boolean requireChangeId, String currentChangeId, String newCommitMessage)
throws ResourceConflictException, BadRequestException {
RevCommit revCommit =
RevCommit.parse(
@@ -199,7 +204,7 @@
CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
- if (changeIdFooters.isEmpty()) {
+ if (requireChangeId && changeIdFooters.isEmpty()) {
throw new ResourceConflictException("missing Change-Id footer");
}
if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 6428435..6844cac 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -147,6 +147,8 @@
args.newChangeForAllNotInTarget =
MoreObjects.firstNonNull(
input.createNewChangeForAllNotInTarget, InheritableBoolean.INHERIT);
+ args.changeIdRequired =
+ MoreObjects.firstNonNull(input.requireChangeId, InheritableBoolean.INHERIT);
args.rejectEmptyCommit =
MoreObjects.firstNonNull(input.rejectEmptyCommit, InheritableBoolean.INHERIT);
args.enableSignedPush =
diff --git a/java/com/google/gerrit/server/schema/AllProjectsInput.java b/java/com/google/gerrit/server/schema/AllProjectsInput.java
index f72bf4d..7231b18 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsInput.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsInput.java
@@ -33,6 +33,8 @@
public static final ImmutableMap<BooleanProjectConfig, InheritableBoolean>
DEFAULT_BOOLEAN_PROJECT_CONFIGS =
ImmutableMap.of(
+ BooleanProjectConfig.REQUIRE_CHANGE_ID,
+ InheritableBoolean.TRUE,
BooleanProjectConfig.USE_CONTENT_MERGE,
InheritableBoolean.TRUE,
BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS,
diff --git a/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java b/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
index 6c49b22..be56782 100644
--- a/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
+++ b/java/com/google/gerrit/server/schema/testing/AllProjectsCreatorTestUtil.java
@@ -40,6 +40,7 @@
"[receive]",
" requireContributorAgreement = false",
" requireSignedOffBy = false",
+ " requireChangeId = true",
" enableSignedPush = false");
private static final ImmutableList<String> DEFAULT_ALL_PROJECTS_SUBMIT_SECTION =
ImmutableList.of("[submit]", " mergeContent = true");
diff --git a/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java b/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
index 56e72e6..df86d63 100644
--- a/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
+++ b/java/com/google/gerrit/sshd/commands/CreateProjectCommand.java
@@ -91,6 +91,9 @@
@Option(name = "--content-merge", usage = "allow automatic conflict resolving within files")
private InheritableBoolean contentMerge = InheritableBoolean.INHERIT;
+ @Option(name = "--change-id", usage = "if change-id is required")
+ private InheritableBoolean requireChangeID = InheritableBoolean.INHERIT;
+
@Option(name = "--reject-empty-commit", usage = "if empty commits should be rejected on submit")
private InheritableBoolean rejectEmptyCommit = InheritableBoolean.INHERIT;
@@ -121,6 +124,14 @@
}
@Option(
+ name = "--require-change-id",
+ aliases = {"--id"},
+ usage = "if change-id is required")
+ void setRequireChangeId(@SuppressWarnings("unused") boolean on) {
+ requireChangeID = InheritableBoolean.TRUE;
+ }
+
+ @Option(
name = "--create-new-change-for-all-not-in-target",
aliases = {"--ncfa"},
usage = "if a new change will be created for every commit not in target branch")
@@ -175,6 +186,7 @@
input.useContributorAgreements = contributorAgreements;
input.useSignedOffBy = signedOffBy;
input.useContentMerge = contentMerge;
+ input.requireChangeId = requireChangeID;
input.createNewChangeForAllNotInTarget = createNewChangeForAllNotInTarget;
input.branches = branch;
input.createEmptyCommit = createEmptyCommit;
diff --git a/java/com/google/gerrit/sshd/commands/SetProjectCommand.java b/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
index f145b9e..8c9fc9f 100644
--- a/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
+++ b/java/com/google/gerrit/sshd/commands/SetProjectCommand.java
@@ -56,6 +56,9 @@
@Option(name = "--content-merge", usage = "allow automatic conflict resolving within files")
private InheritableBoolean contentMerge;
+ @Option(name = "--change-id", usage = "if change-id is required")
+ private InheritableBoolean requireChangeID;
+
@Option(
name = "--use-contributor-agreements",
aliases = {"--ca"},
@@ -101,6 +104,22 @@
}
@Option(
+ name = "--require-change-id",
+ aliases = {"--id"},
+ usage = "if change-id is required")
+ void setRequireChangeId(@SuppressWarnings("unused") boolean on) {
+ requireChangeID = InheritableBoolean.TRUE;
+ }
+
+ @Option(
+ name = "--no-change-id",
+ aliases = {"--nid"},
+ usage = "if change-id is not required")
+ void setNoChangeId(@SuppressWarnings("unused") boolean on) {
+ requireChangeID = InheritableBoolean.FALSE;
+ }
+
+ @Option(
name = "--project-state",
aliases = {"--ps"},
usage = "project's visibility state")
@@ -114,6 +133,7 @@
@Override
protected void run() throws Failure {
ConfigInput configInput = new ConfigInput();
+ configInput.requireChangeId = requireChangeID;
configInput.submitType = submitType;
configInput.useContentMerge = contentMerge;
configInput.useContributorAgreements = contributorAgreements;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 6d1c223..5c2f033 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3843,6 +3843,24 @@
}
@Test
+ public void changeCommitMessageWithNoChangeIdSucceedsIfChangeIdNotRequired() throws Exception {
+ ConfigInput configInput = new ConfigInput();
+ configInput.requireChangeId = InheritableBoolean.FALSE;
+ gApi.projects().name(project.get()).config(configInput);
+
+ PushOneCommit.Result r = createChange();
+ r.assertOkStatus();
+ assertThat(getCommitMessage(r.getChangeId()))
+ .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
+
+ String newMessage = "modified commit\n";
+ gApi.changes().id(r.getChangeId()).setMessage(newMessage);
+ RevisionApi rApi = gApi.changes().id(r.getChangeId()).current();
+ assertThat(rApi.files().keySet()).containsExactly("/COMMIT_MSG", "a.txt");
+ assertThat(getCommitMessage(r.getChangeId())).isEqualTo(newMessage);
+ }
+
+ @Test
public void changeCommitMessageWithNoChangeIdFails() throws Exception {
PushOneCommit.Result r = createChange();
assertThat(getCommitMessage(r.getChangeId()))
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java
index 05295ae..27dd16a 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CheckProjectIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.extensions.api.projects.CheckProjectInput.AutoCloseableChangesCheckInput;
import com.google.gerrit.extensions.api.projects.CheckProjectResultInfo;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -65,7 +66,7 @@
@Test
public void detectAutoCloseableChangeByCommit() throws Exception {
- RevCommit commit = pushCommitForReview();
+ RevCommit commit = pushCommitWithoutChangeIdForReview();
ChangeInfo change =
Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get());
@@ -89,7 +90,7 @@
@Test
public void fixAutoCloseableChangeByCommit() throws Exception {
- RevCommit commit = pushCommitForReview();
+ RevCommit commit = pushCommitWithoutChangeIdForReview();
ChangeInfo change =
Iterables.getOnlyElement(gApi.changes().query("commit:" + commit.name()).get());
@@ -279,7 +280,8 @@
+ ProjectsConsistencyChecker.AUTO_CLOSE_MAX_COMMITS_LIMIT);
}
- private RevCommit pushCommitForReview() throws Exception {
+ private RevCommit pushCommitWithoutChangeIdForReview() throws Exception {
+ setRequireChangeId(InheritableBoolean.FALSE);
RevCommit commit =
testRepo
.branch("HEAD")
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index da0cd43..a9c7d99 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -316,6 +316,7 @@
assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
.isEqualTo(input.createNewChangeForAllNotInTarget);
+ assertThat(info.requireChangeId.configuredValue).isEqualTo(input.requireChangeId);
assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges);
assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
@@ -345,6 +346,7 @@
assertThat(info.useSignedOffBy.configuredValue).isEqualTo(input.useSignedOffBy);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
.isEqualTo(input.createNewChangeForAllNotInTarget);
+ assertThat(info.requireChangeId.configuredValue).isEqualTo(input.requireChangeId);
assertThat(info.rejectImplicitMerges.configuredValue).isEqualTo(input.rejectImplicitMerges);
assertThat(info.enableReviewerByEmail.configuredValue).isEqualTo(input.enableReviewerByEmail);
assertThat(info.createNewChangeForAllNotInTarget.configuredValue)
@@ -681,6 +683,7 @@
input.useContentMerge = InheritableBoolean.TRUE;
input.useSignedOffBy = InheritableBoolean.TRUE;
input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE;
+ input.requireChangeId = InheritableBoolean.TRUE;
input.rejectImplicitMerges = InheritableBoolean.TRUE;
input.enableReviewerByEmail = InheritableBoolean.TRUE;
input.createNewChangeForAllNotInTarget = InheritableBoolean.TRUE;
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 3d9b88d..c85aa93 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -89,6 +89,7 @@
import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -123,6 +124,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
@@ -457,6 +459,20 @@
}
@Test
+ public void pushWithoutChangeIdDeprecated() throws Exception {
+ setRequireChangeId(InheritableBoolean.FALSE);
+ testRepo
+ .branch("HEAD")
+ .commit()
+ .message("A change")
+ .author(admin.newIdent())
+ .committer(new PersonIdent(admin.newIdent(), testRepo.getDate()))
+ .create();
+ PushResult result = pushHead(testRepo, "refs/for/master");
+ assertThat(result.getMessages()).contains("warning: pushing without Change-Id is deprecated");
+ }
+
+ @Test
public void autocloseByChangeId() throws Exception {
// Create a change
PushOneCommit.Result r = pushTo("refs/for/master");
@@ -1556,6 +1572,9 @@
RevCommit c = createCommit(testRepo, "Message without Change-Id");
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "missing Change-Id in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewOk(testRepo);
}
@Test
@@ -1579,6 +1598,9 @@
+ "More text, uh oh.\n");
assertThat(GitUtil.getChangeId(testRepo, c)).isEmpty();
pushForReviewRejected(testRepo, "Change-Id must be in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewRejected(testRepo, "Change-Id must be in message footer");
}
@Test
@@ -1615,6 +1637,9 @@
+ "Change-Id: I10f98c2ef76e52e23aa23be5afeb71e40b350e86\n"
+ "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n");
pushForReviewRejected(testRepo, "multiple Change-Id lines in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewRejected(testRepo, "multiple Change-Id lines in message footer");
}
@Test
@@ -1631,6 +1656,9 @@
private void testpushWithInvalidChangeId() throws Exception {
createCommit(testRepo, "Message with invalid Change-Id\n\nChange-Id: X\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
}
@Test
@@ -1652,12 +1680,18 @@
+ "\n"
+ "Change-Id: I0000000000000000000000000000000000000000\n");
pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewRejected(testRepo, "invalid Change-Id line format in message footer");
}
@Test
public void pushWithChangeIdInSubjectLine() throws Exception {
createCommit(testRepo, "Change-Id: I1234000000000000000000000000000000000000");
pushForReviewRejected(testRepo, "missing subject; Change-Id must be in message footer");
+
+ setRequireChangeId(InheritableBoolean.FALSE);
+ pushForReviewRejected(testRepo, "missing subject; Change-Id must be in message footer");
}
@Test
@@ -1675,6 +1709,19 @@
"same Change-Id in multiple changes.\n"
+ "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ " commit");
+
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .getProject()
+ .setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
+ u.save();
+ }
+
+ pushForReviewRejected(
+ testRepo,
+ "same Change-Id in multiple changes.\n"
+ + "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ + " commit");
}
@Test
@@ -1688,6 +1735,19 @@
"same Change-Id in multiple changes.\n"
+ "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ " commit");
+
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .getProject()
+ .setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, InheritableBoolean.FALSE);
+ u.save();
+ }
+
+ pushForReviewRejected(
+ testRepo,
+ "same Change-Id in multiple changes.\n"
+ + "Squash the commits with the same Change-Id or ensure Change-Ids are unique for each"
+ + " commit");
}
private static RevCommit createCommit(TestRepository<?> testRepo, String message)
@@ -2663,6 +2723,10 @@
return cds.get(0);
}
+ private static void pushForReviewOk(TestRepository<?> testRepo) throws GitAPIException {
+ pushForReview(testRepo, RemoteRefUpdate.Status.OK, null);
+ }
+
private static void pushForReviewRejected(TestRepository<?> testRepo, String expectedMessage)
throws GitAPIException {
pushForReview(testRepo, RemoteRefUpdate.Status.REJECTED_OTHER_REASON, expectedMessage);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
index b102619..043bde7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java
@@ -218,6 +218,7 @@
in.useContributorAgreements = InheritableBoolean.TRUE;
in.useSignedOffBy = InheritableBoolean.TRUE;
in.useContentMerge = InheritableBoolean.TRUE;
+ in.requireChangeId = InheritableBoolean.TRUE;
ProjectInfo p = gApi.projects().create(in).get();
assertThat(p.name).isEqualTo(newProjectName);
Project project = projectCache.get(Project.nameKey(newProjectName)).getProject();
@@ -230,6 +231,8 @@
.isEqualTo(in.useSignedOffBy);
assertThat(project.getBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE))
.isEqualTo(in.useContentMerge);
+ assertThat(project.getBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID))
+ .isEqualTo(in.requireChangeId);
}
@Test
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index aacd41b..75e1cd7 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -16,7 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
-import static com.google.gerrit.reviewdb.client.BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS;
+import static com.google.gerrit.reviewdb.client.BooleanProjectConfig.REQUIRE_CHANGE_ID;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -612,13 +612,13 @@
public void readAllProjectsBaseConfigFromSitePaths() throws Exception {
ProjectConfig cfg = factory.create(ALL_PROJECTS);
cfg.load(db);
- assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
.isEqualTo(InheritableBoolean.INHERIT);
- writeDefaultAllProjectsConfig("[receive]", "requireContributorAgreement = false");
+ writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false");
cfg.load(db);
- assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
.isEqualTo(InheritableBoolean.FALSE);
}
@@ -626,17 +626,17 @@
public void readOtherProjectIgnoresAllProjectsBaseConfig() throws Exception {
ProjectConfig cfg = factory.create(Project.nameKey("test"));
cfg.load(db);
- assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
.isEqualTo(InheritableBoolean.INHERIT);
- writeDefaultAllProjectsConfig("[receive]", "requireContributorAgreement = false");
+ writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false");
cfg.load(db);
// If we went through ProjectState, then this would return FALSE, since project.config for
// All-Projects would inherit from all_projects.config, and this project would inherit from
// All-Projects. But in ProjectConfig itself, there is no inheritance from All-Projects, so this
// continues to return the default.
- assertThat(cfg.getProject().getBooleanConfig(USE_CONTRIBUTOR_AGREEMENTS))
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
.isEqualTo(InheritableBoolean.INHERIT);
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
index 044e1fe..52f143c 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
@@ -154,6 +154,21 @@
</gr-select>
</span>
</section>
+ <section>
+ <span class="title">Require Change-Id in commit message</span>
+ <span class="value">
+ <gr-select
+ id="requireChangeIdSelect"
+ bind-value="{{_repoConfig.require_change_id.configured_value}}">
+ <select disabled$="[[_readOnly]]">
+ <template is="dom-repeat"
+ items="[[_formatBooleanSelect(_repoConfig.require_change_id)]]">
+ <option value="[[item.value]]">[[item.label]]</option>
+ </template>
+ </select>
+ </gr-select>
+ </span>
+ </section>
<section
id="enableSignedPushSettings"
class$="repositorySettings [[_computeRepositoriesClass(_repoConfig.enable_signed_push)]]">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.html b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.html
index 744ef42..53db6d5 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.html
@@ -57,6 +57,10 @@
value: false,
configured_value: 'FALSE',
},
+ require_change_id: {
+ value: false,
+ configured_value: 'FALSE',
+ },
enable_signed_push: {
value: false,
configured_value: 'FALSE',
@@ -318,6 +322,7 @@
use_content_merge: 'TRUE',
use_signed_off_by: 'TRUE',
create_new_change_for_all_not_in_target: 'TRUE',
+ require_change_id: 'TRUE',
enable_signed_push: 'TRUE',
require_signed_push: 'TRUE',
reject_implicit_merges: 'TRUE',
@@ -347,6 +352,8 @@
configInputObj.use_content_merge;
element.$.newChangeSelect.bindValue =
configInputObj.create_new_change_for_all_not_in_target;
+ element.$.requireChangeIdSelect.bindValue =
+ configInputObj.require_change_id;
element.$.enableSignedPush.bindValue =
configInputObj.enable_signed_push;
element.$.requireSignedPush.bindValue =
