WineHQ Patch submission guidelines
Use your real name
The Wine project will not accept patches from an anonymous person.
Try to imitate wine-patches style
Look at the wine-patches mailing list archives to get an idea of how most people submit patches, and also look at the wine-cvs mailing list archives to see which patches actually get committed. Then imitate the style of the successful patches.
Patch workflow
Before making any changes, run the ConformanceTests to make sure your system is ok. (See MakeTestFailures for a few notes on common problems.) Then run them again after you make your changes to verify you didn't break anything.
Test your changes (and if you changed any conformance tests, test them on Windows), then generate a patch and send it to the wine-patches mailing list.
Watch the wine-devel mailing list for feedback on your patch. (The messages should be cc'd to you, but sometimes they aren't.)
If anybody replies to your patch, address their feedback carefully and send your patch(es) again.
Watch the patch status page to see the status of your patch. The maintainer usually commits twenty or so patches all at once at the end of the day (European time).
If your patch had no negative feedback, the maintainer will have a look at it as soon as he can. Patches from people 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, so the maintainer might not be able to look at them immediately.
If your patch isn't committed in a day or two, improve it (perhaps by adding more tests) and resend, or ask on IRC/wine-devel for suggestions.
Generating patches
Please use Git to generate your patches.
It's best to send small patches against the current tree using Git. First, make sure your tree is up to date, as described on the Git page.
Check in your patches to your Git tree using git commit.
If you're going to use git imap-send make sure that you've set up the proper options in your Git config file, then run:
git format-patch --stdout --keep-subject --attach origin | git imap-send
That should upload your patches into your IMAP drafts folder, and allow you to check and send them. In Mozilla Thunderbird, that is as simple as clicking "Edit Draft..." then "Send" if the mail headers in the Git config file are set correctly.
If you can't use IMAP, use the following command to generate patches:
git format-patch --keep-subject origin
That should generate a number of .txt files, which you can send manually. (Please use the .txt extension as it makes the patches easier to review.)
For instance:
git fetch; git rebase origin # make sure you're up to date ./configure && make depend && make # rebuild the tree cd dlls/msvcrt # change to directory of interest rm -rf ~/.wine # be sure to test in a clean WINEPREFIX cd tests; make testclean; make test; cd .. # make sure all tests pass there before your changes vi file.c # change some stuff make # check that there's no errors or warnings rm -rf ~/.wine # be sure to test in a clean WINEPREFIX cd tests; make testclean; make test; cd .. # make sure all tests pass there after your changes cd ../.. git commit -a # check in your changes to your local repository git format-patch --keep-subject origin # generate patches
This will generate files 0001-yourcommitname.patch, 0002-yourcommitname.patch, etc. Look over the patches carefully, and adjust the subject line if needed (for instance, number the patches in a series as described below). Then send the patches to wine-patches@winehq.org, one per message.
Files to include
Include a single diff of all the files you changed. You don't need to include diffs for the following automatically generated files:
configure include/config.h dlls/Makefile.in programs/Makefile.in */Makefile
If you're using Git, check-in changes to these files in a separate commit.
Subject lines
There are a number of conventions regarding subject lines:
Start with the component your changing and a colon, like this:
msvcrt/tests: Test fopen of left-handed files.
If you're sending a series of patches which depend on each other, number them like this:
[1/2] msvcrt/tests: Test fopen of left-handed files. [2/2] msvcrt: Fix fopen of left-handed files.
If you're resending a patch unchanged, add "(resend)" to the end of the subject line.
If you're resending a patch with some changes, add "(try 2)" to the end of the subject line, and describe what the differences are with the previous version of the patch.
Include a description
Your email should describe what change you made and why you made the change. It should mention what operating systems you ran the tests on. If this is an updated version of the patch, also say how it changed since the last version. If you're addressing somebody else's feedback, say so.
If you're working on a bug in bugzilla, mention the bug number, and consider filing a bug if none exists.
Small, clear and atomic changes
Keep the patch small and the point of the change clear. Small patches can be reviewed and applied more quickly than large changes, and they are. "Atomic" changes are independent of each other. One of your patches or the other might be applied, or dropped, but Wine should keep working, and not contain superfluous code despite this.
Small and atomic patches also make regression analysis easier. To do a regression analysis, we need to know at all points in the CVS/GIT tree that Wine compiled and functioned.
This also means you should submit one fix or group of related changes per patch.
Use the latest Wine CVS or GIT tree
Generate your patch against the latest version of Wine from the WineHQ CVS or GIT tree. Make sure your source is updated before sending the patch, as patches that don't apply cleanly will probably be silently ignored.
Test your patch!
Did you compile it after that last change you made? You'd be surprised what can happen in last minute edits ...
Does it really do what you think it does? A good way to show that is to write a test case demonstrating it is correct.
Test cases show up many problems with code, and it will save you embarrassing "brown paper bag" experiences. It also means that the bug you solved will not creep back in because somebody else didn't understand it.
If you fixed a bug or added a feature, but didn't add a test to cover that case, please consider doing so.
If you added tests, did you make sure they pass on Windows?
Know your mail client
Be careful to not corrupt the patch through line-wrapping. Some mailers let you enclose the patch in the body of the mail, and won't mess up the formatting. Mozilla isn't one of those! If you use Mozilla Thunderbird or the Mozilla suite, make sure to attach your patches so it doesn't wrap the lines.
Avoid reformatting
My C coding style is the best! But it's not the same as yours. So, how about we just agree to disagree?
When you edit a piece of code, preserve the original formatting. The only thing worse than the wrong coding style is 10 mixed coding styles, so let's just stick to one bad one per file.
It's good to read through your diff, and check that there's no unnecessary diff chunks in it. Removing useless chunks will reduce the size of your diff, which is also good (see above).
In general, unless the file you're working on using tab formatting, avoid tabs in Wine code. If you have to use tabs, a tab is defined as 8 spaces. If you use ts=4 in your editor to make it look better, remember to go back and make sure things look sane with ts=8 before submitting your patch. Tabs generally suck, and you shouldn't use them.
Avoid C++, C99 and non-portable C constructs
Wine adheres to standard C89/C90. Code should be written so that it works with as many compilers as possible, so you should avoid compiler-specific constructs, C99 and C++.
int foo() /* bad: use 'int foo(void)' instead */
{
bar();
int ch; /* bad: inline variable declaration */
ch = baz();
// does something /* bad: C99/C++ style comment */
for (int i = 0; i < 4; i++) /* bad: inline variable declaration */
putc('a' + i);
}
void bar(void) __attribute__((noreturn)) /* bad: not portable; use */
{ /* DECLSPEC_NORETURN instead */
long x; /* bad: long is different in Win64 and Unix64; use */
/* int or LONG instead */
int y = (int) &x; /* bad: in 64-bit mode, the pointer will be truncated; */
/* use INT_PTR or LONG_PTR instead */
ExitProcess(0);
}
int baz(void)
{
wchar_t str[6]; /* bad: may be 32-bit in Unix; use WCHAR instead */
const WCHAR str1[] = L"Hello"; /* bad: L"" uses wchar_t; use syntax below */
const WCHAR str2[] = {
'H','e','l','l','o',0
};
const TCHAR str3[] = { 'A', 0 }; /* bad: use WCHAR explicitly */
lstrcpy(str, str2); /* bad: use -W form explicitly */
return L'\0'; /* bad: L'' uses wchar_t; use '\0' or 0 */
}
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.
If you're simply resending a patch somebody else wrote, put a 'From:' line with the original author's name and email address in the body of the message, in the same format as it would have been on the original mail.
The best way is to commit the author's original patch to your tree with the correct authorship information, and then send the complete git-format-patch output with all the headers in the body of your mail. If the Wine maintainer's commit script detects an embedded git commit in the mail body it will ignore the actual SMTP headers in favor of the git info.
