mercurial-setup cannot read config with %include

RESOLVED FIXED in mozilla34

Status

enhancement
RESOLVED FIXED
5 years ago
Last year

People

(Reporter: clokep, Assigned: clokep)

Tracking

unspecified
mozilla34
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Posted file 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.
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.
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!
Posted patch sane-error.diff (obsolete) — Splinter Review
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.)
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #8467182 - Flags: review?(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.
Attachment #8467182 - Flags: review?(gps) → feedback+
Attachment #8467182 - Attachment is obsolete: true
Attachment #8467206 - Flags: review?(gps)
Comment on attachment 8467206 [details] [diff] [review]
sane-error.diff v2

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

Perfect.
Attachment #8467206 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/41a1dc07bb48
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Blocks: 1049893
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.