Differences between revisions 45 and 46
Revision 45 as of 2016-11-19 17:33:03
Size: 18308
Editor: brobecke
Comment: typo
Revision 46 as of 2017-01-10 16:17:33
Size: 18804
Editor: PedroAlves
Comment: Add Attribution policy
Deletions are marked like this. Additions are marked like this.
Line 163: Line 163:

== Attribution ==
 * "{{{Contributed by}}}" statements are [[https://sourceware.org/ml/gdb-patches/2016-12/msg00320.html|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 [[https://sourceware.org/gdb/current/onlinedocs/gdb/Contributors.html|Contributors node in the manual]], however.

Contribution Checklist

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:

  • [PATCH] <Subject>

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:

  • [RFC] <Subject>

For a request for comments on a patch or patch set:

  • [RFC][PATCH] <Subject>

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.

3.2. ChangeLog

All patches for consideration must have ChangeLog entry updates (see below) of they will be rejected during mailing list moderation.

  • Run find . -name ChangeLog to help identify the correct ChangeLog file. E.g.,

    • ./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.

4. Documentation

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.

5. Testing

  • 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.
  • See here for a cookbook on writing GDB tests.

  • 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.

  • 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.

8. Attribution

9. Properly Formatted GNU ChangeLog

  • Your ChangeLog must be in the GNU ChangeLog format. The ChangeLog format is described here and here.

  • 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 have a tool called mklog to help generate ChangeLogs. This script creates a ChangeLog template for a diff file. Run "mklog diff-file" and the template will be added to the top of the file.

  • 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.

  • 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><johndoe@some.email.address>
<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--->* configure.ac (build_warnings): Do not use -Wformat-nonliteral
<tab--->with -Wno-format.
<tab--->* configure: Regenerate.

2013-12-12  John Doe  <johndoe@some.email.address>

        PR gdb/9999
        * breakpoint.c (handle_some_event): Remove reference to foo.  Call
        * configure.ac (build_warnings): Do not use -Wformat-nonliteral
        with -Wno-format.
        * configure: Regenerate.
        * NEWS: Mention awesome feature.

It's okay (and not uncommon) to include a one sentence explanation to the patch as a whole, in addition or in place of "PR comp/xyz". E.g.,

YYYY-MM-DD<2 spaces>John Doe<2 spaces><johndoe@some.email.address>
<tab--->Short explanation.
<tab--->* (install_breakpoint): Remove reference to bar.

2013-11-11  John Doe  <johndoe@some.email.address>

        Code cleanup.
        * breakpoint.c (install_breakpoint): Remove reference to bar.

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:

2013-12-12  John Doe  <johndoe@some.email.address>

        PR gdb/9999
        * breakpoint.c (handle_some_event): Remove reference to foo.  Call
        * configure.ac (build_warnings): Do not use -Wformat-nonliteral
        with -Wno-format.
        * configure: Regenerate.
        * NEWS: Mention awesome feature.

2013-12-12  John Doe  <johndoe@some.email.address>

        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 gdb-patches@sourceware.org .

11. More info

The file <gdb-src>/gdb/CONTRIBUTE contains more instructions on what is expected from patches submitted to GDB.

12. Submitting patches

  • Please send patches to gdb-patches@sourceware.org; one email per patch as described in the sections above.

  • We accept patches as plain text, (preferred for the compilers themselves), MIME attachments (preferred for the web pages), or as uuencoded gzipped text.
  • Although we accept patches created via "git diff" or "diff -up OLD NEW" which you can then send by email to the mailing list, we recommend instead that you either:

    • Send the commit(s) using the "git send-email" command; or

    • Use the "git format-patch" command to dump the commits in .patch files which you can then email as attachements.

    Sending the patches this way makes it trivial for everyone else to apply the patches (eg. for testing).
  • 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.).

13. Patch status

See GDB's Patchwork instance.

And with s/glibc/GDB/g in mind, read Patch Review Workflow.

14. 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.

15. 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>
<paragraph or paragraphs describing of the change>
<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.

For example:

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.


        * btrace.c: Include defs.h.
        * common/ptid.c: Include defs.h or server.h as appropriate.
        * nat/mips-linux-watch.c: Likewise.


        * hostio-errno.c: Move server.h to top of includes list.
        * inferiors.c: Likewise.
        * linux-x86-low.c: Likewise.
        * notif.c: Include server.h.

The discussions leading up to this format are in this thread and this thread.

16. 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.

None: ContributionChecklist (last edited 2017-01-10 16:17:33 by PedroAlves)

All content (C) 2008 Free Software Foundation. For terms of use, redistribution, and modification, please see the WikiLicense page.