- One Patch Per Independent Change
- Contribution Email Subject Line
- Detailed Explanation of the Patch
- FSF copyright Assignment
- Copyright Header
- Properly Formatted GNU ChangeLog
- Properly Formatted Changes
- Submitting patches
- Patch status
- Ping and keep pinging
- Properly formatted commit messages
- Fixing commit dates
A high-quality contribution includes the following things:
1. One Patch Per Independent Change
It is very important that independent changes be split into separate patches, as it has many advantages:
- It helps the review process, since each patch is therefore smaller, increasing the chances of being able to accept each patch faster;
- When investigating which patch introduced a change of behavior, once the patch is found, it is easier to determine why it changes the behavior if the patch is smaller;
- Very seldomly, a patch causes a regression severe enough that, when unable to fix the regression immediately, a revert is needed. When that happens, smaller patches means we end up reverting only the part that causes the problem.
Typical examples where splitting patches up will be requested:
- Patches that make unrelated formatting changes while also fixing a problem;
- Patches that combine fixes for multiple problems;
- Patches that combine multiple enhancements;
Also, for fixes/enhancements which you implemented in multiple logical steps, we encourage you to also submit your changes in multiple commits, each commit corresponding to each logical step. For instance, if implementing a given enhancement made you...
- move a routine from one .c file to another;
- then modify that routine and a few callers;
... submitting your changes as two patches, the first one just doing the move, and the second one making the changes after the move, will greatly simplify the review of both patches, as the second patch will clearly show what changes you made.
2. Contribution Email Subject Line
A high-quality email subject line for a contribution contains the following tags:
For a Single patch contribution:
For Multi-part patch set contributions where [PATCH 0/2] is used to described the forth coming patches:
[PATCH 0/2] <Optional unique prefix e.g. "String cleanup:"><Subject related to the patch set e.g. Cleanup all broken string routines.>
[PATCH 1/2] <Optional unique prefix e.g. "String cleanup:"><Distinct subject line for this patch e.g. Cleanup strstr.>
[PATCH 2/2] <Optional unique prefix e.g. "String cleanup:"><Another distinct subject line for this patch e.g. Cleanup strncmp.>
For a request for comments that doesn't have to be a patch at all:
For a request for comments on a patch or patch set:
For a patch that fixes a bug filed in bugzilla:
[PATCH][PR <component/number>] <Subject>
For a patch that is the Nth version of an earlier patch:
[PATCH vN] <Subject>
For a patch that is already committed:
[COMMITTED PATCH] <Subject>
For a patch that falls under the The Obvious Rule as described in gdb/MAINTAINERS:
[OB PATCH] <Subject>
Most reviewers get a lot of email. To help them, please make <Subject> as descriptive as possible, with the most important words early on:
Fix PR 14963 is a bad subject: everybody on the list has to open this email to see what it relates to.
Fix segfault is a little better: we can see this patch fixes an important issue, but in which component?
Fix segfault caused by infinite loop in demangler is better, but the component is at the end and may be truncated in people's email clients.
Fix segfault demangling C++11 rvalue references is pretty great.
3. Detailed Explanation of the Patch
3.1. General requirements
- Describe the rationale for the patch. E.g., don't assume the change will be obvious to others just because it's a small patch. It may be that it only looks obvious to you, because you've spent the effort investigating the issue, and you have all the context in your head. Your job is to make it easy for the reviewer to give you an OK.
- If you experimented other seemingly valid or seemingly better approaches for your fix/feature, and they didn't work out, mention it in the patch submission, along with the reason for why it didn't work out.
- Explain the steps required to reproduce the problem.
- Extra points are awarded for a test-case which demonstrates the problem and can be used in the test-suite to demonstrate success of a fix.
- Explain the observed versus expected behaviour. If you're changing the output of some command, include a paste of the relevant parts of gdb session, before and after the change.
- Explain how you tested the patch. If you ran the testsuite, say so, and say on which targets. If you didn't run the testsuite, you should have. If you still haven't run the testsuite, say so, and explain why.
All patches for consideration must have ChangeLog entry updates (see below) or they will be rejected during review.
Run find . -name ChangeLog to help identify the correct ChangeLog file. E.g.,
./ChangeLog ./stubs/ChangeLog ./gdbserver/ChangeLog ./testsuite/ChangeLog ./doc/ChangeLog
- This should not be part of the change diff; paste it in clear text in the body of the email. If you are using format-patch, make it a part of the commit message as described in the section below on properly formatting commit messages.
When posting a new version of a patch, either as a new thread, or in reply to an existing thread, always repost the ChangeLog entry as well, even if it hasn't changed.
If the code in question is a Request For Comment (RFC) please indicate as such in the email subject. Explicit RFC emails aren't constrained by the ChangeLog rules but they won't be considered for inclusion until they comply.
If your patch adds a new command then that command should be documented in the GDB manual, and mentioned in the NEWS file. All commands must be documented, including maintenance commands (e.g., "set debug" or "maint" commands).
New target ports must be mentioned in the NEWS file.
New host configurations must be mentioned in the NEWS file.
New remote protocol packets must be documented in the GDB manual, and mentioned in the NEWS file.
Standard target features for xml target descriptions (e.g., org.gnu.gdb.arm.core, org.gnu.gdb.i386.sse) must be documented in the GDB manual.
New MI commands, attributes, etc., must be documented in the GDB manual, and mentioned in the NEWS file.
New Python scripting features must be documented in the GDB manual, and mentioned in the NEWS file.
Make sure there are no regressions in the test suite.
- The easiest way to verify this is to run the testsuite before and after your patch, and comparing the testsuite reports obtained before and after.
- Explain for which targets the test suite was run.
- Adding a test-case to the test suite may increase the likelihood of acceptance of your patch. In many cases it will be necessary.
- Don't add new tests that are known to FAIL. Mark them as KFAIL or XFAIL.
6. FSF copyright Assignment
The Free Software Foundation (holder of the GDB copyrights) requires copyright assignment for all legally significant changes by a particular author (read not a company). This webpage describes the copyright assignment requirements.
- Explain your current FSF copyright assignment status.
In the event that you have not previously completed an FSF copyright assignment, a GDB maintainer will direct you to a FSF copyright assignment request form that's best suited to your contribution under the guidelines on this gnu.org webpage.
As from August, 2013, the FSF began accepting GPG signed assignments from U.S. residents. More information at this link.
7. Copyright Header
All source files must have a copyright header. Although the FSF only requires that for legally significant files (IOW, files with only a couple lines wouldn't require a header), GDB maintainers prefer not having to worry about later having to add the header when the file grows.
In addition, we track which files don't have copyright headers, and it improves the S/N ratio of the output of updating copyright years for each new year if every file has a copyright header. Thus a copyright header is required even for simple 3 line testcases. This GNU Copyright-Notices webpage indicates the proper format for the copyright notice and how to write the dates:
- A copyright notice takes the following form:
Copyright (C) year copyright-holder
For GDB, spanning of consecutive years IS allowed. Therefore, when adding a second (or more) consecutive year to the copyright statement, you may write a year range, e.g.,
Copyright (C) year1-year2 copyright-holder
The simplest is to just copy the header from some other file.
If you've based your new file on some other file, as e.g., it is common to do when writing new tests, then the copyright years of the original file must be retained in the new file.
"Contributed by" statements are no longer required or desired in GDB source files. Attribution is now recorded in git commit logs as the commit "Author". Please do not use "Contributed by" statements in your submissions.
Please do feel free to propose additions to the Contributors node in the manual, however.
9. Properly Formatted GNU ChangeLog
The patch must include the necessary ChangeLog entries. Here are the things to watch for. In doubt, look at past entries and try to follow the style.
If your patch fixes a bug in GDB's bug database, a brief reference to the bug should be included in the ChangeLog entry (see format and example below). This not only documents your change in the ChangeLog file, but also ensures that the commits gets logged in the bug database as well.
- Files that were regenerated should be identified and the change should say "Regenerate."
The GCC folks made a tool called mklog.py to help generate ChangeLogs, which is also checked in the binutils-gdb repo. This script creates a ChangeLog skeleton from a diff. Run "git show | contrib/mklog.py" to use it.
Every change line in a ChangeLog should start with a capitalized word and end with a period.
The ChangeLog should indicate what was changed in the file. For instance, if you change the size of a variable you need to indicate as much in the ChangeLog.
- For added and removed files, you can just list the file and say "Added." or "Removed.", no need to list all entities in the file.
- When an entity (e.g. a function) is changed and other spots are trivially adjusted to follow that change, it is ok to just mention the changed entity and say something like "All callers updated.".
ChangeLog standard tab-width is 8.
ChangeLog standard max-line-length is 74.
An example for a trivial change follows: Note: words in arrow brackets shouldn't appear in the ChangeLog, they indicate whitespace.
YYYY-MM-DD<2 spaces>John Doe<2 spaces><email@example.com> <blank-line> <tab--->PR <component>/<number><use this if there is a bugzilla PR associated with the patch> <tab--->* breakpoint.c (handle_some_event): Remove reference to foo. Call<line wrap at or before column 74> <tab--->bar. <tab--->* configure.ac (build_warnings): Do not use -Wformat-nonliteral <tab--->with -Wno-format. <tab--->* configure: Regenerate.
2013-12-12 John Doe <firstname.lastname@example.org> PR gdb/9999 * breakpoint.c (handle_some_event): Remove reference to foo. Call bar. * configure.ac (build_warnings): Do not use -Wformat-nonliteral with -Wno-format. * configure: Regenerate. * NEWS: Mention awesome feature.
In your patch email you should also specify which ChangeLog is being modified. Before the line containing the date and your name/email, add a line with the path to the ChangeLog. If there are multiple components being updated with multiple ChangeLog edits, split these into sections, one for each ChangeLog:
gdb/ChangeLog: 2013-12-12 John Doe <email@example.com> PR gdb/9999 * breakpoint.c (handle_some_event): Remove reference to foo. Call bar. * configure.ac (build_warnings): Do not use -Wformat-nonliteral with -Wno-format. * configure: Regenerate. * NEWS: Mention awesome feature. gdb/testsuite/ChangeLog: 2013-12-12 John Doe <firstname.lastname@example.org> PR gdb/9999 * Makefile: Test changes for awesome feature.
10. Properly Formatted Changes
Please see Coding Standards for a full description of coding guidelines.
If you have any questions regarding these criteria please email email@example.com .
11. Submitting patches
Please send patches to firstname.lastname@example.org; one email per patch as described in the sections above. You can take a look at the list's archives to get a feel for what others do. If you're going to be contributing routinely, it's recommended to subscribe to the list.
Patches must be sent using git send-email command. This makes sure your patch doesn't get wrongly reformatted by your email client. There are numerous good references on how to configure and use git send-email, one of which is here.
If using git send-email is not possible for you, you can still try to send it using your email client, pasting the output of git format-patch in the email body. However, you need to be very careful that the client doesn't modify the patch in a way that makes it impossible to apply:
- Make sure your mailer doesn't turn TABS into SPACES, i.e. turn on preformat.
Don't send patches with format=flowed. This messes up line breaks and makes clients display tabs wrong.
Thunderbird users, see here.
See this excellent guide, that touches on the issues above, and more, including instructions for several mailers (Thunderbird, Alpine, KMail, Gmail, etc.).
Finally, as a last resort, you can send the output of git format-patch as an attachment. This is discouraged, however, because it makes it difficult for others to reply inline when reviewing. Please try to use git send-email as much as possible and everybody will be happy.
12. Patch status
And with s/glibc/GDB/g in mind, read Patch Review Workflow.
13. Ping and keep pinging
We try to do our best, but as a volunteer-based project, unfortunately lots of patches wait a bit before review. If your patch is not reviewed it thus may just mean that the reviewers are busy.
The generally accepted ping frequency is 2 weeks after first submission, and every week thereafter.
When sending a ping, reply to the original email and add "[PING]" to the reply's subject. In other words, don't start a new email thread for the ping. See rationale here.
14. Properly formatted commit messages
When the time comes to push your commits, please ensure the commit messages are formatted as follows:
<single line summarizing the change> <blank-line> <paragraph or paragraphs describing of the change> <blank-line> <all changelog entries relating to this change>
Remember to include the PR number (in the ChangeLog entries) if you commit is to fix a bug reported in the bugzilla.
Make all source files include defs.h or server.h first This commit makes all source files under gdb/ that include headers from gdb/ include either defs.h or server.h before any other code. This ensures that definitions and macros from the two config.h files are always in place for our code. An exception has been made for gdb/gdbserver/gdbreplay.c which seems to be a special case. gdb/ChangeLog: * btrace.c: Include defs.h. * common/ptid.c: Include defs.h or server.h as appropriate. * nat/mips-linux-watch.c: Likewise. gdb/gdbserver/ChangeLog: * hostio-errno.c: Move server.h to top of includes list. * inferiors.c: Likewise. * linux-x86-low.c: Likewise. * notif.c: Include server.h.
15. Fixing commit dates
If you have used git rebase to create your commits it is likely the commit dates are wrong. Before pushing please run this command:
git rebase --ignore-date origin/master
To set the timestamps of all the commits you are about to push to the current time.
The bug tracker is Bugzilla on sourceware.org. New users do not have the editbugs group access which means that while they can create bugs they cannot edit any aspects of the bug once created. This is done to avoid spammers modifying existing bugs. If you need the ability to edit bugs please request it on email@example.com or on IRC freenode at #gdb, the bugzilla admins and senior members of the community can grant this access once they know you're not a spammer.