Difference between revisions of "Development:Committing"
(Commit existing "Checking In" section of Development:Coding wholesale; editing to come) |
(→Figuring out what patches (commits) you are pushing to the remote repository: add the caveat here) |
||
(4 intermediate revisions by the same user not shown) | |||
Line 1: | Line 1: | ||
− | + | After a patch has '''review+''' from two reviewers and '''superreview+''' from one of the Camino leads, it needs to be committed to one or more of Camino’s code repositories. | |
− | After a patch has '''review+''' from two reviewers and '''superreview+''' from one of the Camino leads, it needs to be | ||
− | Check-ins for Camino can be made by any of the super-reviewers listed | + | Check-ins for Camino can be made by any of the super-reviewers listed [[Development:Reviewing#.E2.80.9CSuper.E2.80.9D_Reviews|here]], as well as ardissone, froodian, hwaara and kreeger (note that froodian, hwaara, kreeger, and smfr are not currently checking in Camino patches, nor, with some exceptions, will pinkerton or mento check in patches they have not written). |
− | + | ==Checking In== | |
− | + | Once code written by contributors who do not have commit access has received requisite reviews (and perhaps approvals, too), it falls to Camino committers to check the new code into the repository. Sometimes one of the committers is following the bug and will land the patch as soon as the committer can watch the tree. Other times the patch author may need to add <tt>checkin-needed</tt> to the bug's Keyword field in order to notify the committers that the patch is ready to land. | |
+ | |||
+ | Typically Camino committers try to land all patches within a day or so. However, sometimes the committer will want to wait for a quiet tree to land a large change, or a committer may need to wait a few days until he can set aside enough time to watch the tree. As with other things in Camino development, politely poke the relevant person if something is blocking your work (e.g., if a particular patch is adding a new feature you need to build upon, or if landing blocks spinning a release). | ||
+ | |||
+ | If you commit a patch and it turns a tinderbox red or orange, you are responsible for figuring out and fixing the problem, even if the patch is not yours. If the patch author is around, he or she should help you out. Use the tinderbox logs to your advantage. If you cannot figure out how to fix the bustage (and neither can the patch author, or he or she is not available), back out the patch and reopen the bug. When reopening the bug, make a helpful comment about what you observed and anything you may have discovered (including links to tinderbox logs). You are still on the hook until the tree goes green, so do not back out and leave (just as you would not check in and leave). | ||
+ | |||
+ | ==Tree Status== | ||
+ | Note that <code>hg.mozilla.org/camino</code> and the <code>mozilla/camino</code> directory hierarchy in CVS are open for approved Camino check-ins regardless of the state of various Mozilla trees and branches, with a few exceptions: IT work on the Mozilla network, repositories, or other systems related to building; red Camino tinderboxen; missing Camino perf or nightly tinderboxen; tagging of major branches; etc. | ||
'''N.B''' When the Camino tree is closed while the Mozilla tree is closed, there is typically a note on the [http://tinderbox.mozilla.org/showbuilds.cgi?tree=Camino Camino tinderbox] explaining the closure, and the closure is often mentioned in the “topic” in the [irc://irc.mozilla.org#camino #camino] channel on IRC. '''However''', when the opposite is true—Mozilla is closed and Camino is open—there is typically no indication of the fact. If you are unsure of the state of the Camino tree, ask on in the [irc://irc.mozilla.org#camino #camino] channel on IRC. | '''N.B''' When the Camino tree is closed while the Mozilla tree is closed, there is typically a note on the [http://tinderbox.mozilla.org/showbuilds.cgi?tree=Camino Camino tinderbox] explaining the closure, and the closure is often mentioned in the “topic” in the [irc://irc.mozilla.org#camino #camino] channel on IRC. '''However''', when the opposite is true—Mozilla is closed and Camino is open—there is typically no indication of the fact. If you are unsure of the state of the Camino tree, ask on in the [irc://irc.mozilla.org#camino #camino] channel on IRC. | ||
− | + | ==Checkin tips for new (and old) committers== | |
− | + | ===Mercurial=== | |
Remember that Hg is a distributed VCS, so that most operations are local. In particular, where “committing” to the official repository was a single operation with CVS (<code>cvs commit</code>), it is a pair of operations with Hg (the local-only <code>hg commit</code> and the local-to-remote <code>hg push</code>). | Remember that Hg is a distributed VCS, so that most operations are local. In particular, where “committing” to the official repository was a single operation with CVS (<code>cvs commit</code>), it is a pair of operations with Hg (the local-only <code>hg commit</code> and the local-to-remote <code>hg push</code>). | ||
− | + | ====Configuration changes to allow you to check code into the official repository==== | |
In order to be able to push changes to a remote hg repository, you will need to make a pair of configuration changes: | In order to be able to push changes to a remote hg repository, you will need to make a pair of configuration changes: | ||
Line 26: | Line 32: | ||
default-push = ssh://hg.mozilla.org/camino/</nowiki></pre> | default-push = ssh://hg.mozilla.org/camino/</nowiki></pre> | ||
− | + | ====Add a hook to ensure complete checkin messages==== | |
− | For some reason Camino committers appear far more likely to drop a required part of the commit message with Hg commits than we did with CVS. An easy way to prevent bad commit messages is to use a “hook” that checks your commit message for required data and aborts the commit with a helpful error when the data is missing. | + | For some reason Camino committers appear far more likely to drop a required part of the commit message with Hg commits than we did with CVS. An easy way to prevent bad commit messages is to use a “hook” that checks your commit message for required data and aborts the commit with a helpful error when the data is missing (particularly since Hg commit messages cannot be corrected after the fact, unlike with CVS). |
Smokey has written a hook that ensures commit messages contain the bug number and at least one reviewer or superreviewer: [http://hg.mozilla.org/users/alqahira_ardisson.org/misc/file/tip/bugnum bugnum]. By default the <code>bugnum</code> hook only enforces these commit message rules for <code>mozilla-*</code> and <code>camino</code>, so you can use the script as a global hook and still commit to other Hg repositories that don’t require bug numbers or reviews, or you can customize the hook to add additional repositories to the <code>grep</code> pattern in order to enforce the rules on additional repositories. | Smokey has written a hook that ensures commit messages contain the bug number and at least one reviewer or superreviewer: [http://hg.mozilla.org/users/alqahira_ardisson.org/misc/file/tip/bugnum bugnum]. By default the <code>bugnum</code> hook only enforces these commit message rules for <code>mozilla-*</code> and <code>camino</code>, so you can use the script as a global hook and still commit to other Hg repositories that don’t require bug numbers or reviews, or you can customize the hook to add additional repositories to the <code>grep</code> pattern in order to enforce the rules on additional repositories. | ||
Line 38: | Line 44: | ||
For more about Hg hooks, see [http://hgbook.red-bean.com/read/handling-repository-events-with-hooks.html Chapter 10. Handling repository events with hooks] in the Hg book. | For more about Hg hooks, see [http://hgbook.red-bean.com/read/handling-repository-events-with-hooks.html Chapter 10. Handling repository events with hooks] in the Hg book. | ||
− | + | ====Checking in patches from others==== | |
Mercurial has a way of recording the actual patch author even when someone else is committing and pushing the patch. When you’re committing a patch written by someone else, use the following commit command syntax:<pre><nowiki>hg commit -u "Patch Author <camino.dev@example.org>" -m "Complete commit message" file1 file2</nowiki></pre> | Mercurial has a way of recording the actual patch author even when someone else is committing and pushing the patch. When you’re committing a patch written by someone else, use the following commit command syntax:<pre><nowiki>hg commit -u "Patch Author <camino.dev@example.org>" -m "Complete commit message" file1 file2</nowiki></pre> | ||
Line 47: | Line 53: | ||
Then commit all of Patch Author’s patches using <code>$ hg patchGuy -m "Complete commit message" file1 file2</code>. | Then commit all of Patch Author’s patches using <code>$ hg patchGuy -m "Complete commit message" file1 file2</code>. | ||
− | + | ====Figuring out what patches (commits) you are pushing to the remote repository==== | |
+ | |||
+ | Once you’ve committed some patches and are ready to push them to the official repository, you can sanity-check what you’re about to send by using the <code>hg outgoing</code> command. If the list of patches (commits) looks correct, <code>hg push</code> will send them to the remote repository. | ||
− | + | Be sure to paste the changeset URLs in the bug when resolving it (the [http://hg.mozilla.org/users/alqahira_ardisson.org/misc/file/tip/echoURL echoURL] hook can help with that, at least for the first changeset of a push)! | |
− | + | ===CVS=== | |
− | + | ====Checking in nibs==== | |
Sometimes a person who doesn't have CVS access will post a new or updated nib on a bug that needs to be checked in. The problem with this is that since a nib is technically a directory, there is a CVS folder inside the nib. If you attempt to check in a nib that has been changed by someone who has the <tt>CVSROOT</tt> set as "<tt>:pserver:anonymous@cvs-mirror.mozilla.org:/cvsrooot</tt>" (''e.g.'', everyone without cvs write access), you will receive an authorization error from cvs. The problem is that the <tt>CVS/Root</tt> file is set as the anonymous <tt>CVSROOT</tt>, not your "<tt>user%email.com@cvs.mozilla.org:/cvsroot</tt>" <tt>CVSROOT</tt>. | Sometimes a person who doesn't have CVS access will post a new or updated nib on a bug that needs to be checked in. The problem with this is that since a nib is technically a directory, there is a CVS folder inside the nib. If you attempt to check in a nib that has been changed by someone who has the <tt>CVSROOT</tt> set as "<tt>:pserver:anonymous@cvs-mirror.mozilla.org:/cvsrooot</tt>" (''e.g.'', everyone without cvs write access), you will receive an authorization error from cvs. The problem is that the <tt>CVS/Root</tt> file is set as the anonymous <tt>CVSROOT</tt>, not your "<tt>user%email.com@cvs.mozilla.org:/cvsroot</tt>" <tt>CVSROOT</tt>. | ||
Line 60: | Line 68: | ||
Alternatively, instead of dropping that person’s nib in place, use <code> cp /path/to/other/nib/*.nib /path/to/your/tree/nib; touch /path/to/your/nib</code> to copy their nib files into your tree. | Alternatively, instead of dropping that person’s nib in place, use <code> cp /path/to/other/nib/*.nib /path/to/your/tree/nib; touch /path/to/your/nib</code> to copy their nib files into your tree. | ||
− | + | ====Trunk/branch mismatches==== | |
Sometimes (due mostly to Gecko changes on the trunk) the trunk and branch versions of a patch will diverge and you can't use cross-commit to land both places. '''If you're confident the branch patch has been well-tested''' and don't have a branch tree, you can pull just the files you need to commit (rather than pull an entire tree). | Sometimes (due mostly to Gecko changes on the trunk) the trunk and branch versions of a patch will diverge and you can't use cross-commit to land both places. '''If you're confident the branch patch has been well-tested''' and don't have a branch tree, you can pull just the files you need to commit (rather than pull an entire tree). | ||
Line 68: | Line 76: | ||
# Apply the patch and commit the files "as normal" | # Apply the patch and commit the files "as normal" | ||
− | + | ====Backing out a patch==== | |
Sometimes you accidentally commit something that shouldn't have landed yet, or sometimes (hopefully rarely) you commit something that breaks the tree and needs to be backed out. To back out a patch, follow these steps: | Sometimes you accidentally commit something that shouldn't have landed yet, or sometimes (hopefully rarely) you commit something that breaks the tree and needs to be backed out. To back out a patch, follow these steps: | ||
Line 78: | Line 86: | ||
# <code>cvs commit</code> "as normal"; this will commit a new copy of the file(s) with the incorrect change backed out | # <code>cvs commit</code> "as normal"; this will commit a new copy of the file(s) with the incorrect change backed out | ||
− | + | ====Adding new files==== | |
* <code>cvs add</code> | * <code>cvs add</code> | ||
* <code>cvs add -kb</code> for any binary files, including images, UTF-16-encoded files (e.g., <code>.strings</code> files from external projects), and <code>keyedobjects.nib</code> files inside .nibs. | * <code>cvs add -kb</code> for any binary files, including images, UTF-16-encoded files (e.g., <code>.strings</code> files from external projects), and <code>keyedobjects.nib</code> files inside .nibs. | ||
− | + | ====Adding new nibs==== | |
* <code>cvs add</code> the wrapper directory | * <code>cvs add</code> the wrapper directory | ||
* <code>cvs add -kb</code> on '''keyedobjects.nib''' | * <code>cvs add -kb</code> on '''keyedobjects.nib''' |
Latest revision as of 20:56, 10 July 2011
After a patch has review+ from two reviewers and superreview+ from one of the Camino leads, it needs to be committed to one or more of Camino’s code repositories.
Check-ins for Camino can be made by any of the super-reviewers listed here, as well as ardissone, froodian, hwaara and kreeger (note that froodian, hwaara, kreeger, and smfr are not currently checking in Camino patches, nor, with some exceptions, will pinkerton or mento check in patches they have not written).
Contents
Checking In
Once code written by contributors who do not have commit access has received requisite reviews (and perhaps approvals, too), it falls to Camino committers to check the new code into the repository. Sometimes one of the committers is following the bug and will land the patch as soon as the committer can watch the tree. Other times the patch author may need to add checkin-needed to the bug's Keyword field in order to notify the committers that the patch is ready to land.
Typically Camino committers try to land all patches within a day or so. However, sometimes the committer will want to wait for a quiet tree to land a large change, or a committer may need to wait a few days until he can set aside enough time to watch the tree. As with other things in Camino development, politely poke the relevant person if something is blocking your work (e.g., if a particular patch is adding a new feature you need to build upon, or if landing blocks spinning a release).
If you commit a patch and it turns a tinderbox red or orange, you are responsible for figuring out and fixing the problem, even if the patch is not yours. If the patch author is around, he or she should help you out. Use the tinderbox logs to your advantage. If you cannot figure out how to fix the bustage (and neither can the patch author, or he or she is not available), back out the patch and reopen the bug. When reopening the bug, make a helpful comment about what you observed and anything you may have discovered (including links to tinderbox logs). You are still on the hook until the tree goes green, so do not back out and leave (just as you would not check in and leave).
Tree Status
Note that hg.mozilla.org/camino
and the mozilla/camino
directory hierarchy in CVS are open for approved Camino check-ins regardless of the state of various Mozilla trees and branches, with a few exceptions: IT work on the Mozilla network, repositories, or other systems related to building; red Camino tinderboxen; missing Camino perf or nightly tinderboxen; tagging of major branches; etc.
N.B When the Camino tree is closed while the Mozilla tree is closed, there is typically a note on the Camino tinderbox explaining the closure, and the closure is often mentioned in the “topic” in the #camino channel on IRC. However, when the opposite is true—Mozilla is closed and Camino is open—there is typically no indication of the fact. If you are unsure of the state of the Camino tree, ask on in the #camino channel on IRC.
Checkin tips for new (and old) committers
Mercurial
Remember that Hg is a distributed VCS, so that most operations are local. In particular, where “committing” to the official repository was a single operation with CVS (cvs commit
), it is a pair of operations with Hg (the local-only hg commit
and the local-to-remote hg push
).
Configuration changes to allow you to check code into the official repository
In order to be able to push changes to a remote hg repository, you will need to make a pair of configuration changes:
- If you do not already have a
~/.ssh/config
file to help manage your ssh connections, create the file using your favorite text editor. Once the file exists, insert the following lines:Host hg.mozilla.org
User user@domain.tld (where user@domain.tld is the email address associated with your Mozilla Hg/LDAP account)
- In each local clone from which you wish to push changes (e.g.,
mozilla-1.9.2
andcamino
), edit the.hg/hgrc
file and add adefault-push
line. Thedefault-push
line should look exactly the same as thedefault
line, except you should replacedefault
’shttp://
withssh://
. For example, the.hg/hgrc
file for thecamino
repository should look like this:[paths]
default = http://hg.mozilla.org/camino/
default-push = ssh://hg.mozilla.org/camino/
Add a hook to ensure complete checkin messages
For some reason Camino committers appear far more likely to drop a required part of the commit message with Hg commits than we did with CVS. An easy way to prevent bad commit messages is to use a “hook” that checks your commit message for required data and aborts the commit with a helpful error when the data is missing (particularly since Hg commit messages cannot be corrected after the fact, unlike with CVS).
Smokey has written a hook that ensures commit messages contain the bug number and at least one reviewer or superreviewer: bugnum. By default the bugnum
hook only enforces these commit message rules for mozilla-*
and camino
, so you can use the script as a global hook and still commit to other Hg repositories that don’t require bug numbers or reviews, or you can customize the hook to add additional repositories to the grep
pattern in order to enforce the rules on additional repositories.
- Install the
bugnum
script somewhere and make sure that it is executable. - Add the following lines to your
~/.hgrc
file:[hooks]
pretxncommit.bugnum = /path/to/bugnum $HG_NODE (If, for some reason, you don’t want a global hook, you can instead add these lines to the .hg/hgrc
file inside each individual repository where you want to use the hook.)
For more about Hg hooks, see Chapter 10. Handling repository events with hooks in the Hg book.
Checking in patches from others
Mercurial has a way of recording the actual patch author even when someone else is committing and pushing the patch. When you’re committing a patch written by someone else, use the following commit command syntax:
hg commit -u "Patch Author <camino.dev@example.org>" -m "Complete commit message" file1 file2
If you find yourself committing patches for someone else frequently, you can add an alias to your ~/.hgrc
file to save you some typing:
[alias] patchGuy = commit -u "Patch Author <camino.dev@example.org>"
Then commit all of Patch Author’s patches using $ hg patchGuy -m "Complete commit message" file1 file2
.
Figuring out what patches (commits) you are pushing to the remote repository
Once you’ve committed some patches and are ready to push them to the official repository, you can sanity-check what you’re about to send by using the hg outgoing
command. If the list of patches (commits) looks correct, hg push
will send them to the remote repository.
Be sure to paste the changeset URLs in the bug when resolving it (the echoURL hook can help with that, at least for the first changeset of a push)!
CVS
Checking in nibs
Sometimes a person who doesn't have CVS access will post a new or updated nib on a bug that needs to be checked in. The problem with this is that since a nib is technically a directory, there is a CVS folder inside the nib. If you attempt to check in a nib that has been changed by someone who has the CVSROOT set as ":pserver:anonymous@cvs-mirror.mozilla.org:/cvsrooot" (e.g., everyone without cvs write access), you will receive an authorization error from cvs. The problem is that the CVS/Root file is set as the anonymous CVSROOT, not your "user%email.com@cvs.mozilla.org:/cvsroot" CVSROOT.
The easiest way to fix this problem is to copy a Root
file from one of the directories you have pulled to and paste it inside the Something.nib/CVS folder.
Alternatively, instead of dropping that person’s nib in place, use cp /path/to/other/nib/*.nib /path/to/your/tree/nib; touch /path/to/your/nib
to copy their nib files into your tree.
Trunk/branch mismatches
Sometimes (due mostly to Gecko changes on the trunk) the trunk and branch versions of a patch will diverge and you can't use cross-commit to land both places. If you're confident the branch patch has been well-tested and don't have a branch tree, you can pull just the files you need to commit (rather than pull an entire tree).
- Set up your cvs environment as normal (i.e.,
export cvsroot
) cvs co -r MOZILLA_1_8_BRANCH -d myfolder mozilla/camino/src/browser/foo mozilla/camino/PreferencePanes/Naviagtion/bar
- where
MOZILLA_1_8_BRANCH
is the branch tag,myfolder
is the folder in which you want the stub tree to live, andfoo
andbar
are the files contained in the patch
- where
- Apply the patch and commit the files "as normal"
Backing out a patch
Sometimes you accidentally commit something that shouldn't have landed yet, or sometimes (hopefully rarely) you commit something that breaks the tree and needs to be backed out. To back out a patch, follow these steps:
- If you need to back out something only on the branch and have no branch tree (i.e., you check in using cross-commit), follow the steps above to grab a "stub" branch tree of the files the patch touched.
- Consult bonsai for the correct backout command(s) to pull files (one per file) to run
- The "Show commands which could be used to back out these changes" link at the top of the appropriate bonsai page will give you the appropriate command, in the form of
cvs update -j1.17.2.4 -j1.17.2.3 mozilla/camino/src/embedding/CHClickListener.mm
There will be one command per file (this particular example is a branch file, as you can see from the multiple sub-versions; the current [bad] version of the file is listed first and the previous [good] version is second)
- The "Show commands which could be used to back out these changes" link at the top of the appropriate bonsai page will give you the appropriate command, in the form of
touch
each filecvs diff -u
the file(s) to sanity-check that the bad changes have been reverted in your local copy of the filescvs commit
"as normal"; this will commit a new copy of the file(s) with the incorrect change backed out
Adding new files
cvs add
cvs add -kb
for any binary files, including images, UTF-16-encoded files (e.g.,.strings
files from external projects), andkeyedobjects.nib
files inside .nibs.
Adding new nibs
cvs add
the wrapper directorycvs add -kb
on keyedobjects.nibcvs add
on the remaining .nib files inside the wrapper directory
[Qalaat-Samaan:trunk/mozilla/camino] smokey% cvs add resources/localized/English.lproj/SafeBrowsingBar.nib ? resources/localized/English.lproj/SafeBrowsingBar.nib/classes.nib ? resources/localized/English.lproj/SafeBrowsingBar.nib/info.nib ? resources/localized/English.lproj/SafeBrowsingBar.nib/keyedobjects.nib Directory /cvsroot/mozilla/camino/resources/localized/English.lproj/SafeBrowsingBar.nib added to the repository [Qalaat-Samaan:trunk/mozilla/camino] smokey% cvs add -kb resources/localized/English.lproj/SafeBrowsingBar.nib/keyedobjects.nib cvs add: scheduling file `resources/localized/English.lproj/SafeBrowsingBar.nib/keyedobjects.nib' for addition cvs add: use 'cvs commit' to add this file permanently [Qalaat-Samaan:trunk/mozilla/camino] smokey% cvs add resources/localized/English.lproj/SafeBrowsingBar.nib/classes.nib resources/localized/English.lproj/SafeBrowsingBar.nib/info.nib cvs add: scheduling file `resources/localized/English.lproj/SafeBrowsingBar.nib/classes.nib' for addition cvs add: scheduling file `resources/localized/English.lproj/SafeBrowsingBar.nib/info.nib' for addition cvs add: use 'cvs commit' to add these files permanently