mercurial bug fix (issue1590)

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.