Difference between revisions of "Development:Coding"

From Camino Wiki
Jump to navigation Jump to search
(update license block info for MPL2)
 
(14 intermediate revisions by the same user not shown)
Line 6: Line 6:
  
 
*[http://www.mozilla.org/hacking/mozilla-style-guide.html Mozilla Coding Style Guide]
 
*[http://www.mozilla.org/hacking/mozilla-style-guide.html Mozilla Coding Style Guide]
*[http://www.mozilla.org/MPL/boilerplate-1.1/ License block for new files]
+
*[http://www.mozilla.org/MPL/headers/ License block for new files] (use the <code>/* */</code> style)
  
 
''Also link to [http://www.mozilla.org/hacking/ Hacking Mozilla] general intro somewhere in our contributor introduction''
 
''Also link to [http://www.mozilla.org/hacking/ Hacking Mozilla] general intro somewhere in our contributor introduction''
Line 13: Line 13:
 
   
 
   
 
<pre>
 
<pre>
if (foo) {
+
- (bip)updateBat
  bar;
+
{
  blam;
+
  if (foo) {
}
+
    bar;
else {
+
    blam;
   baz;
+
  }
   zap;
+
  else {
 +
    baz;
 +
    zap;
 +
   }
 +
 
 +
  if (murkyWaterFlowsUnderTheBridgeNearTheTallOakTree ||
 +
      dirtyWaterRunsPastTheOldMillNearTheOpera)
 +
  {
 +
    return nil;
 +
   }
 +
  else {
 +
    bop;
 +
  }
 
}
 
}
 
</pre>
 
</pre>
 +
 +
When the condition is multiple lines, braces are required on the statement (with the opening brace on a line by itself).  If the <code>if</code> statement had braces, the <code>else</code> statement must also have braces.
  
 
Braces for single statements are optional (and discouraged, but some still include them).
 
Braces for single statements are optional (and discouraged, but some still include them).
  
 
We explicitly have no style rule regarding the placement of <tt>*</tt>.  Aim to be consistent within a file in as much as the files are consistent.
 
We explicitly have no style rule regarding the placement of <tt>*</tt>.  Aim to be consistent within a file in as much as the files are consistent.
 +
 +
As alluded to above, many of Camino’s older files have a mish-mash of styles.  If you find particularly egregious cases, file bugs.  You should not include unrelated whitespace or style cleanup in a patch; instead, you should perform cleanup in separate patches (or bugs) either before or after code changes.  This keeps (blame for) code changes clear: if you are researching a section of code and hit a “mass style change” you can simply drop back to the previous revision and continue tracking the blame from there.
  
 
=== Coding for latest-OS-version features or using private APIs ===
 
=== Coding for latest-OS-version features or using private APIs ===
Line 35: Line 51:
 
The spell-checker's <code>learnWord:</code> method is undocumented on Mac OS X versions older than 10.5, but it is the only way to achieve proper "learned spelling" integration with the Mac OS X spelling system. The code supporting Camino’s use of this method is [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/embedding/CHBrowserView%2BSpelling.mm&rev=1.1&mark=62-67#62 here] and [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/browser/BrowserWindowController.mm&rev=1.388&mark=4517-4525#4517 here].
 
The spell-checker's <code>learnWord:</code> method is undocumented on Mac OS X versions older than 10.5, but it is the only way to achieve proper "learned spelling" integration with the Mac OS X spelling system. The code supporting Camino’s use of this method is [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/embedding/CHBrowserView%2BSpelling.mm&rev=1.1&mark=62-67#62 here] and [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/browser/BrowserWindowController.mm&rev=1.388&mark=4517-4525#4517 here].
  
On Mac OS X 10.5, Camino also support building a [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/browser/BrowserWindowController.mm&rev=1.388&mark=4447-4490#4447 Language sub-menu] for switching spell-check languages.
+
On Mac OS X 10.5, Camino also supports building a [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/browser/BrowserWindowController.mm&rev=1.388&mark=4447-4490#4447 Language sub-menu] for switching spell-check languages.
  
 
Setting Spotlight metadata on files is still undocumented as of the 10.5 SDK. Our download listener code has [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/download/nsDownloadListener.mm&rev=1.39&mark=652-702#652 a very well-documented and commented example] of how to use a method like this safely while running on Mac OS X 10.4 and 10.5.   
 
Setting Spotlight metadata on files is still undocumented as of the 10.5 SDK. Our download listener code has [http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/download/nsDownloadListener.mm&rev=1.39&mark=652-702#652 a very well-documented and commented example] of how to use a method like this safely while running on Mac OS X 10.4 and 10.5.   
Line 52: Line 68:
 
====New Files====
 
====New Files====
  
If you add a new file, be sure to give it the appropriate tri-license license block from [http://www.mozilla.org/MPL/boilerplate-1.1/ http://www.mozilla.org/MPL/boilerplate-1.1/]; do not copy boilerplate from other files. 
+
If you add a new file, be sure to give it the appropriate MPL2 license block from [http://www.mozilla.org/MPL/headers/ http://www.mozilla.org/MPL/headers/] (in C/C++/Obj-C/Obj-C++ files, this is the <code>/* */</code> style); do not copy boilerplate from other files.   
 
 
For files added in <code>mozilla/camino</code>, the ”The original code” should be “Camino code.”  There are a few general exceptions to this:
 
# Makefiles are often “just a lowly Makefile”
 
# If you are creating a discrete “sub-module,” e.g. [http://mxr.mozilla.org/seamonkey/source/camino/feedhandlers/feedhandlers.applescript.in#14 the feed handlers], you can provide a more descriptive name for the module of code
 
# If you are contributing code you have written yourself for another project, the original code should be the name of the project from which the code was borrowed, e.g. if [[User:Wevah|Wevah]] were to contribute punycode-handling code from Paparazzi! and this code was implemented in a new file, the original code would be “Paparazzi!”
 
 
 
Unless you are working on Camino in a work-for-hire situation (or similar), the “Initial Developer” should be your name (optionally followed by your email address)In a work-for-hire situation, the original author is typically your employer; check with your employer for the specifics of your situation.
 
  
The copyright year should be the year you began working on the code in the file, even if it's not committed until the following year (e.g., you began developing the patch in December 2009 but the patch was not committed until January 2010).
+
=====New Third-Party Code=====
 +
If you are adding a new file from another source (including code you yourself have written but not written specifically for Camino)—for instance, when we include [[User:Wevah|Wevah]]’s [https://github.com/Wevah/Punycode-Cocoa punycode methods]—you should preserve the existing license blocks on the files.  (You should, of course, first check to make sure that the license of the external code is [https://www.mozilla.org/MPL/license-policy.html#Licenses_Compatible_with_the_MPL compatible with the MPL v2].)
  
Finally, list yourself (along with your email address) under “Contributor(s):” and add “(Original Author)” following your name. Note that in work-for-hire situations, even if your employer is listed as the Initial Developer, you should list yourself here.
+
If you are adding an entire bundle of third-party code (e.g., [http://mxr.mozilla.org/camino/source/camino/sparkle/ Sparkle] or [http://mxr.mozilla.org/camino/source/camino/google-breakpad/ Breakpad]), Camino style is to include a [http://mxr.mozilla.org/camino/source/camino/sparkle/README README] file in the code's top-level directory containing the source repository and licensing information for the third-party code, along with a list of any changes made to that code in order to use it in Camino (this list should be updated any time the third-party code is touched or updated).  If the third-party code already includes a [http://mxr.mozilla.org/camino/source/camino/google-breakpad/README README] file, add a [http://mxr.mozilla.org/camino/source/camino/google-breakpad/README_CAMINO README_CAMINO] file instead.
  
=====Moving Existing Code to a New File=====
+
When third-party code (either a few files or an entire bundle of code) is committed, the commit comment must include information or references to the author of the code and the source repository, if available.  Make sure your committer is aware of this information.
  
If, as part of your patch, you refactor code and move a collection of related code into a new file, you should make a good-faith effort to consult cvs blame in Bonsai to determine the Initial Developer of the code being moved. 
+
==Testing your patch==
  
If the moved code is all (or mostly) from one developer and you have not made substantial changes to the code when moving it, or have not made substantial additions (i.e., such that your code in the new file now comprises more of the file than the moved code), list the principal author of the moved code (or the author’s employer, if the author was in a work-for-hire situation) as the Initial Developer and as the first contributor (along with the “(Original Author)” tag).  Add other developers of the moved code as contributors and then add yourselfIf you have made substantial changes or additions to the code being moved, you should list yourself as the Initial Developer and Original Author in the Contributors section, and then include authors of the moved code as other Contributors.
+
Once you’ve written code and gotten it to compile, you need to test your patch to make sure it works (and works as expected).  There are several methods available that allow you to launch another Camino at the same time as your daily-use Camino and/or protect your daily-use profile from development buildsSome methods may be more useful in certain situations than others.
  
====Contributors to Existing Files====
+
See [[Development:Using Custom Profiles]] for detailed explanations of the various methods.
If you have made substantial additions or changes to a given file in a patch, or if, over time, you have done so, be sure to add yourself (along with your email address) to the “Contributor(s):” section of the license block.
 
  
 
==Setting breakpoints in external files==
 
==Setting breakpoints in external files==
  
Sometimes when debugging you will want to set breakpoints in files that are part of Camino, but not in the XCode project. For example, <tt>widget/src/cocoa/nsNativeThemeCocoa.mm</tt>. If you get an error about gdb not being able to find the file:linenumber, you will need to add the relevant shared library. Head to Run->Show->Shared Libraries... and set the Starting Level for the library to All. Which library to set it for depends on the file; for nsNativeThemeCocoa.mm, it is libwidget_mac.dylib. Turning off "Lazy Symbol Loading" in Debugger preferences may also help.
+
Sometimes when debugging you will want to set breakpoints in files that are part of Camino, but not in the XCode project. For example, <tt>widget/src/cocoa/nsNativeThemeCocoa.mm</tt>. If you get an error about gdb not being able to find the file:linenumber, you will need to add the relevant shared library. Head to Run->Show->Shared Libraries... and set the Starting Level for the library to All. Which library to set it for depends on the file; for nsNativeThemeCocoa.mm, it is libwidget_mac.dylib. Turning off “Lazy Symbol Loading” in Debugger preferences may also help.
  
 
==Preparing a patch to submit for review==
 
==Preparing a patch to submit for review==
 
'''XXXFIXME''' for hg commands
 
  
 
Before creating a patch for review, be sure that your source tree is up-to-date and that Camino builds with your changes.
 
Before creating a patch for review, be sure that your source tree is up-to-date and that Camino builds with your changes.
  
In the <tt>$SRCDIR</tt>, <tt>cd mozilla/camino</tt> and then <tt>cvs up -dP</tt>.  By default, cvs will attempt to merge any changes from the repository into your working copy.  If it fails to merge the repository changes with your changes, cvs will inform you with a <tt>C</tt> mark before the file and a message about conflicts.  You will need to resolve the conflict manually.
+
In the <code>$SRCDIR</code>, <code>cd mozilla/camino</code> and then <code>hg pull -u</code>.  By default, Mercurial will attempt to merge any changes from the repository into your working copy.  If it fails to merge the repository changes with your changes, hg will inform you with a message about conflicts (and, depending on your configuration, dump you into your [[Development:Building:Mozilla_1.9.2_Branch#Setting_up_your_Hg_Environment|merge tool]]).  You will need to resolve the conflict manually and then continue as instructed by Mercurial’s message.
  
After updating your tree, switch back to <tt>$OBJDIR/camino</tt> and <tt>make</tt> (or build in Xcode) to make sure your changes and the latest code build.  Ideally you would also launch Camino and test your changes one last time.
+
After updating your tree, switch back to <code>$OBJDIR/camino</code> and <code>make</code> (or build in Xcode) to make sure your changes and the latest code build.  Ideally you would also launch Camino and test your changes one last time.
  
 
===Proper patch format===
 
===Proper patch format===
  
Use <tt>cvs diff -u8N</tt> for patches to Camino code. Diffs should be done from <tt>$SRCDIR/mozilla/camino</tt> or <tt>$SRCDIR/mozilla</tt> for consistency and ease of application by reviewers and committers.  All changes—including new source code files and project file changes, if applicable—should be submitted as a single patch (''e.g.'', <tt>cvs diff -u8N src/foo/bar src/baz/bam > bugnumber.patch)</tt>, with the exception of changes to nibs and images; see below for more about requirements for those three file types.
+
You should have configured the proper patch format when [[Development:Building:Mozilla_1.9.2_Branch#Setting_up_your_Hg_Environment|setting up your Hg environment]] as part of the build instructions. Diffs should be done from <code>$SRCDIR/mozilla/camino</code> (or <code>$SRCDIR/mozilla</code> if you’re hacking on Gecko).  All changes—including new source code files and project file changes, if applicable—should be submitted as a single patch (''e.g.'', <code>hg diff src/foo/bar src/baz/bam > bugnumber.patch</code>), with the exception of changes to nibs and images; see below for more about requirements for those file types.
  
 
====Adding new files in a patch====
 
====Adding new files in a patch====
Sometimes the code you are writing requires the creation of new source files.  In order to allow these files to be included in your single patch file, you must perform the additional step of <tt>cvs add</tt>ing the file, or, if you do not have cvs write privileges, of imitating the <tt>cvs add</tt> process.
+
Sometimes the code you are writing requires the creation of new source files.  In order to allow these files to be included in your single patch file, you must perform the additional step of <code>hg add</code>ing the file before creating your patch.
  
To make a patch including a new file (which otherwise requires write access to the repository), use the third-party [http://developer.mozilla.org/en/docs/Creating_a_patch#Including_new_files_in_a_patch cvsdo] utility (which you'll need to install) to imitate the <tt>cvs add</tt> operation.  <tt>cvsdo add</tt> each new file (alternatively add <code>/Foo.h/0/dummy timestamp//</code> to <tt>path/to/file/CVS/Entries</tt>), then diff as normal. ''If you do not include <code>-N</code> in your <code>cvs diff</code> command, the diff will fail.''
+
====Removing files in a patch====
 +
Sometimes your patch obsoletes existing files in the tree. To indicate the file has been removed, <code>hg remove</code> each file you want to remove and then diff as usual.
  
To make a patch including files in new directories, first <tt>cvsdo add</tt> each new directory and then <tt>cvsdo add</tt> the files in those directories.  Then create a diff of everything NOT in a new directory, and finally <tt>cvsdo diff</tt> on ''each'' new directory, appending the changes to your existing diff (<tt>cvsdo diff path/to/newdir/ >> EXISTING_DIFF</tt>).  ''If you do not include <code>-N</code> in your <code>cvs diff</code> command, the diff will fail.''
+
====Moving, renaming, or copying files in a patch====
  
====Removing files in a patch====
+
Sometimes you want to move (or rename) a source file as part of your patch.  As described in [[Development:Helpful_Mercurial_Commands#rename (mv)|Helpful Mercurial Commands]], you should '''always''' use <code>hg mv</code> to move/rename a file; '''you should never move or rename a file outside of Mercurial''', because it will corrupt history.
Sometimes your patch obsoletes existing files in the tree. If you do not have write access to the repository, you will have to use the third-party [http://developer.mozilla.org/en/docs/Creating_a_patch#Including_new_files_in_a_patch cvsdo] utility (which you'll need to install) to imitate the <tt>cvs remove</tt> operation.  To indicate the file has been removed, first delete the file. Then <code>cvsdo remove</code> each removed file and diff as usual. ''If you do not include <code>-N</code> in your <code>cvs diff</code> command, the diff will fail.''
+
 
 +
If you want to copy a file, e.g., you are splitting a file into two pieces, or making a new file using much of the code in a given file and then changing some parts of it, use <code>hg cp</code> to create a ''copy'' of the original file with a new name/in its new location, and then make your changes to the files.
 +
 
 +
Once you have made your changes to the moved/renamed/copied files, you can diff as usual.
 +
 
 +
'''N.B.''' Showing file moves/renames or copies in patches requires you to have [[Development:Building:Mozilla_1.9.2_Branch#Setting_up_your_Hg_Environment|configured up your Hg environment]] to use “git-style” diffs as shown in the <code>[diff]</code> section of the Mercurial environment settings in the build instructions.  To apply a patch that contains file moves/renames or copies, you must use <code>hg import</code>; see the [[Development:Helpful_Mercurial_Commands#Applying a patch with renames, copies, or binary files|Applying a patch with renames, copies, or binary files]] section of [[Development:Helpful_Mercurial_Commands|Helpful Mercurial Commands]] for the full syntax.
 +
 
 +
=== <code>Localizable.strings</code> changes ===
 +
 
 +
Any UI string that appears in the code rather than in a nib needs to have an entry in a Cocoa <code>.strings</code> file (e.g., <code>Localizable.strings</code>, or, in some instances, a preference pane’s <code>Localizable.strings</code> files or one of the other <code>.strings</code> files).  When writing new strings, always use “curly quotes” in the actual string text.
 +
 
 +
====Editing <code>.strings</code> files====
 +
'''N.B''' All Camino <code>.strings</code> files exist in the tree as UTF-8 <code>.strings.in</code> files, since these files can be <code>diff</code>ed and included in patches.  Any changes to strings should be made to the <code>.strings.in</code> files, and these files should be included in your diff(The build system later converts these files to the UTf-16 <code>.strings</code> files Cocoa requires.) To see string changes, you must rebuild Camino from the command line.
 +
 
 +
====Adding new <code>.strings</code> files====
 +
If you need to add a new <code>.strings</code> file, create it in the tree as a UTF-8 <code>.strings.in</code> file and add it to the <code>STRINGS_FILES</code> block in <code>Makefile.in</code> (which will convert to the <code>.strings.in</code> file to a UTF-16 <code>.strings</code> files).  Then rebuild Camino from the command line and add the resulting <code>.strings</code> files in <code>generated/</code> to the project in the appropriate places.
 +
 
 +
====String freezes====
 +
 
 +
Branches are typically string-frozen (the string freeze usually coincides with the code freeze for the last beta milestone of a given release). ''Baring any critical security bugs requiring <code>.strings</code> changes, the '''CAMINO_2_0_BRANCH''' is string frozen, and no string changes should occur there.''
  
=== <tt>Localizable.strings</tt> changes ===
+
=== Project (<code>Camino.xcodeproj</code>) changes ===
 +
Making project changes has to be done using Xcode (when adding, deleting, or moving files, as well as changing most build/compiler options)<!--, specifically when adding files-->, using the copy of the project in the source directory.
  
Any UI string that appears in the code rather than in a nib needs to have an entry in <tt>Localizable.strings</tt> (or, in rare instances, a preference pane's <tt>Localizable.strings</tt> or one of the other <tt>.strings</tt> files). When writing new strings, always use “curly quotes” in the actual string text.
+
After making any project changes, simply diff the project file as well and include it in your patch (if you made any changes to build configuration or settings, be sure to diff <code>camino/config/</code> as well to pick up xcconfig changes). Please check to ensure that your project patches do not change the state of the Camino.app entries in the Camino and CaminoStatic targets (this is usually a problem with srcdir configurations).
  
Beginning with Camino 1.6a1 (23-Oct-2007 nightlies), we have converted all <tt>.strings</tt> files to UTF-8 <tt>.strings.in</tt> files which can be <tt>diff</tt>ed and which are then turned into UTF-16 <tt>.strings</tt> files in the build process.  Any changes to <tt>.strings.in</tt> files should be included in your diff.  Any new <tt>.strings</tt> files should be added to the tree as <tt>.strings.in</tt> files, converted to UTF-16 <tt>.strings</tt> files via the <code>STRINGS_FILES</code> mechanism in <tt>Makefile.in</tt>, and then add the resulting .strings files in <tt>generated/</tt> to the project in the appropriate places.
+
You should also verify that Xcode has not added “garbage” to any of the sections it modifies automatically, such as header or library search paths (this is most common with Gecko build products; see below).
 +
<!--
 +
(Removing files can be done by hand if you don't have the proper version of Xcode, but this isn't recommended because removing the obvious entries for files will still leave non-obvious entries in the project file and cause crashes/build failures.)-->
  
To see string changes, you must rebuild Camino from the command line.
+
====Adding new files to Camino and the project file====
  
====CAMINO_2_0_BRANCH====
+
At some point, you may need to add entirely new Camino (source) files to the project. This section concerns files added to the <code>src</code>, <code>resources</code>, and <code>PreferencePanes</code> directories, as well as the feed handlers, the <code>.plist</code> files at the root of <code>camino</code>, and any linked Xcode projects (frameworks); see [[#Adding Gecko build products to Camino and the project file|below]] for specific instructions for Gecko files and build products).
''Baring any critical security bugs requiring <tt>.strings</tt> changes, the '''CAMINO_2_0_BRANCH''' is string frozen, and no string changes should occur there.''
 
  
=== Project (<tt>Camino.xcodeproj</tt>) changes ===
+
To add the files, drag the files from the source directory to the appropriate folders in the “Groups & Files” panel; please try to keep the folder contents in order. When Xcode asks where to add them, choose both “CaminoApp” and “CaminoStaticApp” targets, and use “Relative to Project” paths. If you’re not adding source code, inspect the build and copy phases of both targets to make sure that Xcode has put the files in the appropriate phase.  
Making project changes has to be done using Xcode (when adding, deleting, or moving files, as well as changing most build/compiler options), specifically when adding files. After making any project changes, simply diff the project file as well and include it in your patch (if you made any changes to build configuration or settings, be sure to diff <tt>camino/config/</tt> as well to pick up xcconfig changes). Please check to ensure that your project patches do not change the state of the Camino.app entries in the Camino and CaminoStatic targets (this is usually a problem with srcdir configurations).
 
  
(Removing files can be done by hand if you don't have the proper version of Xcode, but this isn't recommended because removing the obvious entries for files will still leave non-obvious entries in the project file and cause crashes/build failures.)
+
Make sure Camino builds with your new files, then diff the project file as part of your changes (and inspect the project section of the diff for issues as instructed above).
  
 
====Adding Gecko build products to Camino and the project file====
 
====Adding Gecko build products to Camino and the project file====
* If the item is "optional" module (e.g., extension) and is not built by default, fix up <tt>confvars.sh</tt> (on the 1.8 branch, instead fix up the Camino section of <tt>configure.in</tt> and regenerate <tt>configure</tt>) to make Camino pull and build the item.
+
* If the item is "optional" module (e.g., extension) and is not built by default, fix up <code>confvars.sh</code> to make Camino pull and build the item.
** In some cases you can use an <tt>ac_add_options</tt> flag in your <tt>.mozconfig</tt> rather than patching <tt>confvars.sh</tt>/<tt>configure.in</tt>, but anything that's required to make Camino not burn when all is done needs to be declared in <tt>confvars.sh</tt>/<tt>configure.in</tt>.
+
** In some cases you can use an <code>ac_add_options</code> flag in your <code>.mozconfig</code> rather than patching <code>confvars.sh</code>, but anything that is required to make Camino not burn when all is done needs to be declared in <code>confvars.sh</code>.
* If the item required <tt>confvars.sh</tt>/<tt>configure.in</tt> or <tt>.mozconfig</tt> changes, do a full rebuild of your tree.
+
* If the item required <code>confvars.sh</code> or <code>.mozconfig</code> changes, do a full rebuild of your tree.
** You'll typically have to build both non-static and static unless you're intimately familiar with the location of build products and naming conventions
+
** You will typically have to build both non-static and static unless you’re intimately familiar with the location of build products and naming conventions.
* Add the libraries to appropriate build target(s), and using "Relative to Project" paths when adding
+
* Add the libraries to appropriate build target(s), and using “Relative to Project” paths when adding
** In most cases you'll want to add to the non-static (default) and static target separately
+
** In most cases you’ll want to add to the non-static (default) and static target separately
** There are separate folders in the Xcode "sources" tree for static and non-static libs, and for components and "regular" libs
+
** There are separate folders in the Xcode “sources” tree for static and non-static libraries, and for components and “regular” libraries.
* Adding is best done from a <code>srcdir</code> build, but can be done with an <code>objdir</code> build if you carefully fix paths manually; note that in some cases Xcode will resolve symlinks when adding, so you'll have to fix the paths to point back to <tt>dist/*</tt>
+
* Adding is best done from a <code>srcdir</code> build, but can be done with an <code>objdir</code> build if you carefully fix paths manually; note that in some cases Xcode will resolve symlinks when adding, so you will have to fix the paths to point back to <code>dist/*</code>.
* Move the products from Bundle Resources Copy Phase to the correct copy phase for that sort of Gecko product (for static builds, the static libs belong in the Libraries and Frameworks phase).  Different versions of Xcode mis-guess the Copy Phase destinations in different ways.
+
* Move the products from Bundle Resources Copy Phase to the correct copy phase for that sort of Gecko product (for static builds, the static libraries belong in the Libraries and Frameworks phase).  Different versions of Xcode mis-guess the Copy Phase destinations in different ways.
* Repeat the previous two steps for any <tt>.xpt</tt> files that the libraries might require
+
* Repeat the previous two steps for any <code>.xpt</code> files that the libraries might require.
* Check for, and remove if necessary, any bizarre changes to the header or library search paths that Xcode may have “helpfully” added for you; Xcode 3 typically adds garbage "QUOTED_SEARCH_PATHS_FOR_FOO" lines to the existing search paths and then declares that variable and sets it to the path of your newly-added library/header; you should delete these and add the library/header path to the existing search paths.
+
* Check for, and remove if necessary, any bizarre changes to the header or library search paths that Xcode may have “helpfully” added for you; Xcode 3 typically adds garbage "QUOTED_SEARCH_PATHS_FOR_FOO" lines to the existing search paths and then declares that variable and sets it to the path of your newly-added library/header; you should delete these and add the library/header path to the existing search paths (or to the declarations in the <code>.xcconfig</code>, if applicable).
* Fix <code>nsStaticComponents.cpp</code> for static builds
+
* Fix <code>nsStaticComponents.cpp</code> for static builds.
  
 
=== Nib changes ===
 
=== Nib changes ===
  
When editing nibs, be sure to follow the guidelines in [[Development:Editing Nibs]].  Until further notice, '''all nib work should be done only in Interface Builder 2.5.x''' (preferably IB 2.5.4 under Mac OS X 10.4, as a number of features do not work properly in IB 2.5.6 under Mac OS X 10.5 and its nibs can load more slowly, but it can be run under in a pinch).
+
When editing nibs, be sure to follow the guidelines in [[Development:Editing Nibs]].  With the exception of <code>FindBarTextured</code>, all nibs should be saved in the Xcode 2.x-compatible format.  '''Performance-critical nibs (<code>BrowserWindow</code>, <code>MainMenu</code>) should be saved in Interface Builder 2.5.x''' (preferably IB 2.5.4 under Mac OS X 10.4, as a number of features do not work properly in IB 2.5.6 under Mac OS X 10.5 and its nibs can load more slowly, but it can be used in a pinch).  If you are working on a performance-critical nib and do not have Xcode 2.5/IB 2.5, make your changes anyway but add a comment when uploading the nib that it will need to be resaved in IB 2.5.
  
The [[Development:Reviewing#Nib changes and other resource files|Nib changes and other resource files]] section on the Reviewing page includes additional information about preparing your nib changes for review.
+
The [[Development:Reviewing#Nib changes and other resource files|Nib changes and other resource files]] section on the Reviewing page includes additional information about preparing your nib changes for review and attaching them to bugs. '''Do not include nibs or other binary resource files as part of a git-style Hg patch.'''
  
 
=== Images and other binary resources ===
 
=== Images and other binary resources ===
  
Images should use the <tt>.tiff</tt> extension (with two “t"s) and be post-processed[https://bugzilla.mozilla.org/show_bug.cgi?id=384725#c13] with <tt>tiffutil</tt>.  See [[Development:Preparing Graphics]] for full details.
+
Images should use the <code>.tiff</code> extension (with two “t”s) and be post-processed[https://bugzilla.mozilla.org/show_bug.cgi?id=384725#c13] with <code>tiffutil</code>.  See [[Development:Preparing Graphics]] for full details.
 +
 
 +
The [[Development:Reviewing#Nib changes and other resource files|Nib changes and other resource files]] section on the Reviewing page includes additional information about preparing your images for review and attaching them to bugs. '''Do not include nibs or other binary resource files as part of a git-style Hg patch.'''

Latest revision as of 14:13, 28 March 2012

Here are some useful tips for coding a feature or bug fix for Camino and creating a patch to send your changes through review.

Code Style

Link to Mozilla coding style guidelines as well as create some for Camino, so we avoid Bug 308942 comment 8 and 14 and Bug 228840 comments 15, 16, 18, and 19.

Also link to Hacking Mozilla general intro somewhere in our contributor introduction

All new code should conform to Camino's style guidelines, which is a descendent of the Mozilla style guidelines (per decree by our leader):

- (bip)updateBat
{
  if (foo) {
    bar;
    blam;
  }
  else {
    baz;
    zap;
  }

  if (murkyWaterFlowsUnderTheBridgeNearTheTallOakTree ||
      dirtyWaterRunsPastTheOldMillNearTheOpera)
  {
    return nil;
  }
  else {
    bop;
  }
}

When the condition is multiple lines, braces are required on the statement (with the opening brace on a line by itself). If the if statement had braces, the else statement must also have braces.

Braces for single statements are optional (and discouraged, but some still include them).

We explicitly have no style rule regarding the placement of *. Aim to be consistent within a file in as much as the files are consistent.

As alluded to above, many of Camino’s older files have a mish-mash of styles. If you find particularly egregious cases, file bugs. You should not include unrelated whitespace or style cleanup in a patch; instead, you should perform cleanup in separate patches (or bugs) either before or after code changes. This keeps (blame for) code changes clear: if you are researching a section of code and hit a “mass style change” you can simply drop back to the previous revision and continue tracking the blame from there.

Coding for latest-OS-version features or using private APIs

Each situation is unique, but we have provided some existing examples of various strategies employed to correctly code for these situations.

Most of the code mentioned below was originally written for Camino 1.6 and allowed Camino to run on Mac OS X version 10.3.9 through 10.5, whether or not the particular feature was supported on that OS or declared in the default Camino SDK (10.4u). The learnWord: code dates from Camino 1.5, which had the same runtime requirements but built against the 10.3.9 SDK on PPC. The code for setting proxies dates from Camino 1.0, which ran on Mac OS X 10.2.8 through 10.4 (and built against the 10.2.8 SDK on PPC, 10.4u SDK on Intel).

The spell-checker's learnWord: method is undocumented on Mac OS X versions older than 10.5, but it is the only way to achieve proper "learned spelling" integration with the Mac OS X spelling system. The code supporting Camino’s use of this method is here and here.

On Mac OS X 10.5, Camino also supports building a Language sub-menu for switching spell-check languages.

Setting Spotlight metadata on files is still undocumented as of the 10.5 SDK. Our download listener code has a very well-documented and commented example of how to use a method like this safely while running on Mac OS X 10.4 and 10.5.

See also the Quarantine attributes code for an example of working with features available only on Mac OS X 10.5 while remaining compatible with older versions of the OS.

Setting Mac OS X proxy preferences is available only in the 10.4u SDK and later, although the feature is available at least as far back as Mac OS X 10.3.2. PreferenceManager.mm implements this feature safely across all Mac OS X versions Camino supports; see here.

mento also gives a pseudo-code explanation of how to guard and do runtime look-up for older SDKs in bug 397717 comment 21.

Situation

Description and mxr link

License Blocks

New Files

If you add a new file, be sure to give it the appropriate MPL2 license block from http://www.mozilla.org/MPL/headers/ (in C/C++/Obj-C/Obj-C++ files, this is the /* */ style); do not copy boilerplate from other files.

New Third-Party Code

If you are adding a new file from another source (including code you yourself have written but not written specifically for Camino)—for instance, when we include Wevah’s punycode methods—you should preserve the existing license blocks on the files. (You should, of course, first check to make sure that the license of the external code is compatible with the MPL v2.)

If you are adding an entire bundle of third-party code (e.g., Sparkle or Breakpad), Camino style is to include a README file in the code's top-level directory containing the source repository and licensing information for the third-party code, along with a list of any changes made to that code in order to use it in Camino (this list should be updated any time the third-party code is touched or updated). If the third-party code already includes a README file, add a README_CAMINO file instead.

When third-party code (either a few files or an entire bundle of code) is committed, the commit comment must include information or references to the author of the code and the source repository, if available. Make sure your committer is aware of this information.

Testing your patch

Once you’ve written code and gotten it to compile, you need to test your patch to make sure it works (and works as expected). There are several methods available that allow you to launch another Camino at the same time as your daily-use Camino and/or protect your daily-use profile from development builds. Some methods may be more useful in certain situations than others.

See Development:Using Custom Profiles for detailed explanations of the various methods.

Setting breakpoints in external files

Sometimes when debugging you will want to set breakpoints in files that are part of Camino, but not in the XCode project. For example, widget/src/cocoa/nsNativeThemeCocoa.mm. If you get an error about gdb not being able to find the file:linenumber, you will need to add the relevant shared library. Head to Run->Show->Shared Libraries... and set the Starting Level for the library to All. Which library to set it for depends on the file; for nsNativeThemeCocoa.mm, it is libwidget_mac.dylib. Turning off “Lazy Symbol Loading” in Debugger preferences may also help.

Preparing a patch to submit for review

Before creating a patch for review, be sure that your source tree is up-to-date and that Camino builds with your changes.

In the $SRCDIR, cd mozilla/camino and then hg pull -u. By default, Mercurial will attempt to merge any changes from the repository into your working copy. If it fails to merge the repository changes with your changes, hg will inform you with a message about conflicts (and, depending on your configuration, dump you into your merge tool). You will need to resolve the conflict manually and then continue as instructed by Mercurial’s message.

After updating your tree, switch back to $OBJDIR/camino and make (or build in Xcode) to make sure your changes and the latest code build. Ideally you would also launch Camino and test your changes one last time.

Proper patch format

You should have configured the proper patch format when setting up your Hg environment as part of the build instructions. Diffs should be done from $SRCDIR/mozilla/camino (or $SRCDIR/mozilla if you’re hacking on Gecko). All changes—including new source code files and project file changes, if applicable—should be submitted as a single patch (e.g., hg diff src/foo/bar src/baz/bam > bugnumber.patch), with the exception of changes to nibs and images; see below for more about requirements for those file types.

Adding new files in a patch

Sometimes the code you are writing requires the creation of new source files. In order to allow these files to be included in your single patch file, you must perform the additional step of hg adding the file before creating your patch.

Removing files in a patch

Sometimes your patch obsoletes existing files in the tree. To indicate the file has been removed, hg remove each file you want to remove and then diff as usual.

Moving, renaming, or copying files in a patch

Sometimes you want to move (or rename) a source file as part of your patch. As described in Helpful Mercurial Commands, you should always use hg mv to move/rename a file; you should never move or rename a file outside of Mercurial, because it will corrupt history.

If you want to copy a file, e.g., you are splitting a file into two pieces, or making a new file using much of the code in a given file and then changing some parts of it, use hg cp to create a copy of the original file with a new name/in its new location, and then make your changes to the files.

Once you have made your changes to the moved/renamed/copied files, you can diff as usual.

N.B. Showing file moves/renames or copies in patches requires you to have configured up your Hg environment to use “git-style” diffs as shown in the [diff] section of the Mercurial environment settings in the build instructions. To apply a patch that contains file moves/renames or copies, you must use hg import; see the Applying a patch with renames, copies, or binary files section of Helpful Mercurial Commands for the full syntax.

Localizable.strings changes

Any UI string that appears in the code rather than in a nib needs to have an entry in a Cocoa .strings file (e.g., Localizable.strings, or, in some instances, a preference pane’s Localizable.strings files or one of the other .strings files). When writing new strings, always use “curly quotes” in the actual string text.

Editing .strings files

N.B All Camino .strings files exist in the tree as UTF-8 .strings.in files, since these files can be diffed and included in patches. Any changes to strings should be made to the .strings.in files, and these files should be included in your diff. (The build system later converts these files to the UTf-16 .strings files Cocoa requires.) To see string changes, you must rebuild Camino from the command line.

Adding new .strings files

If you need to add a new .strings file, create it in the tree as a UTF-8 .strings.in file and add it to the STRINGS_FILES block in Makefile.in (which will convert to the .strings.in file to a UTF-16 .strings files). Then rebuild Camino from the command line and add the resulting .strings files in generated/ to the project in the appropriate places.

String freezes

Branches are typically string-frozen (the string freeze usually coincides with the code freeze for the last beta milestone of a given release). Baring any critical security bugs requiring .strings changes, the CAMINO_2_0_BRANCH is string frozen, and no string changes should occur there.

Project (Camino.xcodeproj) changes

Making project changes has to be done using Xcode (when adding, deleting, or moving files, as well as changing most build/compiler options), using the copy of the project in the source directory.

After making any project changes, simply diff the project file as well and include it in your patch (if you made any changes to build configuration or settings, be sure to diff camino/config/ as well to pick up xcconfig changes). Please check to ensure that your project patches do not change the state of the Camino.app entries in the Camino and CaminoStatic targets (this is usually a problem with srcdir configurations).

You should also verify that Xcode has not added “garbage” to any of the sections it modifies automatically, such as header or library search paths (this is most common with Gecko build products; see below).

Adding new files to Camino and the project file

At some point, you may need to add entirely new Camino (source) files to the project. This section concerns files added to the src, resources, and PreferencePanes directories, as well as the feed handlers, the .plist files at the root of camino, and any linked Xcode projects (frameworks); see below for specific instructions for Gecko files and build products).

To add the files, drag the files from the source directory to the appropriate folders in the “Groups & Files” panel; please try to keep the folder contents in order. When Xcode asks where to add them, choose both “CaminoApp” and “CaminoStaticApp” targets, and use “Relative to Project” paths. If you’re not adding source code, inspect the build and copy phases of both targets to make sure that Xcode has put the files in the appropriate phase.

Make sure Camino builds with your new files, then diff the project file as part of your changes (and inspect the project section of the diff for issues as instructed above).

Adding Gecko build products to Camino and the project file

  • If the item is "optional" module (e.g., extension) and is not built by default, fix up confvars.sh to make Camino pull and build the item.
    • In some cases you can use an ac_add_options flag in your .mozconfig rather than patching confvars.sh, but anything that is required to make Camino not burn when all is done needs to be declared in confvars.sh.
  • If the item required confvars.sh or .mozconfig changes, do a full rebuild of your tree.
    • You will typically have to build both non-static and static unless you’re intimately familiar with the location of build products and naming conventions.
  • Add the libraries to appropriate build target(s), and using “Relative to Project” paths when adding
    • In most cases you’ll want to add to the non-static (default) and static target separately
    • There are separate folders in the Xcode “sources” tree for static and non-static libraries, and for components and “regular” libraries.
  • Adding is best done from a srcdir build, but can be done with an objdir build if you carefully fix paths manually; note that in some cases Xcode will resolve symlinks when adding, so you will have to fix the paths to point back to dist/*.
  • Move the products from Bundle Resources Copy Phase to the correct copy phase for that sort of Gecko product (for static builds, the static libraries belong in the Libraries and Frameworks phase). Different versions of Xcode mis-guess the Copy Phase destinations in different ways.
  • Repeat the previous two steps for any .xpt files that the libraries might require.
  • Check for, and remove if necessary, any bizarre changes to the header or library search paths that Xcode may have “helpfully” added for you; Xcode 3 typically adds garbage "QUOTED_SEARCH_PATHS_FOR_FOO" lines to the existing search paths and then declares that variable and sets it to the path of your newly-added library/header; you should delete these and add the library/header path to the existing search paths (or to the declarations in the .xcconfig, if applicable).
  • Fix nsStaticComponents.cpp for static builds.

Nib changes

When editing nibs, be sure to follow the guidelines in Development:Editing Nibs. With the exception of FindBarTextured, all nibs should be saved in the Xcode 2.x-compatible format. Performance-critical nibs (BrowserWindow, MainMenu) should be saved in Interface Builder 2.5.x (preferably IB 2.5.4 under Mac OS X 10.4, as a number of features do not work properly in IB 2.5.6 under Mac OS X 10.5 and its nibs can load more slowly, but it can be used in a pinch). If you are working on a performance-critical nib and do not have Xcode 2.5/IB 2.5, make your changes anyway but add a comment when uploading the nib that it will need to be resaved in IB 2.5.

The Nib changes and other resource files section on the Reviewing page includes additional information about preparing your nib changes for review and attaching them to bugs. Do not include nibs or other binary resource files as part of a git-style Hg patch.

Images and other binary resources

Images should use the .tiff extension (with two “t”s) and be post-processed[1] with tiffutil. See Development:Preparing Graphics for full details.

The Nib changes and other resource files section on the Reviewing page includes additional information about preparing your images for review and attaching them to bugs. Do not include nibs or other binary resource files as part of a git-style Hg patch.