Submitting Xen Project Patches: Difference between revisions

From Xen
Jump to navigationJump to search
mNo edit summary
m (Removed superfluous <nowiki></nowiki> tag pairs.)
Line 14: Line 14:
Here's a recommendation on how to send patches, as suggested by the Xen maintainers.
Here's a recommendation on how to send patches, as suggested by the Xen maintainers.


To begin, configure Mercurial by making the following additions to your <code><nowiki>~/.hgrc</nowiki></code>:
To begin, configure Mercurial by making the following additions to your <code>~/.hgrc</code>:


* Enable the [http://mercurial.selenic.com/wiki/MqExtension Patchqueue extension]
* Enable the [http://mercurial.selenic.com/wiki/MqExtension Patchqueue extension]
<pre>
<pre><nowiki>[extensions]
[extensions]
mq=</nowiki></pre>
mq=
</pre>




* Show function names in patch hunk headers (which makes patches easier to review).
* Show function names in patch hunk headers (which makes patches easier to review).
<pre><nowiki>[diff]
<pre>
[diff]
showfunc=True</nowiki></pre>
showfunc=True
</pre>




When you want to make a patch, create a patch on the patch queue:
When you want to make a patch, create a patch on the patch queue:
<pre>
<pre><nowiki>hg qnew patch01.diff
hg qnew patch01.diff
</nowiki></pre>
</pre>




Then edit the source files you want to change. You can see which files have changed by using <code><nowiki>hg status</nowiki></code>.
Then edit the source files you want to change. You can see which files have changed by using <code>hg status</code>.


When you're done editing, save your changes into the patch, and add a description. The easiest way to add a description is to use the <code><nowiki>-e</nowiki></code> option to <code><nowiki>hg qrefresh</nowiki></code>:
When you're done editing, save your changes into the patch, and add a description. The easiest way to add a description is to use the <code>-e</code> option to <code>hg qrefresh</code>:
<pre>
<pre><nowiki>hg qrefresh -e
hg qrefresh -e
</nowiki></pre>
</pre>


<pre>
<pre><nowiki>foobar: Add a new trondle calls
foobar: Add a new trondle calls


Add a some new trondle calls to the foobar interface to support
Add a some new trondle calls to the foobar interface to support
Line 48: Line 55:


Signed-off-by: Joe Smith <joe.smith@citrix.com>
Signed-off-by: Joe Smith <joe.smith@citrix.com>
</nowiki></pre>
</pre>


Alternately, after refreshing the patch with <code><nowiki>hg qrefresh</nowiki></code>, you can edit the files directly in <code><nowiki>.hg/patches/</nowiki></code>. Everything above the line starting with <code><nowiki>diff</nowiki></code> will be ignored:
Alternately, after refreshing the patch with <code>hg qrefresh</code>, you can edit the files directly in <code>.hg/patches/</code>. Everything above the line starting with <code>diff</code> will be ignored:
<pre>
<pre><nowiki>foobar: Add a new trondle calls
foobar: Add a new trondle calls


Add a some new trondle calls to the foobar interface to support
Add a some new trondle calls to the foobar interface to support
Line 62: Line 70:
--- a/tools/libxc/Makefile Mon Dec 07 17:01:11 2009 +0000
--- a/tools/libxc/Makefile Mon Dec 07 17:01:11 2009 +0000
+++ b/tools/libxc/Makefile Mon Dec 21 11:45:00 2009 +0000
+++ b/tools/libxc/Makefile Mon Dec 21 11:45:00 2009 +0000
...
...</nowiki></pre>
</pre>


Repeat as necessary for all of your changes
Repeat as necessary for all of your changes
<pre>
<pre><nowiki>hg qnew patch02.diff
hg qnew patch02.diff
</nowiki></pre>
</pre>


...
...
Line 75: Line 85:


<pre>
<pre><nowiki>Developer's Certificate of Origin 1.1
Developer's Certificate of Origin 1.1


By making a contribution to this project, I certify that:
By making a contribution to this project, I certify that:
Line 100: Line 111:
maintained indefinitely and may be redistributed consistent with
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
this project or the open source license(s) involved.
</nowiki></pre>
</pre>


=== Making good patches ===
=== Making good patches ===
Line 115: Line 126:
* Don't mix clean-up patches (which make things look prettier or move things round but don't change functionality) with code-change patches. Clean-up patches should be clearly marked as having no functional changes.
* Don't mix clean-up patches (which make things look prettier or move things round but don't change functionality) with code-change patches. Clean-up patches should be clearly marked as having no functional changes.
==== Write a good description for each patch ====
==== Write a good description for each patch ====
* If you're using mercurial queues, as described above, use <code><nowiki>hg qrefresh -e</nowiki></code> to edit the description, or edit the patch files directly in <code><nowiki>.hg/patches/</nowiki></code>. NB that if you edit the files directly, you'll have to <code><nowiki>hg qpop -a ; hg qpush -a</nowiki></code> to get the description into mercurial.
* If you're using mercurial queues, as described above, use <code>hg qrefresh -e</code> to edit the description, or edit the patch files directly in <code>.hg/patches/</code>. NB that if you edit the files directly, you'll have to <code>hg qpop -a ; hg qpush -a</code> to get the description into mercurial.
* The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches
* The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches
* The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as <code><nowiki>Code cleanup -- no functional changes</nowiki></code>, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing.
* The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as <code>Code cleanup -- no functional changes</code>, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing.


