Permit projects to require Signed-off-by lines to create changes
In some projects, for example the Linux kernel and any offshoot
of it, the Signed-off-by line in a change has roughly the same
meaning as signing a site-wide contributor agreement within Gerrit.
For the Linux kernel, this line means that the author agrees to
the "Developer's Certificate of Origin 1.1"[1] and is permitted to
offer the content under the relevant license, thereby granting at
least some redistribution rights.
If use_signed_off_by is true in a project Gerrit requires that the
author, committer, and patch uploader have recorded a Signed-off-by
line within the commit message footer. If any of these are missing
the upload is rejected.
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;hb=HEAD
Bug: GERRIT-113
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 924f41d..65cd906 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -6,6 +6,7 @@
* link:cmd-index.html[Command Line Tools]
* link:user-upload.html[Uploading Changes]
+* link:user-signedoffby.html[Signed-off-by Lines]
* link:access-control.html[Access Controls]
Features
diff --git a/Documentation/user-signedoffby.txt b/Documentation/user-signedoffby.txt
new file mode 100644
index 0000000..a535d59
--- /dev/null
+++ b/Documentation/user-signedoffby.txt
@@ -0,0 +1,177 @@
+Gerrit2 - Signed-off-by Lines
+=============================
+
+[NOTE]
+This document was literally taken from link:http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;hb=4e8a2372f9255a1464ef488ed925455f53fbdaa1[linux-2.6 Documentation/SubmittingPatches]
+and is covered by the GPLv2.
+
+[[Signed-off-by]]
+Signed-off-by:
+--------------
+
+To improve tracking of who did what, especially with patches that can
+percolate to their final resting place in the kernel through several
+layers of maintainers, we've introduced a "sign-off" procedure on
+patches that are being emailed around.
+
+The sign-off is a simple line at the end of the explanation for the
+patch, which certifies that you wrote it or otherwise have the right to
+pass it on as a open-source patch. The rules are pretty simple: if you
+can certify the below:
+
+----
+ Developer's Certificate of Origin 1.1
+
+ By making a contribution to this project, I certify that:
+
+ (a) The contribution was created in whole or in part by me and I
+ have the right to submit it under the open source license
+ indicated in the file; or
+
+ (b) The contribution is based upon previous work that, to the best
+ of my knowledge, is covered under an appropriate open source
+ license and I have the right under that license to submit that
+ work with modifications, whether created in whole or in part
+ by me, under the same open source license (unless I am
+ permitted to submit under a different license), as indicated
+ in the file; or
+
+ (c) The contribution was provided directly to me by some other
+ person who certified (a), (b) or (c) and I have not modified
+ it.
+
+ (d) I understand and agree that this project and the contribution
+ are public and that a record of the contribution (including all
+ personal information I submit with it, including my sign-off) is
+ maintained indefinitely and may be redistributed consistent with
+ this project or the open source license(s) involved.
+----
+
+then you just add a line saying
+
+----
+ Signed-off-by: Random J Developer <random@developer.example.org>
+----
+
+using your real name (sorry, no pseudonyms or anonymous contributions.)
+
+Some people also put extra tags at the end. They'll just be ignored for
+now, but you can do this to mark internal company procedures or just
+point out some special detail about the sign-off.
+
+If you are a subsystem or branch maintainer, sometimes you need to slightly
+modify patches you receive in order to merge them, because the code is not
+exactly the same in your tree and the submitters'. If you stick strictly to
+rule (c), you should ask the submitter to rediff, but this is a totally
+counter-productive waste of time and energy. Rule (b) allows you to adjust
+the code, but then it is very impolite to change one submitter's code and
+make him endorse your bugs. To solve this problem, it is recommended that
+you add a line between the last Signed-off-by header and yours, indicating
+the nature of your changes. While there is nothing mandatory about this, it
+seems like prepending the description with your mail and/or name, all
+enclosed in square brackets, is noticeable enough to make it obvious that
+you are responsible for last-minute changes. Example :
+
+----
+ Signed-off-by: Random J Developer <random@developer.example.org>
+ [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h]
+ Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>
+----
+
+This practise is particularly helpful if you maintain a stable branch and
+want at the same time to credit the author, track changes, merge the fix,
+and protect the submitter from complaints. Note that under no circumstances
+can you change the author's identity (the From header), as it is the one
+which appears in the changelog.
+
+[[Acked-by]]
+[[Cc]]
+Acked-by:, Cc:
+--------------
+
+The Signed-off-by: tag indicates that the signer was involved in the
+development of the patch, or that he/she was in the patch's delivery path.
+
+If a person was not directly involved in the preparation or handling of a
+patch but wishes to signify and record their approval of it then they can
+arrange to have an Acked-by: line added to the patch's changelog.
+
+Acked-by: is often used by the maintainer of the affected code when that
+maintainer neither contributed to nor forwarded the patch.
+
+Acked-by: is not as formal as Signed-off-by:. It is a record that the acker
+has at least reviewed the patch and has indicated acceptance. Hence patch
+mergers will sometimes manually convert an acker's "yep, looks good to me"
+into an Acked-by:.
+
+Acked-by: does not necessarily indicate acknowledgement of the entire patch.
+For example, if a patch affects multiple subsystems and has an Acked-by: from
+one subsystem maintainer then this usually indicates acknowledgement of just
+the part which affects that maintainer's code. Judgement should be used here.
+When in doubt people should refer to the original discussion in the mailing
+list archives.
+
+If a person has had the opportunity to comment on a patch, but has not
+provided such comments, you may optionally add a "Cc:" tag to the patch.
+This is the only tag which might be added without an explicit action by the
+person it names. This tag documents that potentially interested parties
+have been included in the discussion
+
+
+[[Reported-by]]
+[[Tested-by]]
+[[Reviewed-by]]
+Reported-by:, Tested-by: and Reviewed-by:
+-----------------------------------------
+
+If this patch fixes a problem reported by somebody else, consider adding a
+Reported-by: tag to credit the reporter for their contribution. Please
+note that this tag should not be added without the reporter's permission,
+especially if the problem was not reported in a public forum. That said,
+if we diligently credit our bug reporters, they will, hopefully, be
+inspired to help us again in the future.
+
+A Tested-by: tag indicates that the patch has been successfully tested (in
+some environment) by the person named. This tag informs maintainers that
+some testing has been performed, provides a means to locate testers for
+future patches, and ensures credit for the testers.
+
+Reviewed-by:, instead, indicates that the patch has been reviewed and found
+acceptable according to the Reviewer's Statement:
+
+----
+ Reviewer's statement of oversight
+
+ By offering my Reviewed-by: tag, I state that:
+
+ (a) I have carried out a technical review of this patch to
+ evaluate its appropriateness and readiness for inclusion into
+ the mainline kernel.
+
+ (b) Any problems, concerns, or questions relating to the patch
+ have been communicated back to the submitter. I am satisfied
+ with the submitter's response to my comments.
+
+ (c) While there may be things that could be improved with this
+ submission, I believe that it is, at this time, (1) a
+ worthwhile modification to the kernel, and (2) free of known
+ issues which would argue against its inclusion.
+
+ (d) While I have reviewed the patch and believe it to be sound, I
+ do not (unless explicitly stated elsewhere) make any
+ warranties or guarantees that it will achieve its stated
+ purpose or function properly in any given situation.
+----
+
+A Reviewed-by tag is a statement of opinion that the patch is an
+appropriate modification of the kernel without any remaining serious
+technical issues. Any interested reviewer (who has done the work) can
+offer a Reviewed-by tag for a patch. This tag serves to give credit to
+reviewers and to inform maintainers of the degree of review which has been
+done on the patch. Reviewed-by: tags, when supplied by reviewers known to
+understand the subject area and to perform thorough reviews, will normally
+increase the likelihood of your patch getting into the kernel.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
diff --git a/src/main/java/com/google/gerrit/client/admin/AdminConstants.java b/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
index b220633..d51ff34 100644
--- a/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
+++ b/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
@@ -30,6 +30,8 @@
String buttonChangeGroupOwner();
String buttonAddProjectRight();
String buttonSaveChanges();
+ String useContributorAgreements();
+ String useSignedOffBy();
String headingOwner();
String headingDescription();
@@ -37,6 +39,7 @@
String headingMembers();
String headingCreateGroup();
String headingAccessRights();
+ String headingAgreements();
String projectSubmitType_FAST_FORWARD_ONLY();
String projectSubmitType_MERGE_ALWAYS();
diff --git a/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
index f0f8c15..87fea6b 100644
--- a/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
+++ b/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
@@ -11,6 +11,8 @@
buttonChangeGroupOwner = Change Owner
buttonAddProjectRight = Add Access Right
buttonSaveChanges = Save Changes
+useContributorAgreements = Require a valid contributor agreement to upload
+useSignedOffBy = Require <a href="http://gerrit.googlecode.com/svn/documentation/2.0/user-signedoffby.html#Signed-off-by" target="_blank"><code>Signed-off-by</code></a> in commit message
headingOwner = Owners
headingDescription = Description
@@ -18,6 +20,7 @@
headingMembers = Members
headingCreateGroup = Create New Group
headingAccessRights = Access Rights
+headingAgreements = Contributor Agreements
projectSubmitType_FAST_FORWARD_ONLY = Fast Forward Only
projectSubmitType_MERGE_IF_NECESSARY = Merge If Necessary
diff --git a/src/main/java/com/google/gerrit/client/admin/ProjectInfoPanel.java b/src/main/java/com/google/gerrit/client/admin/ProjectInfoPanel.java
index 0791756..85274d8 100644
--- a/src/main/java/com/google/gerrit/client/admin/ProjectInfoPanel.java
+++ b/src/main/java/com/google/gerrit/client/admin/ProjectInfoPanel.java
@@ -17,6 +17,7 @@
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.client.reviewdb.ProjectRight;
+import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.AccountGroupSuggestOracle;
import com.google.gerrit.client.ui.SmallHeading;
@@ -25,7 +26,10 @@
import com.google.gwt.event.dom.client.ChangeHandler;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.event.logical.shared.ValueChangeEvent;
+import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.user.client.ui.Button;
+import com.google.gwt.user.client.ui.CheckBox;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.ListBox;
@@ -48,6 +52,10 @@
private Panel submitTypePanel;
private ListBox submitType;
+ private Panel agreementsPanel;
+ private CheckBox useContributorAgreements;
+ private CheckBox useSignedOffBy;
+
private NpTextArea descTxt;
private Button saveProject;
@@ -64,6 +72,7 @@
initOwner(body);
initDescription(body);
initSubmitType(body);
+ initAgreements(body);
body.add(saveProject);
initWidget(body);
@@ -95,6 +104,8 @@
submitType.setEnabled(on);
ownerTxtBox.setEnabled(on);
descTxt.setEnabled(on);
+ useContributorAgreements.setEnabled(on);
+ useSignedOffBy.setEnabled(on);
}
private void initOwner(final Panel body) {
@@ -158,6 +169,29 @@
body.add(submitTypePanel);
}
+ private void initAgreements(final Panel body) {
+ final ValueChangeHandler<Boolean> onChangeSave =
+ new ValueChangeHandler<Boolean>() {
+ @Override
+ public void onValueChange(ValueChangeEvent<Boolean> event) {
+ saveProject.setEnabled(true);
+ }
+ };
+
+ agreementsPanel = new VerticalPanel();
+ agreementsPanel.add(new SmallHeading(Util.C.headingAgreements()));
+
+ useContributorAgreements = new CheckBox(Util.C.useContributorAgreements());
+ useContributorAgreements.addValueChangeHandler(onChangeSave);
+ agreementsPanel.add(useContributorAgreements);
+
+ useSignedOffBy = new CheckBox(Util.C.useSignedOffBy(), true);
+ useSignedOffBy.addValueChangeHandler(onChangeSave);
+ agreementsPanel.add(useSignedOffBy);
+
+ body.add(agreementsPanel);
+ }
+
private void setSubmitType(final Project.SubmitType newSubmitType) {
if (submitType != null) {
for (int i = 0; i < submitType.getItemCount(); i++) {
@@ -179,20 +213,23 @@
ownerTxt.setText(Util.M.deletedGroup(project.getOwnerGroupId().get()));
}
- if (ProjectRight.WILD_PROJECT.equals(project.getId())) {
- ownerPanel.setVisible(false);
- submitTypePanel.setVisible(false);
- } else {
- ownerPanel.setVisible(true);
- submitTypePanel.setVisible(true);
- }
+ final boolean isall = ProjectRight.WILD_PROJECT.equals(project.getId());
+ ownerPanel.setVisible(!isall);
+ submitTypePanel.setVisible(!isall);
+ agreementsPanel.setVisible(!isall);
+ useContributorAgreements.setVisible(Common.getGerritConfig()
+ .isUseContributorAgreements());
descTxt.setText(project.getDescription());
+ useContributorAgreements.setValue(project.isUseContributorAgreements());
+ useSignedOffBy.setValue(project.isUseSignedOffBy());
setSubmitType(project.getSubmitType());
}
private void doSave() {
project.setDescription(descTxt.getText().trim());
+ project.setUseContributorAgreements(useContributorAgreements.getValue());
+ project.setUseSignedOffBy(useSignedOffBy.getValue());
if (submitType.getSelectedIndex() >= 0) {
project.setSubmitType(Project.SubmitType.valueOf(submitType
.getValue(submitType.getSelectedIndex())));
diff --git a/src/main/java/com/google/gerrit/client/reviewdb/Project.java b/src/main/java/com/google/gerrit/client/reviewdb/Project.java
index b7d2fa5..2637a46 100644
--- a/src/main/java/com/google/gerrit/client/reviewdb/Project.java
+++ b/src/main/java/com/google/gerrit/client/reviewdb/Project.java
@@ -130,6 +130,9 @@
protected boolean useContributorAgreements;
@Column
+ protected boolean useSignedOffBy;
+
+ @Column
protected char submitType;
protected Project() {
@@ -178,6 +181,14 @@
useContributorAgreements = u;
}
+ public boolean isUseSignedOffBy() {
+ return useSignedOffBy;
+ }
+
+ public void setUseSignedOffBy(final boolean sbo) {
+ useSignedOffBy = sbo;
+ }
+
public SubmitType getSubmitType() {
return SubmitType.forCode(submitType);
}
@@ -189,6 +200,7 @@
public void copySettingsFrom(final Project update) {
description = update.description;
useContributorAgreements = update.useContributorAgreements;
+ useSignedOffBy = update.useSignedOffBy;
submitType = update.submitType;
}
}
diff --git a/src/main/java/com/google/gerrit/client/reviewdb/ReviewDb.java b/src/main/java/com/google/gerrit/client/reviewdb/ReviewDb.java
index 1ef1603..7aef867 100644
--- a/src/main/java/com/google/gerrit/client/reviewdb/ReviewDb.java
+++ b/src/main/java/com/google/gerrit/client/reviewdb/ReviewDb.java
@@ -31,7 +31,7 @@
* </ul>
*/
public interface ReviewDb extends Schema {
- public static final int VERSION = 13;
+ public static final int VERSION = 14;
@Relation
SchemaVersionAccess schemaVersion();
diff --git a/src/main/java/com/google/gerrit/server/ssh/Receive.java b/src/main/java/com/google/gerrit/server/ssh/Receive.java
index bb90ff7..8f868f1 100644
--- a/src/main/java/com/google/gerrit/server/ssh/Receive.java
+++ b/src/main/java/com/google/gerrit/server/ssh/Receive.java
@@ -61,6 +61,8 @@
import org.spearce.jgit.lib.PersonIdent;
import org.spearce.jgit.lib.Ref;
import org.spearce.jgit.lib.RefUpdate;
+import org.spearce.jgit.revwalk.FooterKey;
+import org.spearce.jgit.revwalk.FooterLine;
import org.spearce.jgit.revwalk.RevCommit;
import org.spearce.jgit.revwalk.RevObject;
import org.spearce.jgit.revwalk.RevSort;
@@ -1033,14 +1035,44 @@
private boolean validCommitter(final ReceiveCommand cmd, final RevCommit c) {
// Require that committer matches the uploader.
+ //
final PersonIdent committer = c.getCommitterIdent();
- final String email = committer.getEmailAddress();
- if (myEmails.contains(email)) {
- return true;
- } else {
- reject(cmd, "invalid committer " + email);
+ if (!myEmails.contains(committer.getEmailAddress())) {
+ reject(cmd, "you are not committer " + committer.getEmailAddress());
return false;
}
+
+ if (proj.isUseSignedOffBy()) {
+ // If the project wants Signed-off-by / Acked-by lines, verify we
+ // have them for the blamable parties involved on this change.
+ //
+ final PersonIdent author = c.getAuthorIdent();
+ boolean sboAuthor = false, sboCommitter = false, sboMe = false;
+ for (final FooterLine footer : c.getFooterLines()) {
+ if (footer.matches(FooterKey.SIGNED_OFF_BY)) {
+ final String e = footer.getEmailAddress();
+ if (e != null) {
+ sboAuthor |= author.getEmailAddress().equals(e);
+ sboCommitter |= committer.getEmailAddress().equals(e);
+ sboMe |= myEmails.contains(e);
+ }
+ }
+ }
+ if (!sboMe) {
+ reject(cmd, "not Signed-off-by you");
+ return false;
+ }
+ if (!sboAuthor) {
+ reject(cmd, "not Signed-off-by " + author.getEmailAddress());
+ return false;
+ }
+ if (!sboCommitter) {
+ reject(cmd, "not Signed-off-by " + committer.getEmailAddress());
+ return false;
+ }
+ }
+
+ return true;
}
private void insertBranchEntity(final ReceiveCommand c) {
diff --git a/src/main/webapp/WEB-INF/sql/upgrade013_014_mysql.sql b/src/main/webapp/WEB-INF/sql/upgrade013_014_mysql.sql
new file mode 100644
index 0000000..5e1a56d
--- /dev/null
+++ b/src/main/webapp/WEB-INF/sql/upgrade013_014_mysql.sql
@@ -0,0 +1,8 @@
+-- Upgrade: schema_version 13 to 14 (MySQL)
+--
+
+ALTER TABLE projects ADD use_signed_off_by CHAR(1);
+UPDATE projects SET use_signed_off_by = 'N';
+ALTER TABLE projects MODIFY use_signed_off_by CHAR(1) DEFAULT 'N' NOT NULL;
+
+UPDATE schema_version SET version_nbr = 14;
diff --git a/src/main/webapp/WEB-INF/sql/upgrade013_014_postgres.sql b/src/main/webapp/WEB-INF/sql/upgrade013_014_postgres.sql
new file mode 100644
index 0000000..1e66639
--- /dev/null
+++ b/src/main/webapp/WEB-INF/sql/upgrade013_014_postgres.sql
@@ -0,0 +1,9 @@
+-- Upgrade: schema_version 13 to 14 (PostgreSQL)
+--
+
+ALTER TABLE projects ADD use_signed_off_by CHAR(1);
+UPDATE projects SET use_signed_off_by = 'N';
+ALTER TABLE projects ALTER COLUMN use_signed_off_by SET DEFAULT 'N';
+ALTER TABLE projects ALTER COLUMN use_signed_off_by SET NOT NULL;
+
+UPDATE schema_version SET version_nbr = 14;
