Difference between revisions of "Preparing Code"

From Second Life Wiki
Jump to navigation Jump to search
Line 58: Line 58:
  diff -urN --exclude="*.o" my_untouched_tree my_modified_tree
  diff -urN --exclude="*.o" my_untouched_tree my_modified_tree
<br/>
<br/>
=== Unified diff of multiple files ===
=== Unified diff of multiple files ===
If for any reason you can not use a whole tree diff as above, you can still merge patches from different files into one, by appending the output of patch commands to the previous output (via '''>>''' redirection for the 2nd and later commands).
If for any reason you can not use a whole tree diff as above, you can still merge patches from different files into one, by appending the output of patch commands to the previous output (via '''>>''' redirection for the 2nd and later commands).
Line 77: Line 78:


<br/>
<br/>
=== Sample output ===
=== Sample output ===
For reference, here's what a unified diff looks like.
For reference, here's what a unified diff looks like.
Line 100: Line 102:
     LLCommon::cleanupClass();
     LLCommon::cleanupClass();
  }
  }


== Submitting the Patch on JIRA ==
== Submitting the Patch on JIRA ==

Revision as of 13:24, 15 September 2007

If you have a piece of source code (be it a fix, new feature, or optimization of existing code) submit the patches in a way that makes it easy for the Lindens to incorporate them into the source code.

Good Patch Practise

General

The best way to think about patches is that the people who are reviewing and applying your patches are the bottleneck in the system. If you can make it easier for us to understand and apply patches without lots of cleanup, we can spend more time reviewing and incorporating your work.

Before You Start

Check JIRA Bug Tracker to see if the issue has been submitted and its status.

If you're going to write a patch, don't forget to sign and send in the contribution agreement. The sooner you do that, the sooner we can accept your patch.

Coding Standard

Please read and follow the coding standard: Second Life coding standard

Comments

Please do not add issue numbers or your name to patches. We have to remove these by hand. Also, please be careful in how you add comments. Bracketing a patch with comments like "// PATCH START" and "// PATCH END" does not add any information, and adds to the manual cleanup we have to do.

If you want to make sure you get credited, then add yourself to doc/contributions.txt as part of the patch. That makes it easier for people to see the exact nature of the changes you've added.

In short, just add to the patch what is expected to appear in the final source.

One Patch for one Issue

Each patch should do exactly one thing. If you have three different bugs to fix, please submit three different patches. When you fold unrelated hunks into a single patch, we have to try to figure out which part of the patch applies to which bug, and split it out by hand, sometimes into several patches. Combo patches are particularly time consuming, and the manual splitting up makes it very easy to lose pieces of patches altogether.

Less is more

Smaller patches are easier to review and therefore more likely to be integrated quickly. The longer it takes for a patch to be incorporated, the higher the likelihood that the underlying code will change enough for the patch to fail. Please do not include formatting or other clean-up changes as that makes it harder to review and more likely to fail.

Path structure

It is not a must, but if your path structure permits, start the patch on the directory level where the linden folder resides, so that the path/file names in your patch will start with linden/.

Give Yourself Credit!

Please don't be shy about adding the issue label to doc/contributions.txt and adding your name. We use this file for a number of purposes, including the generation of the Source contributions page.

Creating a Patch

Patches should be submitted in unified diff format. This format is similar to simple diffs, but with more detailed information, and it can be automatically integrated into the source. You can generate a unified diff by calling diff -u <original file> <new file> (under Windows you will find the diff command as part of the CygWin project in C:\CYGWIN\BIN).

Please submit a single plain text uncompressed patch file that affects all of the files you need to modify. This is far easier to review than a collection of tiny one-file patches.

Also bad for reviewability are a compressed patch, or a tarball or zip containing one or more patches.

Unified diff of all changed files in a folder tree

The easiest way to generate a clean patch is to keep two copies of the source tree. Leave one completely unmodified, and make all of your changes in the second. Then use "diff -urN" to generate the patch. You can use the --exclude option to omit unwanted files. For example:

diff -urN --exclude="*.o" my_untouched_tree my_modified_tree


Unified diff of multiple files

If for any reason you can not use a whole tree diff as above, you can still merge patches from different files into one, by appending the output of patch commands to the previous output (via >> redirection for the 2nd and later commands).

diff -u linden-untouched/indra/newview/viewer.h linden/indra/newview/viewer.h >mychanges.patch.txt
diff -u linden-untouched/indra/newview/viewer.cpp linden/indra/newview/viewer.cpp >>mychanges.patch.txt
diff -u linden-untouched/indra/newview/llviewerwindow.cpp linden/indra/newview/llviewerwindow.cpp >>mychanges.patch.txt


CR/LF vs LF and Whitespace

In some situations, you will find that the patch contains the whole file, rather than just the lines changed. The reason is that sometimes, files in Linux format find their way into the source distribution. With these diff.exe will treat all lines as different from those in your modified file.

There are various options that will help here. You can add --strip-trailing-cr to the command line to fix this.

 diff -u --strip-trailing-cr linden-untouched/indra/newview/viewer.h linden/indra/newview/viewer.h >mychanges.patch.txt 

In some situations try the w (ignore whitespace) and/or B (ignore empty lines) options are also helpful (e.g. with tabs vs. space or spurious space characters while editing). Type diff --help for more details.


Sample output

For reference, here's what a unified diff looks like.

--- linden-untouched/indra/newview/viewer.cpp    2007-05-14 16:47:26.000000000 +0200
+++ linden/indra/newview/viewer.cpp 2007-05-22 08:49:50.484375000 +0200
@@ -6302,7 +6326,7 @@

    llinfos << "Cleaning Up" << llendflush;

-   LLKeyframeMotion::flushKeyframeCache();
+   LLKeyframeMotion::flushKeyframeCache(TRUE); 

    // Must clean up texture references before viewer window is destroyed.
    LLHUDObject::cleanupHUDObjects();
@@ -6562,6 +6586,8 @@
    delete gVFS;
    gVFS = NULL;

+   LLCurl::cleanup(); 
+
    // This will eventually be done in LLApp
    LLCommon::cleanupClass();
}

Submitting the Patch on JIRA

When you are ready for the world to view your patch, submit the patch on the JIRA Bug Tracker. Create a new issue or attach the patch to an existing entry as appropriate. Attach the .patch.txt file itself as a file attachment and make sure that the check mark for Patch attached is set (you can do that when creating the issue or by choosing Edit for an existing one). Take a look at existing patches through the Issues with patches attached filter on the JIRA main page to get a general idea how it is done.

With larger or sensitive patches it is a good idea to submit the patch for peer review on the mailing list. This way fellow developers can review the patch, uncover possible bugs, comment on undesired interactions or generally share ideas with you.

Just subscribe to the mailing list, then write an email to the list with a link to the description of what it does in the text.

Also, offer a description of the patch (the why and what) as a separate file and/or as a description or comment with the issue.

Good luck!


Applying patches to your source

In case you are interested, the application of other patches to your source is easy. Assuming that you are at the directory level from where the patch references the files (usually in the folder where you see the linden folder), applying a patch can be done via:

c:\cygwin\bin\patch -p 0 -i changes.patch

or

 c:\cygwin\bin\patch -p 0 < changes.patch

On your first attempts, you might also be interested in the --verbose, --backup or --dry-run (simulate patch) options. See patch --help for details.

c:\cygwin\bin\patch -p 0 --verbose --backup --dry-run -i changes.patch

Also, if a patch fails, look for files ending with .rej in the target folder.