Closed Bug 1047592 Opened 7 years ago Closed 7 years ago

mercurial-setup cannot read config with %include


(Firefox Build System :: Mach Core, enhancement)

Not set


(Not tracked)



(Reporter: clokep, Assigned: clokep)




(2 files, 1 obsolete file)

Attached 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:

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 (, 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!
Attached 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
Attachment #8467182 - Flags: review?(gps)
Comment on attachment 8467182 [details] [diff] [review]

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

::: tools/mercurial/hgsetup/
@@ +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

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

class HgIncludeException(Exception):

raise this exception from here, and catch it in From, 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]:

Attachment #8467206 - Flags: review?(gps) → review+
Closed: 7 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.