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>

Documentation/index.txt

@@ -6,6 +6,7 @@ User Guide
 
 * 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

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]

src/main/java/com/google/gerrit/client/admin/AdminConstants.java

@@ -30,6 +30,8 @@ public interface AdminConstants extends Constants {
   String buttonChangeGroupOwner();
   String buttonAddProjectRight();
   String buttonSaveChanges();
+  String useContributorAgreements();
+  String useSignedOffBy();
 
   String headingOwner();
   String headingDescription();
@@ -37,6 +39,7 @@ public interface AdminConstants extends Constants {
   String headingMembers();
   String headingCreateGroup();
   String headingAccessRights();
+  String headingAgreements();
 
   String projectSubmitType_FAST_FORWARD_ONLY();
   String projectSubmitType_MERGE_ALWAYS();

src/main/java/com/google/gerrit/client/admin/AdminConstants.properties

@@ -11,6 +11,8 @@ buttonCreateGroup = Create Group
 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 @@ headingSubmitType = Change Submit Action
 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

src/main/java/com/google/gerrit/client/admin/ProjectInfoPanel.java

@@ -17,6 +17,7 @@ package com.google.gerrit.client.admin;
 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.ChangeEvent;
 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 @@ public class ProjectInfoPanel extends Composite {
   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 @@ public class ProjectInfoPanel extends Composite {
     initOwner(body);
     initDescription(body);
     initSubmitType(body);
+    initAgreements(body);
     body.add(saveProject);
 
     initWidget(body);
@@ -95,6 +104,8 @@ public class ProjectInfoPanel extends Composite {
     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 @@ public class ProjectInfoPanel extends Composite {
     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 @@ public class ProjectInfoPanel extends Composite {
       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())));

src/main/java/com/google/gerrit/client/reviewdb/Project.java

@@ -129,6 +129,9 @@ public final class Project {
   @Column
   protected boolean useContributorAgreements;
 
+  @Column
+  protected boolean useSignedOffBy;
+
   @Column
   protected char submitType;
 
@@ -178,6 +181,14 @@ public final class Project {
     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 final class Project {
   public void copySettingsFrom(final Project update) {
     description = update.description;
     useContributorAgreements = update.useContributorAgreements;
+    useSignedOffBy = update.useSignedOffBy;
     submitType = update.submitType;
   }
 }

src/main/java/com/google/gerrit/client/reviewdb/ReviewDb.java

@@ -31,7 +31,7 @@ import com.google.gwtorm.client.Sequence;
  * </ul>
  */
 public interface ReviewDb extends Schema {
-  public static final int VERSION = 13;
+  public static final int VERSION = 14;
 
   @Relation
   SchemaVersionAccess schemaVersion();

src/main/java/com/google/gerrit/server/ssh/Receive.java

@@ -61,6 +61,8 @@ import org.spearce.jgit.lib.ObjectId;
 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 @@ class Receive extends AbstractGitCommand {
 
   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) {

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;

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;