A patch for the mercurial bug Bogus exit code 0 for some commands was produced. In the process a few pages of the wiki were updated to improve the introduction to the contribution process. The tests were tuned on a high end machine to run in less than 2mn instead of 15mn.
Patrick Mezard who has been a contributor for mercurial during many years proposed that the Bogus exit code 0 for some commands bug was a good candidate for a first contribution.
Running the tests
A first attempt at running the tests showed that it could not be done from the Debian GNU/Linux packages:
$ apt-get source mercurial $ apt-get build-dep mercurial $ debuild -uc -us ... make[2]: Leaving directory `/home/loic/software/mercurial/mercurial-1.8.1/doc' # Tests are not yet ready to be run in Debian build context #/usr/bin/make tests
In any case, contributions should be against http://selenic.com/hg and this is confirmed on the IRC channel at irc.freenode.net#mercurial :
(01:01:44 PM) mg: we used to have a separate repository for the stable branch, but we're now using named branches (01:01:51 PM) dachary: ah ! (01:02:38 PM) dachary: mg: does this mean http://mercurial.selenic.com/wiki/DeveloperRepos should be updated accordingly ? Or am I reading it incorrectly ? (01:02:48 PM) mg: what we're saying is that bugfixes should be based on "convert/svn: fix _iterfiles() output in root dir case (issue2647)" in http://hg.intevation.org/mercurial/crew/graph/a1dae38acbc6 and not on "test-https: match output from 31eac42d9123" (01:03:58 PM) mg: well, I confused you a little... the page is correct and you'll notice that hg-stable has the "convert/svn: fix..." changeset as the tip (01:04:05 PM) mg: so hg-stable is a subset of hg
Since the build dependencies were extracted when attempting to build the Debian GNU/Linux package, the tests could be run immediately:
hg clone http://selenic.com/hg mercurial cd mercurial make ; make local ; make tests
The only problem was that some test insisted on using the installed mercurial libraries.
... Some tests failed : Ran 405 tests, 14 skipped, 21 failed.
To resolve this issue mercurial was uninstalled:
apt-get remove mercurial-common mercurial
And the tests successfully run. It took 14 minutes on a Intel(R) Core(TM)2 Duo CPU P9400 @ 2.40GHz
. The use of the -j
was
suggested as a mean to speed up tests execution.
(01:27:18 PM) mg: dachary: did you use the -j option to run several tests in parallel? (01:27:34 PM) mg: that can make a big difference because of all the I/O (01:27:51 PM) mg: I suggest number_of_cores * 2 (01:52:48 PM) mg: with a SSD in the machine I can run the test suite in 2 minutes on a quad core i7
Using a ram file system and parallel execution, it was reduced to 1mn11s on a dual Intel(R) Xeon(R) CPU X5670 @ 2.93GHz
mkdir /mercurial mount -o size=1G -t tmpfs tmpfs /mercurial make TESTFLAGS=-j24 tests
(03:03:14 PM) dachary: mg: python run-tests.py -j24 : real 1mn11.955s :-D (03:04:09 PM) mg: dachary: cool, that's a new record!
As it turns out, running the tests with make TESTFLAGS=-j24 tests without a ram disk completes in 1m15s (versus 1m11s with a ramdisk). If there is enough RAM, the kernel cache will basically optimize in the same way tmpfs does. During these experimentations, a bug was reported because it turns out that make test writes in $HOME.
Learning how to contribute to mercurial
In order to comment on the proposed bug, a BTS account was created and the confirmation email retrieved from the spam folder a few hours later. Starting from the home page, the developer information can be found by following the News/Wiki link and then, at the bottom of the page, the Developer Info link. The following links can be read in sequence:
- Contributing Changes
- Coding Style
- It is worth opening a separate window with this page to be used as a reference. A reference to unified try/except/finally was added
- Mail patch
- It is the preferred submission method as can be seen with an example. Using patchbomb with the proper arguments is the shortest way.
- Automated Checking
- Turns out to be redundant with running the tests because one of them does the same
Producing a bug fix
After reading the instructions on Writing Tests and experimenting, a new one was created to demonstrate the problem : hg annotate would have an exit code of zero even if given a non existent file.
http://mercurial.selenic.com/bts/issue1590 The exit status of annotate when a file does not exist must not be 0 $ hg init $ hg annotate nonexistent nonexistent: no such file in rev 000000000000 abort: nonexistent did not match any file [255]
From there the instructions on how to debug tests were followed (and clarified) to establish a diagnostic that was appended to the bug report.
A tentative patch was posted using the patchbomb method:
hg email --rev b3a0db338d9b --subject 'hg annotate nonexistent exits on error (issue1590)'
Patrick Mezard took a few minutes to comment on it. It was then reworked into another patch that was queued by Matt Mackall an hour later.