1047592 - mercurial-setup cannot read config with %include

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Patrick Cloke [:clokep]]

Attachment: My .hgrc

When I run mach mercurial-setup I get:
> Error importing existing Mercurial config!
> 
> Invalid line at line "60".
> Invalid line at line "63".

It seems to barf on the %include directive that hg supports: http://www.selenic.com/mercurial/hgrc.5.html#syntax

I could get around this pretty easily, but it's inconvenient.

Comment 1

[Gregory Szorc [:gps]]

Hmm. I'm not sure we can make this work properly since configobj doesn't support the % include syntax.

I think the best we can do is detect the syntax and error gracefully. Boo.

Comment 2

[Patrick Cloke [:clokep]]

I was thinking of this and the only somewhat sane thing I came up with was using Mercurials actual config parser (http://selenic.com/hg/file/tip/mercurial/config.py), but that would of course require you to be able to `import mercurial`, which I don't think people normally have installed.

Or...pre-parse the file, find any %include directives and strip those lines / follow them to include the files and pass a StringIO instance that contains the preparsed file. But that seems disgusting. (And would most likely break extremely hard when trying to modify the file.) :)

Feel free to WONTFIX, just wanted you guys to know about it! I simply commented out these two lines and then ran |mach mercurial-setup| and it worked fine!

Comment 3

[Patrick Cloke [:clokep]]

Attachment: sane-error.diff

I don't know how you would normally display an error in mach (this still gives the message about how "You should consider filing a bug for this issue." which doesn't seem right. :) But this at least tells you specifically what the issue is.

(Sorry if my Python foo isn't up to par, I don't normally do file I/O in Python.)

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 8467182 [details] [diff] [review]
sane-error.diff

Review of attachment 8467182 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/mercurial/hgsetup/config.py
@@ +40,5 @@
> +        # error saying this.
> +        with open(infile, 'r') as f:
> +            for line in f:
> +                if line.startswith('%include'):
> +                    raise Exception('%include directive is not supported by MercurialConfig')

The I/O is all sane.

While this file is the proper place to trap this particular parse failure, raising an uncaught exception is not good because it will cause mach to print a stack.

The easy solution is to move this hunk to wizard.py:189.

The slightly-more-involved solution is to invent a new exception type:

class HgIncludeException(Exception):
    pass

raise this exception from here, and catch it in wizard.py:195. From wizard.py, you can print() and |return 1| to signal failure.

Comment 5

[Patrick Cloke [:clokep]]

Attachment: sane-error.diff v2

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8467206 [details] [diff] [review]
sane-error.diff v2

Review of attachment 8467206 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect.

Comment 7

[Patrick Cloke [:clokep]]

Thanks! https://hg.mozilla.org/integration/mozilla-inbound/rev/41a1dc07bb48

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/41a1dc07bb48