=== Sending the patches to the list ===
=== Sending the patches to the list ===
Line 123: Line 134:
The most robust way to send files is by using the [http://mercurial.selenic.com/wiki/PatchbombExtension Mercurial Patchbomb extension].
The most robust way to send files is by using the [http://mercurial.selenic.com/wiki/PatchbombExtension Mercurial Patchbomb extension].


To do this, first set up the Patchbomb extension in your <code><nowiki>.hgrc</nowiki></code> file:
To do this, first set up the Patchbomb extension in your <code>.hgrc</code> file:
<pre>
<pre><nowiki>[extensions]
[extensions]
hgext.patchbomb =
hgext.patchbomb =


Line 136: Line 148:
[smtp]
[smtp]
host = mail.yourdomain.com
host = mail.yourdomain.com
port = 25</nowiki></pre>
port = 25
</pre>


Then check to make sure you have the right patches to be submitted by using <code><nowiki>hg out</nowiki></code>:
Then check to make sure you have the right patches to be submitted by using <code>hg out</code>:
<pre><nowiki> $ hg out
<pre>
$ hg out
comparing with http://hg/xen-unstable.hg
comparing with http://hg/xen-unstable.hg
searching for changes
searching for changes
Line 150: Line 164:
summary: foobar: Add new trondle calls
summary: foobar: Add new trondle calls


...
...</nowiki></pre>
</pre>


Once you're sure you have the proper series of patches, you're ready to use the patchbomb tool:
Once you're sure you have the proper series of patches, you're ready to use the patchbomb tool:
<pre>
<pre><nowiki>$ hg email -o --plain
$ hg email -o --plain
</nowiki></pre>
</pre>


(The -o option tels patchbomb to submit all outgoing patches; --plain removes the extraneous HG header information from the patches in the e-mails.) It will ask you some questions, like this:
(The -o option tels patchbomb to submit all outgoing patches; --plain removes the extraneous HG header information from the patches in the e-mails.) It will ask you some questions, like this:
<pre>
<pre><nowiki>comparing with http://hg/xen-unstable.hg
comparing with http://hg/xen-unstable.hg
searching for changes
searching for changes
This patch series consists of 5 patches.
This patch series consists of 5 patches.


Subject: [PATCH 0 of 5]</nowiki></pre>
Subject: [PATCH 0 of 5]
</pre>





Revision as of 23:43, 28 November 2011


How To Generate and Submit a Patch to Xen

Xen uses Mercurial for its source code management. The current Xen development tree is at http://xenbits.xensource.com/xen-unstable.hg. An excellent tutorial is here. Download and install mercurial to your system.

Generating a patch


Here's a recommendation on how to send patches, as suggested by the Xen maintainers.

To begin, configure Mercurial by making the following additions to your ~/.hgrc:

[extensions]
mq=


  • Show function names in patch hunk headers (which makes patches easier to review).
[diff]
showfunc=True


When you want to make a patch, create a patch on the patch queue:

hg qnew patch01.diff


Then edit the source files you want to change. You can see which files have changed by using hg status.

When you're done editing, save your changes into the patch, and add a description. The easiest way to add a description is to use the -e option to hg qrefresh:

hg qrefresh -e


foobar: Add a new trondle calls

Add a some new trondle calls to the foobar interface to support
the new zot feature.

Signed-off-by: Joe Smith <joe.smith@citrix.com>

Alternately, after refreshing the patch with hg qrefresh, you can edit the files directly in .hg/patches/. Everything above the line starting with diff will be ignored:

foobar: Add a new trondle calls

Add a some new trondle calls to the foobar interface to support
the new zot feature.

Signed-off-by: Joe Smith <joe.smith@citrix.com>

diff -r 63531e640828 tools/libxc/Makefile
--- a/tools/libxc/Makefile	Mon Dec 07 17:01:11 2009 +0000
+++ b/tools/libxc/Makefile	Mon Dec 21 11:45:00 2009 +0000
...

Repeat as necessary for all of your changes

hg qnew patch02.diff

...

Signing off a patch


All patches to the Xen code base must include the the line "Signed-off-by: your_name <your_email>" at the end of the change description. This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states:


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.

Making good patches


Patches, if accepted, will turn into commits in the mercurial source tree. There are three people to think about when writing the patch and the comment:

  • Reviewers on the mailing list, who are trying to understand what your patch does, if it's correct, and what side effects it will have.
  • People skimming through changesets deciding if a patch is important for them to look at for backporting, review, implications on out-of-tree patches, &c.
  • Someone in the perhaps distant future, who is trying to understand why the code is the way it is.

Try to make your patches with all of these people in mind. To do this:

Break down your patches

  • Each patch should preferably do one logical thing (or one related set of things). The goal is for reviewers to understand the change that each patch is making with a minimum of effort.
  • No patch should ever break existing functionality.
  • Never fix bugs in one of your patches by changing it in a later patch; go back and change the patch with the bug in it.
  • Don't mix clean-up patches (which make things look prettier or move things round but don't change functionality) with code-change patches. Clean-up patches should be clearly marked as having no functional changes.

Write a good description for each patch

  • If you're using mercurial queues, as described above, use hg qrefresh -e to edit the description, or edit the patch files directly in .hg/patches/. NB that if you edit the files directly, you'll have to hg qpop -a ; hg qpush -a to get the description into mercurial.
  • The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches
  • The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as Code cleanup -- no functional changes, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing.

Sending the patches to the list


The most robust way to send files is by using the Mercurial Patchbomb extension.

To do this, first set up the Patchbomb extension in your .hgrc file:

[extensions]
hgext.patchbomb =

[email]
method = smtp
from = Your Name <your.email@your-domain.com>
to = xen-devel@lists.xensource.com
cc = your.email@your-domain.com

[smtp]
host = mail.yourdomain.com
port = 25

Then check to make sure you have the right patches to be submitted by using hg out:

$ hg out
comparing with http://hg/xen-unstable.hg
searching for changes
changeset:   21165:2631707c54b3
tag:         qbase
tag:         add-trondle-call.diff
user:        Joe Smith <joe.smith@citrix.com>
date:        Wed Apr 14 11:16:58 2010 +0100
summary:     foobar: Add new trondle calls

...

Once you're sure you have the proper series of patches, you're ready to use the patchbomb tool:

$ hg email -o --plain

(The -o option tels patchbomb to submit all outgoing patches; --plain removes the extraneous HG header information from the patches in the e-mails.) It will ask you some questions, like this:

comparing with http://hg/xen-unstable.hg
searching for changes
This patch series consists of 5 patches.

Subject: [PATCH 0 of 5]


Enter in a summary for the entire patch series here, and press enter. You will then be dropped into an editor, to write the text for the patch series intro e-mail.

NB, emacs users: if you're running this from an emacs shell, this can get tricky, since all of the editors it wants to offer you require a terminal. Best to run it in a normal terminal window somewhere.

Tada! You should shortly see your patch series appear in your inbox, and on the xen-devel mailing list (depending on how you've configured it.)

Other useful options include:

  • -n will go through all the motions of preparing the patchbomb, but instead of sending a mail, will just output the mails it would have sent. Useful for testing.
  • -d produces a "diffstat", showing lines added and removed for each file. It asks a lot more quesitons, though.

What If


  • Your patch is really big?
  • You have lots and lots of patches? If you are going to generate > 20 patches a week:
    • You might want to host a repository tree that the maintainers can pull from, talk to the maintainers
  • My patch is not accepted (e.g. no comments)?
    • Re-send the patch after 2-4 weeks, with a comment to the effect that you didn't get a response last time; with [RESEND] in the subject line, to alert people that the last mail was dropped
  • Guidelines??? See Xen.org Blog Post at http://blog.xen.org/index.php/2009/09/16/submitting-patches-to-xen-org-community/