From: Daniel P. Berrangé <berrange@redhat.com>
Currently we have a short paragraph saying that patches must include
a Signed-off-by line, and merely link to the kernel documentation.
The linked kernel docs have a lot of content beyond the part about
sign-off an thus are misleading/distracting to QEMU contributors.
This introduces a dedicated 'code-provenance' page in QEMU talking
about why we require sign-off, explaining the other tags we commonly
use, and what to do in some edge cases.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++
docs/devel/index-process.rst | 1 +
docs/devel/submitting-a-patch.rst | 18 +--
3 files changed, 221 insertions(+), 16 deletions(-)
create mode 100644 docs/devel/code-provenance.rst
diff --git a/docs/devel/code-provenance.rst b/docs/devel/code-provenance.rst
new file mode 100644
index 0000000000..4fc12061b5
--- /dev/null
+++ b/docs/devel/code-provenance.rst
@@ -0,0 +1,218 @@
+.. _code-provenance:
+
+Code provenance
+===============
+
+Certifying patch submissions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The QEMU community **mandates** all contributors to certify provenance of
+patch submissions they make to the project. To put it another way,
+contributors must indicate that they are legally permitted to contribute to
+the project.
+
+Certification is achieved with a low overhead by adding a single line to the
+bottom of every git commit::
+
+ Signed-off-by: YOUR NAME <YOUR@EMAIL>
+
+using a known identity (sorry, no anonymous contributions.)
+
+The addition of this line asserts that the author of the patch is contributing
+in accordance with the clauses specified in the
+`Developer's Certificate of Origin <https://developercertificate.org>`__:
+
+.. _dco:
+
+ 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.
+
+It is generally expected that the name and email addresses used in one of the
+``Signed-off-by`` lines, matches that of the git commit ``Author`` field.
+It's okay if you subscribe or contribute to the list via more than one
+address, but using multiple addresses in one commit just confuses
+things.
+
+If the person sending the mail is not one of the patch authors, they are
+nonetheless expected to add their own ``Signed-off-by`` to comply with the
+DCO clause (c).
+
+Multiple authorship
+~~~~~~~~~~~~~~~~~~~
+
+It is not uncommon for a patch to have contributions from multiple authors. In
+this scenario, git commits will usually be expected to have a ``Signed-off-by``
+line for each contributor involved in creation of the patch. Some edge cases:
+
+ * The non-primary author's contributions were so trivial that they can be
+ considered not subject to copyright. In this case the secondary authors
+ need not include a ``Signed-off-by``.
+
+ This case most commonly applies where QEMU reviewers give short snippets
+ of code as suggested fixes to a patch. The reviewers don't need to have
+ their own ``Signed-off-by`` added unless their code suggestion was
+ unusually large, but it is common to add ``Suggested-by`` as a credit
+ for non-trivial code.
+
+ * Both contributors work for the same employer and the employer requires
+ copyright assignment.
+
+ It can be said that in this case a ``Signed-off-by`` is indicating that
+ the person has permission to contribute from their employer who is the
+ copyright holder. It is nonetheless still preferable to include a
+ ``Signed-off-by`` for each contributor, as in some countries employees are
+ not able to assign copyright to their employer, and it also covers any
+ time invested outside working hours.
+
+When multiple ``Signed-off-by`` tags are present, they should be strictly kept
+in order of authorship, from oldest to newest.
+
+Other commit tags
+~~~~~~~~~~~~~~~~~
+
+While the ``Signed-off-by`` tag is mandatory, there are a number of other tags
+that are commonly used during QEMU development:
+
+ * **``Reviewed-by``**: when a QEMU community member reviews a patch on the
+ mailing list, if they consider the patch acceptable, they should send an
+ email reply containing a ``Reviewed-by`` tag. Subsystem maintainers who
+ review a patch should add this even if they are also adding their
+ ``Signed-off-by`` to the same commit.
+
+ * **``Acked-by``**: when a QEMU subsystem maintainer approves a patch that
+ touches their subsystem, but intends to allow a different maintainer to
+ queue it and send a pull request, they would send a mail containing a
+ ``Acked-by`` tag. Where a patch touches multiple subsystems, ``Acked-by``
+ only implies review of the maintainers' own areas of responsibility. If a
+ maintainer wants to indicate they have done a full review they should use
+ a ``Reviewed-by`` tag.
+
+ * **``Tested-by``**: when a QEMU community member has functionally tested the
+ behaviour of the patch in some manner, they should send an email reply
+ containing a ``Tested-by`` tag.
+
+ * **``Reported-by``**: when a QEMU community member reports a problem via the
+ mailing list, or some other informal channel that is not the issue tracker,
+ it is good practice to credit them by including a ``Reported-by`` tag on
+ any patch fixing the issue. When the problem is reported via the GitLab
+ issue tracker, however, it is sufficient to just include a link to the
+ issue.
+
+ * **``Suggested-by``**: when a reviewer or other 3rd party makes non-trivial
+ suggestions for how to change a patch, it is good practice to credit them
+ by including a ``Suggested-by`` tag.
+
+Subsystem maintainer requirements
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When a subsystem maintainer accepts a patch from a contributor, in addition to
+the normal code review points, they are expected to validate the presence of
+suitable ``Signed-off-by`` tags.
+
+At the time they queue the patch in their subsystem tree, the maintainer
+**must** also then add their own ``Signed-off-by`` to indicate that they have
+done the aforementioned validation. This is in addition to any of their own
+``Reviewed-by`` tags the subsystem maintainer may wish to include.
+
+Tools for adding ``Signed-off-by``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+There are a variety of ways tools can support adding ``Signed-off-by`` tags
+for patches, avoiding the need for contributors to manually type in this
+repetitive text each time.
+
+git commands
+^^^^^^^^^^^^
+
+When creating, or amending, a commit the ``-s`` flag to ``git commit`` will
+append a suitable line matching the configured git author details.
+
+If preparing patches using the ``git format-patch`` tool, the ``-s`` flag can
+be used to append a suitable line in the emails it creates, without modifying
+the local commits. Alternatively to modify all the local commits on a branch::
+
+ git rebase master -x 'git commit --amend --no-edit -s'
+
+emacs
+^^^^^
+
+In the file ``$HOME/.emacs.d/abbrev_defs`` add:
+
+.. code:: elisp
+
+ (define-abbrev-table 'global-abbrev-table
+ '(
+ ("8rev" "Reviewed-by: YOUR NAME <your@email.addr>" nil 1)
+ ("8ack" "Acked-by: YOUR NAME <your@email.addr>" nil 1)
+ ("8test" "Tested-by: YOUR NAME <your@email.addr>" nil 1)
+ ("8sob" "Signed-off-by: YOUR NAME <your@email.addr>" nil 1)
+ ))
+
+with this change, if you type (for example) ``8rev`` followed by ``<space>``
+or ``<enter>`` it will expand to the whole phrase.
+
+vim
+^^^
+
+In the file ``$HOME/.vimrc`` add::
+
+ iabbrev 8rev Reviewed-by: YOUR NAME <your@email.addr>
+ iabbrev 8ack Acked-by: YOUR NAME <your@email.addr>
+ iabbrev 8test Tested-by: YOUR NAME <your@email.addr>
+ iabbrev 8sob Signed-off-by: YOUR NAME <your@email.addr>
+
+with this change, if you type (for example) ``8rev`` followed by ``<space>``
+or ``<enter>`` it will expand to the whole phrase.
+
+Re-starting abandoned work
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+For a variety of reasons there are some patches that get submitted to QEMU but
+never merged. An unrelated contributor may decide (months or years later) to
+continue working from the abandoned patch and re-submit it with extra changes.
+
+The general principles when picking up abandoned work are:
+
+ * Continue to credit the original author for their work, by maintaining their
+ original ``Signed-off-by``
+ * Indicate where the original patch was obtained from (mailing list, bug
+ tracker, author's git repo, etc) when sending it for review
+ * Acknowledge the extra work of the new contributor by including their
+ ``Signed-off-by`` in the patch in addition to the orignal author's
+ * Indicate who is responsible for what parts of the patch. This is typically
+ done via a note in the commit message, just prior to the new contributor's
+ ``Signed-off-by``::
+
+ Signed-off-by: Some Person <some.person@example.com>
+ [Rebased and added support for 'foo']
+ Signed-off-by: New Person <new.person@mycorp.test>
+
+In complicated cases, or if otherwise unsure, ask for advice on the project
+mailing list.
+
+It is also recommended to attempt to contact the original author to let them
+know you are interested in taking over their work, in case they still intended
+to return to the work, or had any suggestions about the best way to continue.
diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst
index cb7c6640fd..5807752d70 100644
--- a/docs/devel/index-process.rst
+++ b/docs/devel/index-process.rst
@@ -13,6 +13,7 @@ Notes about how to interact with the community and how and where to submit patch
maintainers
style
submitting-a-patch
+ code-provenance
trivial-patches
stable-process
submitting-a-pull-request
diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst
index 65c64078cb..8624f21673 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -344,28 +344,14 @@ Patch emails must include a ``Signed-off-by:`` line
Your patches **must** include a Signed-off-by: line. This is a hard
requirement because it's how you say "I'm legally okay to contribute
-this and happy for it to go into QEMU". The process is modelled after
-the `Linux kernel
-<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
-policy.
-
-If you wrote the patch, make sure your "From:" and "Signed-off-by:"
-lines use the same spelling. It's okay if you subscribe or contribute to
-the list via more than one address, but using multiple addresses in one
-commit just confuses things. If someone else wrote the patch, git will
-include a "From:" line in the body of the email (different from your
-envelope From:) that will give credit to the correct author; but again,
-that author's Signed-off-by: line is mandatory, with the same spelling.
+this and happy for it to go into QEMU". For full guidance, read the
+:ref:`code-provenance` documentation.
The name used with "Signed-off-by" does not need to be your legal name,
nor birth name, nor appear on any government ID. It is the identity you
choose to be known by in the community, but should not be anonymous,
nor misrepresent whom you are.
-There are various tooling options for automatically adding these tags
-include using ``git commit -s`` or ``git format-patch -s``. For more
-information see `SubmittingPatches 1.12
-<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
.. _include_a_meaningful_cover_letter:
--
2.48.1
Markus Armbruster <armbru@redhat.com> writes:
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> Currently we have a short paragraph saying that patches must include
> a Signed-off-by line, and merely link to the kernel documentation.
> The linked kernel docs have a lot of content beyond the part about
> sign-off an thus are misleading/distracting to QEMU contributors.
>
> This introduces a dedicated 'code-provenance' page in QEMU talking
> about why we require sign-off, explaining the other tags we commonly
> use, and what to do in some edge cases.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++
> docs/devel/index-process.rst | 1 +
> docs/devel/submitting-a-patch.rst | 18 +--
> 3 files changed, 221 insertions(+), 16 deletions(-)
> create mode 100644 docs/devel/code-provenance.rst
>
> diff --git a/docs/devel/code-provenance.rst b/docs/devel/code-provenance.rst
> new file mode 100644
> index 0000000000..4fc12061b5
> --- /dev/null
> +++ b/docs/devel/code-provenance.rst
> @@ -0,0 +1,218 @@
> +.. _code-provenance:
> +
> +Code provenance
> +===============
> +
> +Certifying patch submissions
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The QEMU community **mandates** all contributors to certify provenance of
> +patch submissions they make to the project. To put it another way,
> +contributors must indicate that they are legally permitted to contribute to
> +the project.
> +
> +Certification is achieved with a low overhead by adding a single line to the
> +bottom of every git commit::
s/git commit/commit/ throughout?
> +
> + Signed-off-by: YOUR NAME <YOUR@EMAIL>
> +
> +using a known identity (sorry, no anonymous contributions.)
> +
maybe "(contributions cannot be anonymous)" is more direct?
> +The addition of this line asserts that the author of the patch is contributing
> +in accordance with the clauses specified in the
> +`Developer's Certificate of Origin <https://developercertificate.org>`__:
> +
> +.. _dco:
> +
> + 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.
> +
> +It is generally expected that the name and email addresses used in one of the
> +``Signed-off-by`` lines, matches that of the git commit ``Author`` field.
> +It's okay if you subscribe or contribute to the list via more than one
> +address, but using multiple addresses in one commit just confuses
> +things.
> +
> +If the person sending the mail is not one of the patch authors, they are
> +nonetheless expected to add their own ``Signed-off-by`` to comply with the
> +DCO clause (c).
We should probably mention that sometimes the committer may update the
patch after they have pulled it into the tree. In those cases we preface
the S-o-B tag with a comment:
Signed-off-by: Original Hacker <hacker@domain>
[MH: tweaked the commit message for clarity]
Signed-off-by: Maintainer Hacker <hacker@another.com>
> +
> +Multiple authorship
> +~~~~~~~~~~~~~~~~~~~
> +
> +It is not uncommon for a patch to have contributions from multiple authors. In
> +this scenario, git commits will usually be expected to have a ``Signed-off-by``
> +line for each contributor involved in creation of the patch. Some edge cases:
> +
> + * The non-primary author's contributions were so trivial that they can be
> + considered not subject to copyright. In this case the secondary authors
> + need not include a ``Signed-off-by``.
> +
> + This case most commonly applies where QEMU reviewers give short snippets
> + of code as suggested fixes to a patch. The reviewers don't need to have
> + their own ``Signed-off-by`` added unless their code suggestion was
> + unusually large, but it is common to add ``Suggested-by`` as a credit
> + for non-trivial code.
> +
> + * Both contributors work for the same employer and the employer requires
> + copyright assignment.
> +
> + It can be said that in this case a ``Signed-off-by`` is indicating that
> + the person has permission to contribute from their employer who is the
> + copyright holder. It is nonetheless still preferable to include a
> + ``Signed-off-by`` for each contributor, as in some countries employees are
> + not able to assign copyright to their employer, and it also covers any
> + time invested outside working hours.
> +
> +When multiple ``Signed-off-by`` tags are present, they should be strictly kept
> +in order of authorship, from oldest to newest.
> +
> +Other commit tags
> +~~~~~~~~~~~~~~~~~
> +
> +While the ``Signed-off-by`` tag is mandatory, there are a number of other tags
> +that are commonly used during QEMU development:
> +
> + * **``Reviewed-by``**: when a QEMU community member reviews a patch on the
> + mailing list, if they consider the patch acceptable, they should send an
> + email reply containing a ``Reviewed-by`` tag. Subsystem maintainers who
> + review a patch should add this even if they are also adding their
> + ``Signed-off-by`` to the same commit.
> +
> + * **``Acked-by``**: when a QEMU subsystem maintainer approves a patch that
> + touches their subsystem, but intends to allow a different maintainer to
> + queue it and send a pull request, they would send a mail containing a
> + ``Acked-by`` tag. Where a patch touches multiple subsystems, ``Acked-by``
> + only implies review of the maintainers' own areas of responsibility. If a
> + maintainer wants to indicate they have done a full review they should use
> + a ``Reviewed-by`` tag.
> +
> + * **``Tested-by``**: when a QEMU community member has functionally tested the
> + behaviour of the patch in some manner, they should send an email reply
> + containing a ``Tested-by`` tag.
> +
> + * **``Reported-by``**: when a QEMU community member reports a problem via the
> + mailing list, or some other informal channel that is not the issue tracker,
> + it is good practice to credit them by including a ``Reported-by`` tag on
> + any patch fixing the issue. When the problem is reported via the GitLab
> + issue tracker, however, it is sufficient to just include a link to the
> + issue.
We don't mention the Link: or Message-Id: tags.
> +
> + * **``Suggested-by``**: when a reviewer or other 3rd party makes non-trivial
> + suggestions for how to change a patch, it is good practice to credit them
> + by including a ``Suggested-by`` tag.
> +
> +Subsystem maintainer requirements
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +When a subsystem maintainer accepts a patch from a contributor, in addition to
> +the normal code review points, they are expected to validate the presence of
> +suitable ``Signed-off-by`` tags.
> +
> +At the time they queue the patch in their subsystem tree, the maintainer
> +**must** also then add their own ``Signed-off-by`` to indicate that they have
> +done the aforementioned validation. This is in addition to any of their own
> +``Reviewed-by`` tags the subsystem maintainer may wish to include.
> +
> +Tools for adding ``Signed-off-by``
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +There are a variety of ways tools can support adding ``Signed-off-by`` tags
> +for patches, avoiding the need for contributors to manually type in this
> +repetitive text each time.
> +
> +git commands
> +^^^^^^^^^^^^
> +
> +When creating, or amending, a commit the ``-s`` flag to ``git commit`` will
> +append a suitable line matching the configured git author details.
> +
> +If preparing patches using the ``git format-patch`` tool, the ``-s`` flag can
> +be used to append a suitable line in the emails it creates, without modifying
> +the local commits. Alternatively to modify all the local commits on a branch::
> +
> + git rebase master -x 'git commit --amend --no-edit -s'
> +
Much as I love Emacs I wonder if this next section is worth it given the
multiple ways you can solve this (I use yas-snippet expansions for
example).
If we do want to mention the editors we should probably also mention b4.
> +emacs
> +^^^^^
> +
> +In the file ``$HOME/.emacs.d/abbrev_defs`` add:
> +
> +.. code:: elisp
> +
> + (define-abbrev-table 'global-abbrev-table
> + '(
> + ("8rev" "Reviewed-by: YOUR NAME <your@email.addr>" nil 1)
> + ("8ack" "Acked-by: YOUR NAME <your@email.addr>" nil 1)
> + ("8test" "Tested-by: YOUR NAME <your@email.addr>" nil 1)
> + ("8sob" "Signed-off-by: YOUR NAME <your@email.addr>" nil 1)
> + ))
> +
> +with this change, if you type (for example) ``8rev`` followed by ``<space>``
> +or ``<enter>`` it will expand to the whole phrase.
> +
> +vim
> +^^^
> +
> +In the file ``$HOME/.vimrc`` add::
> +
> + iabbrev 8rev Reviewed-by: YOUR NAME <your@email.addr>
> + iabbrev 8ack Acked-by: YOUR NAME <your@email.addr>
> + iabbrev 8test Tested-by: YOUR NAME <your@email.addr>
> + iabbrev 8sob Signed-off-by: YOUR NAME <your@email.addr>
> +
> +with this change, if you type (for example) ``8rev`` followed by ``<space>``
> +or ``<enter>`` it will expand to the whole phrase.
> +
> +Re-starting abandoned work
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +For a variety of reasons there are some patches that get submitted to QEMU but
> +never merged. An unrelated contributor may decide (months or years later) to
> +continue working from the abandoned patch and re-submit it with extra changes.
> +
> +The general principles when picking up abandoned work are:
> +
> + * Continue to credit the original author for their work, by maintaining their
> + original ``Signed-off-by``
> + * Indicate where the original patch was obtained from (mailing list, bug
> + tracker, author's git repo, etc) when sending it for review
> + * Acknowledge the extra work of the new contributor by including their
> + ``Signed-off-by`` in the patch in addition to the orignal author's
> + * Indicate who is responsible for what parts of the patch. This is typically
> + done via a note in the commit message, just prior to the new contributor's
> + ``Signed-off-by``::
> +
> + Signed-off-by: Some Person <some.person@example.com>
> + [Rebased and added support for 'foo']
> + Signed-off-by: New Person <new.person@mycorp.test>
> +
> +In complicated cases, or if otherwise unsure, ask for advice on the project
> +mailing list.
> +
> +It is also recommended to attempt to contact the original author to let them
> +know you are interested in taking over their work, in case they still intended
> +to return to the work, or had any suggestions about the best way to continue.
> diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst
> index cb7c6640fd..5807752d70 100644
> --- a/docs/devel/index-process.rst
> +++ b/docs/devel/index-process.rst
> @@ -13,6 +13,7 @@ Notes about how to interact with the community and how and where to submit patch
> maintainers
> style
> submitting-a-patch
> + code-provenance
> trivial-patches
> stable-process
> submitting-a-pull-request
> diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst
> index 65c64078cb..8624f21673 100644
> --- a/docs/devel/submitting-a-patch.rst
> +++ b/docs/devel/submitting-a-patch.rst
> @@ -344,28 +344,14 @@ Patch emails must include a ``Signed-off-by:`` line
>
> Your patches **must** include a Signed-off-by: line. This is a hard
> requirement because it's how you say "I'm legally okay to contribute
> -this and happy for it to go into QEMU". The process is modelled after
> -the `Linux kernel
> -<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
> -policy.
> -
> -If you wrote the patch, make sure your "From:" and "Signed-off-by:"
> -lines use the same spelling. It's okay if you subscribe or contribute to
> -the list via more than one address, but using multiple addresses in one
> -commit just confuses things. If someone else wrote the patch, git will
> -include a "From:" line in the body of the email (different from your
> -envelope From:) that will give credit to the correct author; but again,
> -that author's Signed-off-by: line is mandatory, with the same spelling.
> +this and happy for it to go into QEMU". For full guidance, read the
> +:ref:`code-provenance` documentation.
>
> The name used with "Signed-off-by" does not need to be your legal name,
> nor birth name, nor appear on any government ID. It is the identity you
> choose to be known by in the community, but should not be anonymous,
> nor misrepresent whom you are.
>
> -There are various tooling options for automatically adding these tags
> -include using ``git commit -s`` or ``git format-patch -s``. For more
> -information see `SubmittingPatches 1.12
> -<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
>
> .. _include_a_meaningful_cover_letter:
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Alex Bennée <alex.bennee@linaro.org> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> From: Daniel P. Berrangé <berrange@redhat.com>
>>
>> Currently we have a short paragraph saying that patches must include
>> a Signed-off-by line, and merely link to the kernel documentation.
>> The linked kernel docs have a lot of content beyond the part about
>> sign-off an thus are misleading/distracting to QEMU contributors.
>>
>> This introduces a dedicated 'code-provenance' page in QEMU talking
>> about why we require sign-off, explaining the other tags we commonly
>> use, and what to do in some edge cases.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++
>> docs/devel/index-process.rst | 1 +
>> docs/devel/submitting-a-patch.rst | 18 +--
>> 3 files changed, 221 insertions(+), 16 deletions(-)
>> create mode 100644 docs/devel/code-provenance.rst
>>
>> diff --git a/docs/devel/code-provenance.rst b/docs/devel/code-provenance.rst
>> new file mode 100644
>> index 0000000000..4fc12061b5
>> --- /dev/null
>> +++ b/docs/devel/code-provenance.rst
>> @@ -0,0 +1,218 @@
>> +.. _code-provenance:
>> +
>> +Code provenance
>> +===============
>> +
>> +Certifying patch submissions
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +The QEMU community **mandates** all contributors to certify provenance of
>> +patch submissions they make to the project. To put it another way,
>> +contributors must indicate that they are legally permitted to contribute to
>> +the project.
>> +
>> +Certification is achieved with a low overhead by adding a single line to the
>> +bottom of every git commit::
>
> s/git commit/commit/ throughout?
Yes.
>> +
>> + Signed-off-by: YOUR NAME <YOUR@EMAIL>
>> +
>> +using a known identity (sorry, no anonymous contributions.)
>> +
>
> maybe "(contributions cannot be anonymous)" is more direct?
If we're deviating from the kernel's text (which is *fine*), let's get
rid of the parenthesis:
using a known identity. Contributions cannot be anonymous.
or in active voice:
using a known identity. We cannot accept anonymous contributions.
I like this one the best.
>> +The addition of this line asserts that the author of the patch is contributing
>> +in accordance with the clauses specified in the
>> +`Developer's Certificate of Origin <https://developercertificate.org>`__:
>> +
>> +.. _dco:
>> +
>> + 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.
>> +
>> +It is generally expected that the name and email addresses used in one of the
>> +``Signed-off-by`` lines, matches that of the git commit ``Author`` field.
>> +It's okay if you subscribe or contribute to the list via more than one
>> +address, but using multiple addresses in one commit just confuses
>> +things.
>> +
>> +If the person sending the mail is not one of the patch authors, they are
>> +nonetheless expected to add their own ``Signed-off-by`` to comply with the
>> +DCO clause (c).
>
> We should probably mention that sometimes the committer may update the
> patch after they have pulled it into the tree. In those cases we preface
> the S-o-B tag with a comment:
>
> Signed-off-by: Original Hacker <hacker@domain>
> [MH: tweaked the commit message for clarity]
> Signed-off-by: Maintainer Hacker <hacker@another.com>
Good idea. Should this go here or under "Subsystem maintainer
requirements"?
>> +
>> +Multiple authorship
>> +~~~~~~~~~~~~~~~~~~~
>> +
>> +It is not uncommon for a patch to have contributions from multiple authors. In
>> +this scenario, git commits will usually be expected to have a ``Signed-off-by``
>> +line for each contributor involved in creation of the patch. Some edge cases:
>> +
>> + * The non-primary author's contributions were so trivial that they can be
>> + considered not subject to copyright. In this case the secondary authors
>> + need not include a ``Signed-off-by``.
>> +
>> + This case most commonly applies where QEMU reviewers give short snippets
>> + of code as suggested fixes to a patch. The reviewers don't need to have
>> + their own ``Signed-off-by`` added unless their code suggestion was
>> + unusually large, but it is common to add ``Suggested-by`` as a credit
>> + for non-trivial code.
>> +
>> + * Both contributors work for the same employer and the employer requires
>> + copyright assignment.
>> +
>> + It can be said that in this case a ``Signed-off-by`` is indicating that
>> + the person has permission to contribute from their employer who is the
>> + copyright holder. It is nonetheless still preferable to include a
>> + ``Signed-off-by`` for each contributor, as in some countries employees are
>> + not able to assign copyright to their employer, and it also covers any
>> + time invested outside working hours.
>> +
>> +When multiple ``Signed-off-by`` tags are present, they should be strictly kept
>> +in order of authorship, from oldest to newest.
>> +
>> +Other commit tags
>> +~~~~~~~~~~~~~~~~~
>> +
>> +While the ``Signed-off-by`` tag is mandatory, there are a number of other tags
>> +that are commonly used during QEMU development:
>> +
>> + * **``Reviewed-by``**: when a QEMU community member reviews a patch on the
>> + mailing list, if they consider the patch acceptable, they should send an
>> + email reply containing a ``Reviewed-by`` tag. Subsystem maintainers who
>> + review a patch should add this even if they are also adding their
>> + ``Signed-off-by`` to the same commit.
>> +
>> + * **``Acked-by``**: when a QEMU subsystem maintainer approves a patch that
>> + touches their subsystem, but intends to allow a different maintainer to
>> + queue it and send a pull request, they would send a mail containing a
>> + ``Acked-by`` tag. Where a patch touches multiple subsystems, ``Acked-by``
>> + only implies review of the maintainers' own areas of responsibility. If a
>> + maintainer wants to indicate they have done a full review they should use
>> + a ``Reviewed-by`` tag.
>> +
>> + * **``Tested-by``**: when a QEMU community member has functionally tested the
>> + behaviour of the patch in some manner, they should send an email reply
>> + containing a ``Tested-by`` tag.
>> +
>> + * **``Reported-by``**: when a QEMU community member reports a problem via the
>> + mailing list, or some other informal channel that is not the issue tracker,
>> + it is good practice to credit them by including a ``Reported-by`` tag on
>> + any patch fixing the issue. When the problem is reported via the GitLab
>> + issue tracker, however, it is sufficient to just include a link to the
>> + issue.
>
> We don't mention the Link: or Message-Id: tags.
Yes, but should it go into code-provenance.rst or
submitting-a-patch.rst?
You asked for guidance on use of "Message-Id:" in your review of v2. I
understand the practice, and can write guidance, but I wanted to get
this out before my vacation next week, so I left it for later, as
mentioned in the cover letter.
How do we use "Link:"? What about "Closes:"?
Here's what the kernel's submitting-patches.rst has to say:
Describe your changes
---------------------
[...]
If related discussions or any other background information behind the change
can be found on the web, add 'Link:' tags pointing to it. If the patch is a
result of some earlier mailing list discussions or something documented on the
web, point to it.
When linking to mailing list archives, preferably use the lore.kernel.org
message archiver service. To create the link URL, use the contents of the
``Message-ID`` header of the message without the surrounding angle brackets.
For example::
Link: https://lore.kernel.org/30th.anniversary.repost@klaava.Helsinki.FI
Please check the link to make sure that it is actually working and points
to the relevant message.
However, try to make your explanation understandable without external
resources. In addition to giving a URL to a mailing list archive or bug,
summarize the relevant points of the discussion that led to the
patch as submitted.
In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
the report in the mailing list archives or a public bug tracker. For example::
Closes: https://example.com/issues/1234
Some bug trackers have the ability to close issues automatically when a
commit with such a tag is applied. Some bots monitoring mailing lists can
also track such tags and take certain actions. Private bug trackers and
invalid URLs are forbidden.
and
Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
----------------------------------------------------------------------
The Reported-by tag gives credit to people who find bugs and report them and it
hopefully inspires them to help us again in the future. The tag is intended for
bugs; please do not use it to credit feature requests. The tag should be
followed by a Closes: tag pointing to the report, unless the report is not
available on the web. The Link: tag can be used instead of Closes: if the patch
fixes a part of the issue(s) being reported. Note, the Reported-by tag is one
of only three tags you might be able to use without explicit permission of the
person named (see 'Tagging people requires permission' below for details).
>> +
>> + * **``Suggested-by``**: when a reviewer or other 3rd party makes non-trivial
>> + suggestions for how to change a patch, it is good practice to credit them
>> + by including a ``Suggested-by`` tag.
>> +
>> +Subsystem maintainer requirements
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +When a subsystem maintainer accepts a patch from a contributor, in addition to
>> +the normal code review points, they are expected to validate the presence of
>> +suitable ``Signed-off-by`` tags.
>> +
>> +At the time they queue the patch in their subsystem tree, the maintainer
>> +**must** also then add their own ``Signed-off-by`` to indicate that they have
>> +done the aforementioned validation. This is in addition to any of their own
>> +``Reviewed-by`` tags the subsystem maintainer may wish to include.
>> +
>> +Tools for adding ``Signed-off-by``
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +There are a variety of ways tools can support adding ``Signed-off-by`` tags
>> +for patches, avoiding the need for contributors to manually type in this
>> +repetitive text each time.
>> +
>> +git commands
>> +^^^^^^^^^^^^
>> +
>> +When creating, or amending, a commit the ``-s`` flag to ``git commit`` will
>> +append a suitable line matching the configured git author details.
>> +
>> +If preparing patches using the ``git format-patch`` tool, the ``-s`` flag can
>> +be used to append a suitable line in the emails it creates, without modifying
>> +the local commits. Alternatively to modify all the local commits on a branch::
>> +
>> + git rebase master -x 'git commit --amend --no-edit -s'
>> +
>
> Much as I love Emacs I wonder if this next section is worth it given the
> multiple ways you can solve this (I use yas-snippet expansions for
> example).
Showing one of them could still be useful for less experienced Emacs
users. We could mention it's just of many ways.
> If we do want to mention the editors we should probably also mention b4.
Can do if somebody contributes a suitable configuration snippet.
Thanks!
[...]
Hi, > > If we do want to mention the editors we should probably also mention b4. > > Can do if somebody contributes a suitable configuration snippet. Nothing to configure ;) Simplest usage is 'b4 shazam $msgid' and b4 will go fetch the complete thread from lore.kernel.org, collect all the review tags from the replies, add them to the patches and apply the whole series to the current branch. You can also ask b4 to generate a mbox file you can feed to 'git am' yourself (this is 'b4 am $msgid'), which can be useful if you want build your maintainer scripting workflow around it. take care, Gerd
On 4/6/25 08:44, Markus Armbruster wrote: > Alex Bennée <alex.bennee@linaro.org> writes: > >> Markus Armbruster <armbru@redhat.com> writes: >> >>> From: Daniel P. Berrangé <berrange@redhat.com> >>> >>> Currently we have a short paragraph saying that patches must include >>> a Signed-off-by line, and merely link to the kernel documentation. >>> The linked kernel docs have a lot of content beyond the part about >>> sign-off an thus are misleading/distracting to QEMU contributors. >>> >>> This introduces a dedicated 'code-provenance' page in QEMU talking >>> about why we require sign-off, explaining the other tags we commonly >>> use, and what to do in some edge cases. >>> >>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++ >>> docs/devel/index-process.rst | 1 + >>> docs/devel/submitting-a-patch.rst | 18 +-- >>> 3 files changed, 221 insertions(+), 16 deletions(-) >>> create mode 100644 docs/devel/code-provenance.rst >>> + >>> + Signed-off-by: YOUR NAME <YOUR@EMAIL> >>> + >>> +using a known identity (sorry, no anonymous contributions.) >>> + >> >> maybe "(contributions cannot be anonymous)" is more direct? > > If we're deviating from the kernel's text (which is *fine*), let's get > rid of the parenthesis: > > using a known identity. Contributions cannot be anonymous. > > or in active voice: > > using a known identity. We cannot accept anonymous contributions. I'd add an anchor in the "commonly known identity" paragraph added in commit 270c81b7d59 and here link to it. > > I like this one the best.
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 4/6/25 08:44, Markus Armbruster wrote: >> Alex Bennée <alex.bennee@linaro.org> writes: >> >>> Markus Armbruster <armbru@redhat.com> writes: >>> >>>> From: Daniel P. Berrangé <berrange@redhat.com> >>>> >>>> Currently we have a short paragraph saying that patches must include >>>> a Signed-off-by line, and merely link to the kernel documentation. >>>> The linked kernel docs have a lot of content beyond the part about >>>> sign-off an thus are misleading/distracting to QEMU contributors. >>>> >>>> This introduces a dedicated 'code-provenance' page in QEMU talking >>>> about why we require sign-off, explaining the other tags we commonly >>>> use, and what to do in some edge cases. >>>> >>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> >>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >>>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>>> --- >>>> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++ >>>> docs/devel/index-process.rst | 1 + >>>> docs/devel/submitting-a-patch.rst | 18 +-- >>>> 3 files changed, 221 insertions(+), 16 deletions(-) >>>> create mode 100644 docs/devel/code-provenance.rst > > >>>> + >>>> + Signed-off-by: YOUR NAME <YOUR@EMAIL> >>>> + >>>> +using a known identity (sorry, no anonymous contributions.) >>>> + >>> >>> maybe "(contributions cannot be anonymous)" is more direct? >> If we're deviating from the kernel's text (which is *fine*), let's get >> rid of the parenthesis: >> using a known identity. Contributions cannot be anonymous. >> or in active voice: >> using a known identity. We cannot accept anonymous contributions. > > I'd add an anchor in the "commonly known identity" paragraph added in > commit 270c81b7d59 and here link to it. Makes sense, thanks! >> I like this one the best.
On Wed, Jun 04, 2025 at 08:44:55AM +0200, Markus Armbruster wrote: > Alex Bennée <alex.bennee@linaro.org> writes: > > > Markus Armbruster <armbru@redhat.com> writes: > > > >> From: Daniel P. Berrangé <berrange@redhat.com> > >> > >> Currently we have a short paragraph saying that patches must include > >> a Signed-off-by line, and merely link to the kernel documentation. > >> The linked kernel docs have a lot of content beyond the part about > >> sign-off an thus are misleading/distracting to QEMU contributors. > >> > >> This introduces a dedicated 'code-provenance' page in QEMU talking > >> about why we require sign-off, explaining the other tags we commonly > >> use, and what to do in some edge cases. > >> > >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> docs/devel/code-provenance.rst | 218 ++++++++++++++++++++++++++++++ > >> docs/devel/index-process.rst | 1 + > >> docs/devel/submitting-a-patch.rst | 18 +-- > >> 3 files changed, 221 insertions(+), 16 deletions(-) > >> create mode 100644 docs/devel/code-provenance.rst > >> > >> diff --git a/docs/devel/code-provenance.rst b/docs/devel/code-provenance.rst > >> new file mode 100644 > >> index 0000000000..4fc12061b5 > >> --- /dev/null > >> +++ b/docs/devel/code-provenance.rst > >> +Other commit tags > >> +~~~~~~~~~~~~~~~~~ > >> + > >> +While the ``Signed-off-by`` tag is mandatory, there are a number of other tags > >> +that are commonly used during QEMU development: > >> + > >> + * **``Reviewed-by``**: when a QEMU community member reviews a patch on the > >> + mailing list, if they consider the patch acceptable, they should send an > >> + email reply containing a ``Reviewed-by`` tag. Subsystem maintainers who > >> + review a patch should add this even if they are also adding their > >> + ``Signed-off-by`` to the same commit. > >> + > >> + * **``Acked-by``**: when a QEMU subsystem maintainer approves a patch that > >> + touches their subsystem, but intends to allow a different maintainer to > >> + queue it and send a pull request, they would send a mail containing a > >> + ``Acked-by`` tag. Where a patch touches multiple subsystems, ``Acked-by`` > >> + only implies review of the maintainers' own areas of responsibility. If a > >> + maintainer wants to indicate they have done a full review they should use > >> + a ``Reviewed-by`` tag. > >> + > >> + * **``Tested-by``**: when a QEMU community member has functionally tested the > >> + behaviour of the patch in some manner, they should send an email reply > >> + containing a ``Tested-by`` tag. > >> + > >> + * **``Reported-by``**: when a QEMU community member reports a problem via the > >> + mailing list, or some other informal channel that is not the issue tracker, > >> + it is good practice to credit them by including a ``Reported-by`` tag on > >> + any patch fixing the issue. When the problem is reported via the GitLab > >> + issue tracker, however, it is sufficient to just include a link to the > >> + issue. > > > > We don't mention the Link: or Message-Id: tags. > > Yes, but should it go into code-provenance.rst or > submitting-a-patch.rst? I considered those other general tags to be under the scope of submitting-a-patch.rst, as they're not directly related to the legal code provenance. > > You asked for guidance on use of "Message-Id:" in your review of v2. I > understand the practice, and can write guidance, but I wanted to get > this out before my vacation next week, so I left it for later, as > mentioned in the cover letter. > > How do we use "Link:"? What about "Closes:"? > > Here's what the kernel's submitting-patches.rst has to say: > > Describe your changes > --------------------- > > [...] > > If related discussions or any other background information behind the change > can be found on the web, add 'Link:' tags pointing to it. If the patch is a > result of some earlier mailing list discussions or something documented on the > web, point to it. > > When linking to mailing list archives, preferably use the lore.kernel.org > message archiver service. To create the link URL, use the contents of the > ``Message-ID`` header of the message without the surrounding angle brackets. > For example:: > > Link: https://lore.kernel.org/30th.anniversary.repost@klaava.Helsinki.FI > > Please check the link to make sure that it is actually working and points > to the relevant message. > > However, try to make your explanation understandable without external > resources. In addition to giving a URL to a mailing list archive or bug, > summarize the relevant points of the discussion that led to the > patch as submitted. > > In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing > the report in the mailing list archives or a public bug tracker. For example:: > > Closes: https://example.com/issues/1234 > > Some bug trackers have the ability to close issues automatically when a > commit with such a tag is applied. Some bots monitoring mailing lists can > also track such tags and take certain actions. Private bug trackers and > invalid URLs are forbidden. > > and > > Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: > ---------------------------------------------------------------------- > > The Reported-by tag gives credit to people who find bugs and report them and it > hopefully inspires them to help us again in the future. The tag is intended for > bugs; please do not use it to credit feature requests. The tag should be > followed by a Closes: tag pointing to the report, unless the report is not > available on the web. The Link: tag can be used instead of Closes: if the patch > fixes a part of the issue(s) being reported. Note, the Reported-by tag is one > of only three tags you might be able to use without explicit permission of the > person named (see 'Tagging people requires permission' below for details). > > >> +git commands > >> +^^^^^^^^^^^^ > >> + > >> +When creating, or amending, a commit the ``-s`` flag to ``git commit`` will > >> +append a suitable line matching the configured git author details. > >> + > >> +If preparing patches using the ``git format-patch`` tool, the ``-s`` flag can > >> +be used to append a suitable line in the emails it creates, without modifying > >> +the local commits. Alternatively to modify all the local commits on a branch:: > >> + > >> + git rebase master -x 'git commit --amend --no-edit -s' > >> + > > > > Much as I love Emacs I wonder if this next section is worth it given the > > multiple ways you can solve this (I use yas-snippet expansions for > > example). > > Showing one of them could still be useful for less experienced Emacs > users. We could mention it's just of many ways. Yep, IMHO it is worth guiding users to a simple example that works. If they are advanced users of emacs or other editors wanting to figure out other options they can ignore this guidance. > > > If we do want to mention the editors we should probably also mention b4. > > Can do if somebody contributes a suitable configuration snippet. > > Thanks! > > [...] > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.