Difference between revisions of "Submitting Patches"
(→Code guidelines: fix broken links)
(→Can I submit patches?: fix wiki links)
|Line 5:||Line 5:|
=== Can I submit patches? ===
=== Can I submit patches? ===
Not everyone can contribute code to Wine, primarily because doing so would violate the
Not everyone can contribute code to Wine, primarily because doing so would violate the . If you have studied the Microsoft Windows source code, even if you aren't under a [https://en.wikipedia.org/wiki/Non-disclosure_agreement non-disclosure agreement], your patches probably won't be accepted. This is to ensure that the Wine project does not violate any copyrights belonging to Microsoft. Some more notes related to this can be found in the .
=== Check your Git setup ===
=== Check your Git setup ===
Revision as of 14:11, 2 November 2016
Translations of this page: not yet ported. Translators, please see the Discussion page.
- 1 WineHQ patch submission guidelines
- 2 Making your changes
- 3 Generating and submitting patches
- 4 Special situations
WineHQ patch submission guidelines
In order to review code better and use established lines of communication, the Wine project accepts code in the form of patch files (instead of pushing directly to the main Git repository). The process may seem a little complicated at first, but it will probably become very natural after just one or two submissions.
Can I submit patches?
Not everyone can contribute code to Wine, primarily because doing so would violate the Clean Room Guidelines. If you have studied the Microsoft Windows source code, even if you aren't under a non-disclosure agreement, your patches probably won't be accepted. This is to ensure that the Wine project does not violate any copyrights belonging to Microsoft. Some more notes related to this can be found in the Developer FAQ.
Check your Git setup
It is important to make sure the author and email settings for your Git repo are configured correctly.
git config --global user.name "Your Name" git config --global user.email "firstname.lastname@example.org"
The Git Wine Tutorial has more details, but the key point is to use your real name. This isn't just for developers to show a stronger commitment and establish trust, but also helps discourage irresponsible programmers from submitting unacceptable code (such as dissassembled Microsoft components).
Finally, there's always a chance that someone has recently changed the same code you are working on. To make sure your code hasn't been superseded, be sure to update your Git repo before generating your patch files.
git pull --rebase
If you prefer to manage your work in separate Git branches, remember that the above command will only update your current branch and all of your branches need to be up-to-date. Keeping your "master" branch in sync with WineHQ's repository and creating branches (and rebasing them on "master") for your own changes might be a good idea. Again, the Git Wine Tutorial page goes into much more detail on using Git to manage your work.
Making your changes
If you are making changes to the Wine program itself, there are some basic rules your code should follow to pass review. In order to make your changes as easy to check as possible, try to keep them small, clear, and atomic:
- Follow the style of the surrounding code as much as possible, except when it is completely impractical to do so
- Use a consistent style in your own changes (e.g. don't write `foo* bar` in one place and `foo *bar` in another)
- Convert tabs to spaces and remove trailing whitespace from your changes, but don't change whitespace in other lines
- Avoid very long lines (100 chars is a safe max width, but it's not a hard limit)
- Only include closely related changes in one patch
- If you've written a fix and new ConformanceTests for an issue, include them in the same patch
- Try to limit a patch to changing a single file or component unless that would break something somewhere else
These rules also help make testing regressions easier, and prevent new bugs from slipping through.
There are other coding rules that affect the quality of Wine itself more than your patch files. To read more about these guidelines, see the Coding Practice chapter of the Wine Developer's Guide and the coding conventions section of Developer Hints.
You can also look at the patch status page to see the patches that have been submitted and committed recently. Try to imitate the style of the successful patches.
Related parts of the Wine project, such as the website, may have slightly looser requirements on patches, but it is still important to keep patches clear and simple, bundle related changes together, and not break anything.
Testing for problems
Be sure to run the conformance tests on an unchanged branch of Wine (or before you make any changes) to see where your system is OK. Then run the tests again after you make your changes to verify that they don't break anything new. If you've added or modified any of the conformance tests, be sure to check that they work properly by using the make crosstest command, then submitting the resulting executable to Wine TestBot.
Also, make note of any significant compiler warnings that come up when building your modified version of Wine and fix the underlying cause before submitting your patch.
The commit message
In the first line of the commit message, write name of the component you changed, followed by a colon, a brief description that you would like to appear in the Git log, and a period. Be brief; the first line should not be more than 72 characters in length.
After the first line, add additional paragraphs explaining what the patch accomplishes. Mention what operating systems you ran the tests on. If it is an revised version of a patch that you submitted previously, describe the changes you made since the last version, and address any feedback from other developers. Also, if your patch fixes a bug, be sure to mention the bug number, but do so here in the explanation, not the first line. Wrap all lines to 72 characters.
Finally, your patch should include a Signed-off-by line. This line indicates that you accept responsibility for fixing any regressions caused by your patch. git commit -s will add this line automatically for you. You can also use git config format.signOff true to enable this setting by default on every new commit.
A complete example:
msvcrt: Implement strfry and add tests. Fixes https://bugs.winehq.org/show_bug.cgi?id=10000 Changes in version 2 of this patch: The function now uses a proper randomization algorithm and all possible combinations are now equally likely. Tested on Windows 7 and Arch Linux. Signed-off-by: John Doe <email@example.com>
Generating and submitting patches
Creating patch files
The Git Wine Tutorial includes more details about the different ways to create patch files. In general you want:
git format-patch origin
You can also add --subject-prefix="" to avoid getting the word PATCH in all your email subjects (i.e. `[PATCH x/y]`).
Alternatively, you can use git config format.subjectprefix "" to do this automatically for all future patches.
A coding system is used in the subject line in order to sort submissions:
- If you are sending multiple patches that depend on each other, the subject line should begin with a fraction enclosed in square brackets, giving the patch's order and the total number of patches in the set (see also -n option of git format-patch):
[1/3] This is the first patch [2/3] This patch depends on the first one [3/3] This patch depends on the others
- Next, if you are submitting a patch for something other than Wine itself (such as the website or documentation), use `--subject-prefix=<project>" to include the project code in square brackets:
[website] Added ... translation [bugzilla] Fix ... [docs] Updated ... [appdb] Add Feature ... [tools] Fix ... [fontforge] Fix ...
- Finally, if your submission somehow fell through the cracks and you're trying to send a patch again without changes, use `--subject-prefix=resend`. If the patch had errors and you've changed it before resubmitting, include which try you're on as `--subject-prefix=v2`, `--subject-prefix=v3`, etc.:
[resend] user32: This patch just got lost somehow [website resend] This patch for the website got lost somehow [v2] ntdll: This patch was wrong the first time [v3] comctl32: This patch has failed twice
It never hurts to check through your patches one last time:
- Look at each line of the patch, make sure there are no unintentional or unrelated changes
- Remember not to actually edit the patch file itself. If you find errors, change the code and generate a new patch file.
- Did your fix pass all related tests? If it wasn't covered by a test, have you written a test of your own?
Sending the patch
The Git Wine Tutorial includes more details about the different ways to send patch files. In general you want:
git send-email *.patch
There are a few things to look out for when mailing in your patch. First, be sure you address the patch to the wine-patches mailing list, firstname.lastname@example.org, and remember that you need to be subscribed to the mailing list in order for your submission to get past the spam filter.
The git send-email command is strongly recommended, but if you can't get it to work, you may send the patch file as an attachment. Remember to only attach one patch per email and use the subject line from the patch file as the subject line in the email. Also, always send your emails as plain text, not HTML or rich text.
Managing your submission
After you've submitted you're patch, you should watch the wine-devel mailing list for feedback on your patch (although replies should normally be cc'd to you, sometimes people may forget). If your patch does get some feedback or a critique, carefully think about what they say and fix your patch before resubmitting. If others point out a few flaws in your patch, it's safe to assume that they missed some too so try to check the whole patch and fix other flaws before resending.
Also be sure to watch the patch status page to see how far it has gone. The maintainer usually commits twenty or so patches all at once at the end of the day (Japan Standard Time). If your patch had no negative feedback, the maintainer will have a look at it as soon as he can. Patches from those who have successfully committed patches before, and who are known to test their work thoroughly, are likely to be committed quickly. Patches from new developers require more scrutiny and the maintainer will probably not get to them immediately, but this does not mean your patch has been forgotten.
If your patch disappears from https://source.winehq.org/patches/ without being committed and without feedback, improving it (perhaps by adding more tests) and resending it may help, or you can ask on the ["IRC"] channel or developers' mailing list for suggestions. If you have patches rejected multiple times, don't be discouraged. You might consider finding a mentor willing to pre-review your patches before you send them to wine-patches (see https://www.winehq.org/pipermail/wine-devel/2010-August/086207.html for a discussion of this)
Submitting patches written by somebody else
Occasionally, the author of a useful patch stops working on it, and somebody else has to pick it up and continue. In this case, commit the author's original patch file to your local tree (`git am`) and add two additional lines to the commit message (git commit --amend):
- A From: line with the original author's name and email address. This is necessary because git send-email will replace the original author's name and email with your own name and email.
- A Signed-off-by: line to indicate that you (not the original author) accept responsibility for fixing any regressions caused by the patch.
If someone else has made a significant contribution to the patch, first try to split up the patch so that each patch has only one author. If that is impractical, explain in the commit message who you collaborated with on the patch. You may share responsibility for the patch by adding multiple Signed-off-by lines, but get permission before adding a Signed-off-by line for anyone but yourself.
Other contributors also need to indicate acceptance of responsibility for a patch by sending a Signed-off-by line to the wine-patches mailing list. For example:
From: Jane Doe <email@example.com> To: firstname.lastname@example.org Subject: Re: msvcrt: Implement strfry and add tests. Signed-off-by: Jane Doe <email@example.com>
Multiple outstanding patchsets
Alexandre says you should concentrate on one thing and get it in before trying others. He may ignore some of your patchsets if he feels you have too many balls in the air.