/!\ This page is primarily intended for Mercurial's developers.

Contributing Changes

How to help us improve Mercurial's code.

1. Submission checklist

Please double-check your patch meets the following guidelines (explained below) before hitting send:

  1. first line of commit message is of the form "topic: uncapitalized, no trailing period"

  2. bugs that are resolved are mentioned in summary in the form "(issueNNNN)" (no space)

  3. if emailing, patches are sent to the mercurial-devel mailing list (not the BTS) and are properly formatted (see below)

  4. patch does just one thing (if you need a bullet list, split your patch)

  5. patch contains no other gratuitous changes:
    • whitespace changes
    • code movement
    • typo fixes
    • string changes
  6. patch leaves Mercurial in a working state (doesn't depend on future patches)
  7. test suite has been updated and runs cleanly

  8. code passes check-code and matches coding style (no foo_bar variable names!)

  9. relevant help text is updated (see mercurial/help/)

  10. appropriate branch is indicated in email subject (eg hg email --flag stable)
    • if a major release code freeze is in effect (see TimeBasedReleasePlan) patches intended for the default branch should not be sent; instead, send them immediately after the release is completed

  11. version is indicated in email subject if this is an amended patch, (eg hg email --flag v2 if it has been amended once, or v3, v4, etc.)

2. Getting started

Start by cloning a copy of Mercurial's main development repo:

hg clone https://www.mercurial-scm.org/repo/hg-committed

You'll also want to read the following pages:

Other pages you may find important:

3. Organizing patches

{X} If your first submission is not 'minimal', you will probably be sent back here. Save yourself time and start small!

If you're making a large change, we're probably going to want it broken into a series of smaller patches (see OneChangePerPatch). This makes for easier review and tests both for us and for you. This can be tricky at first and you might find tools like MQ and record useful in this process.

Each patch in a series should:

<!> Do not mix formatting changes, organizational changes, or multiple functional changes in the same patch!

Things to consider:

3.1. Editing history

You will almost certainly find it necessary to do some form of history editing to generate clean commits, especially after you get your first review feedback. There are multiple tools of varying degrees of power available for this purpose:

4. Coding style and testing

<!> If you send a patch with an underscore in a variable name, we'll know you haven't read this page!

5. Example patch

Here is an example of a Mercurial patch with proper summary, description, coding style, and testing.

changeset:   21111:dda11e799529
branch:      stable
user:        FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
date:        Tue Mar 25 19:34:17 2014 +0900
files:       mercurial/hg.py tests/test-clone.t
description:
hg: use "os.path.join()" to join path components which may be empty (issue4203)

Changset 2d0ab571b822 rewriting "hg.copystore()" with vfs uses
'dstbase + "/lock"' instead of "os.path.join()", because target files
given from "store.copyfiles()" already uses "/" as path separator

But in the repository using revlog format 0, "dstbase" becomes empty
("data" directory is located under ".hg" directly), and 'dstbase +
"/lock"' is treated as "/lock": in almost all cases, write access to
"/lock" causes "permission denied".

This patch uses "os.path.join()" to join path components which may be
empty in "hg.copystore()".


diff -r c57c9cece645 -r dda11e799529 mercurial/hg.py
--- a/mercurial/hg.py   Mon Mar 24 21:27:40 2014 -0400
+++ b/mercurial/hg.py   Tue Mar 25 19:34:17 2014 +0900
@@ -213,8 +213,10 @@
                 dstvfs.mkdir(dstbase)
             if srcvfs.exists(f):
                 if f.endswith('data'):
+                    # 'dstbase' may be empty (e.g. revlog format 0)
+                    lockfile = os.path.join(dstbase, "lock")
                     # lock to avoid premature writing to the target
-                    destlock = lock.lock(dstvfs, dstbase + "/lock")
+                    destlock = lock.lock(dstvfs, lockfile)
                 hardlink, n = util.copyfiles(srcvfs.join(f), dstvfs.join(f),
                                              hardlink)
                 num += n
diff -r c57c9cece645 -r dda11e799529 tests/test-clone.t
--- a/tests/test-clone.t        Mon Mar 24 21:27:40 2014 -0400
+++ b/tests/test-clone.t        Tue Mar 25 19:34:17 2014 +0900
@@ -621,3 +621,17 @@
 #endif

   $ cd ..
+
+Test clone from the repository in (emulated) revlog format 0 (issue4203):
+
+  $ mkdir issue4203
+  $ mkdir -p src/.hg
+  $ echo foo > src/foo
+  $ hg -R src add src/foo
+  $ hg -R src commit -m '#0'
+  $ hg -R src log -q
+  0:e1bab28bca43
+  $ hg clone -U -q src dst
+  $ hg -R dst log -q
+  0:e1bab28bca43
+  $ cd ..

6. Changing C code

Changes involving the C code should be done so that the tests will pass across changesets even without recompiling. This allows "bisect" to be run without needing to run "make" each time, for example.

A recommended strategy for changing behavior in the C code is to add a new interface to the C code when changing behavior (do not change old interfaces). Then in the Python code, check for the existence of the new interface.

If you change tests that rely on changes to C extensions, make sure you also run the tests in "pure" Python mode (and vice versa). For example:

$ python run-tests.py --pure

7. Patch descriptions

It's important that you describe your patch. Patch descriptions should be in the following format:

opener: check hardlink count reporting (issue1866)

The Linux CIFS kernel driver (even in 2.6.36) suffers from a hardlink
count blindness bug (lstat() returning 1 in st_nlink when it is expected
to return >1), which causes repository corruption if Mercurial running
...

Patch descriptions should be aimed at helping the reviewer understand the issue you're addressing.

Try to use the form "When I tried to do X, I got result Y, but the result should be Z". This is better than "X does not work" which assumes a common understanding of what it means for X to work and leaves the reader to intuit what Y and Z might have been.

Try to answer the following, where appropriate:

8. Sending patches

/!\ Don't send your patch to the BugTracker - it can't be reviewed there, so it won't go anywhere!

/!\ Sending a patch implies granting permission to use it in our project under an appropriate license

There are multiple options for sending patches to Mercurial. In order of preference:

  1. Phabricator. See Phabricator for usage instructions.

  2. email (see below)
  3. pushgate

If you have already multiple series waiting for review, sending any more patches is not advised. Please, please, please read the Flow control section.

Because this is a community project and our developers are very busy, patches will sometimes fall through the cracks. If you've gotten no response to your patch after a few days, feel free to resend it. Find the patch submission on the mercurial-devel@mercurial-scm.org and reply to it, kindly requesting someone look at it. If it was submitted to Phabricator, you can leave a common on Phabricator.

8.1. Emailing patches

The best way to achieve well-formed patches is to use patchbomb extension which automates the process. Add something like the following to your .hgrc:

[extensions]
patchbomb=

[patchbomb]
confirm=True
intro=never

[email]
to = mercurial-devel@mercurial-scm.org
from = Ada Lovelace <adalovelace@gmail.com>
method = smtp

[smtp]
host = smtp.gmail.com
port = 587
tls = starttls
username = adalovelace@gmail.com

Run hg help config.smtp for more SMTP configuration options and hg help email and hg help -e patchbomb for more info on the email command and the extension that provides it.

Then run the following to do a dryrun test:

$ hg email --test <change1> <change2> ...

Notable options:

9. Flow control

Please try not to swamp the list with patches. We have many contributors and few reviewers and we'd like to be sure everyone's patches get looked at.

Please also make sure the state of your patches in Patchwork is updated. This system is strictly secondary to the mailing list, but is helpful to other reviewers for finding patches that still need review.

Other advices:

10. Etiquette and advice

11. See also


CategoryDeveloper

ContributingChanges (last edited 2018-02-07 17:33:34 by GregorySzorc)