Patch for monotone.py

[Patrick Mézard]

Mike Rogers a écrit :
> The CS department at TTU has been using Monotone for a couple of years now as our revision control system.  However, due to its lack of popularity (and thus lack of tool support), we are contemplating moving to Bazaar, Git, or Mercurial.
> 
> To experiment with Mercurial, I needed to import our Monotone repositories.  However, when using "hg convert", the extension would seemingly "lose" Monotone certs.  After inspecting monotone.py in the extensions directory, I found that the the Python script was splitting the certs at double newlines.  Our repositories have comments in the changelogs (which are just certs) with double newlines.
> 
> I have fixed the script so that it no longer splits the Monotone certs string.  Now it uses a regular expression to find the first key-value pair, iterating though the whole string of certs until no more key-value pairs are found.
> 
> Here is the patch - I hope its useful.  Thanks.

Patches are always welcome but I have two major problems with your current contribution:
- It breaks the test suite. To reproduce: apply it then cd to tests/ and "python run-tests.py test-convert-mtn".
- I don't know how to reproduce your issue which makes it harder for me to evaluate its fix. I have tried to update the test script to register a commit message with 2 newlines:

diff --git a/tests/test-convert-mtn b/tests/test-convert-mtn
--- a/tests/test-convert-mtn
+++ b/tests/test-convert-mtn
@@ -54,7 +54,14 @@
 mtn add e
 mtn drop dir/b
 mtn mv bin bin2
-mtn ci -m 'update2 "with" quotes'
+cat > changelog <<EOF
+update2 "with" quotes and
+
+
+double new lines to confuse the cert parser.
+EOF
+mtn ci --message-file changelog
+rm changelog
 # Test directory move
 mtn mv dir dir2
 mtn ci -m movedir
@@ -85,5 +92,7 @@
 echo % contents
 cat dir2/a
 test -d dir2/dir && echo 'removed dir2/dir is still there!'
+echo % complicate log messages
+hg -v log -r 2
 exit 0


And the related output (from run-tests.py) looks like:


+% complicate log messages
+changeset:   2:be8a3878f86a
+branch:      com.selenic.test
+user:        test at selenic.com
+date:        Sat Nov 29 11:58:17 2008 +0000
+files:       bin bin2 dir/b e
+description:
+update2 "with" quotes and
+
+
+double new lines to confuse the cert parser.
+
+


So the double newline is clearly preserved. Perhaps the issue appears when a commit has more than one cert attached to it but being pretty ignorant about monotone I don't know how to build an sample. It would be great if you could edit "tests/test-convert-mtn" to build a revision exhibiting the issue, so I can see what the patch really changes.

Also, I have a couple of remarks about coding conventions, please find annotations below.

More details in:

	http://www.selenic.com/mercurial/wiki/index.cgi/BasicCodingStyle

> # HG changeset patch
> # User mrogers at tntech.edu
> # Date 1227920754 21600
> # Node ID b91ac08390cca41067f356f84e5cef2842a15d8e
> # Parent  b965605dfb2ed188a54d4aca18ac3ae5b0726953
> Changes to monotone.py.  Now "hg convert" handles monotone changelog certs more robustly.
> 
> diff -r b965605dfb2e -r b91ac08390cc hgext/convert/monotone.py
> --- a/hgext/convert/monotone.py	Sat Nov 15 15:51:26 2008 +0100
> +++ b/hgext/convert/monotone.py	Fri Nov 28 19:05:54 2008 -0600
> @@ -16,7 +16,7 @@
>  
>          # regular expressions for parsing monotone output
>          space    = r'\s*'
> -        name     = r'\s+"((?:\\"|[^"])*)"\s*'
> +        name     = r'\s+"((?:[^"]|\\")*)"\s*'
>          value    = name
>          revision = r'\s+\[(\w+)\]\s*'
>          lines    = r'(?:.|\n)+'
> @@ -29,6 +29,7 @@
>          self.delete_re   = re.compile(space + "delete" + name)
>          self.tag_re      = re.compile(space + "tag" + name + "revision" + revision)
>          self.cert_re     = re.compile(lines + space + "name" + name + "value" + value)
> +	self.nvpair      = re.compile("name" + name + "value" + value)

Please use spaces instead of tabs, 4 spaces per tab.
Since nvpair is a regular expression, let's preserve the naming convention and call it nvpair_re.

>  
>          attr = space + "file" + lines + space + "attr" + space
>          self.attr_execute_re = re.compile(attr  + '"mtn:execute"' + space + '"true"')
> @@ -98,14 +99,16 @@
>      def mtngetcerts(self, rev):
>          certs = {"author":"<missing>", "date":"<missing>",
>              "changelog":"<missing>", "branch":"<missing>"}
> -        cert_list = self.mtnrun("certs", rev).split('\n\n      key "')
> -        for e in cert_list:
> -            m = self.cert_re.match(e)
> -            if m:
> -                name, value = m.groups()
> -                value = value.replace(r'\"', '"')
> -                value = value.replace(r'\\', '\\')
> -                certs[name] = value
> +        all_certs = self.mtnrun("certs", rev)
> +
> +	haveMore = True

Variable names should be in lower-case (no underscores).

RegexObject.finditer() may be enough and simpler to use.

> +        while haveMore:
> +            m = self.nvpair.search(all_certs)
> +            if m == None:
> +                haveMore = False
> +            else:
> +                certs[m.group(1)] = m.group(2)
> +                all_certs = all_certs[m.end()+1:]
>          return certs
>  
>      def mtnrenamefiles(self, files, fromdir, todir):

--
Patrick Mézard