[PATCH 2 of 2 RFC] config: introduce platform specific sections

Matt Harbison mharbison72 at gmail.com
Sat Mar 28 20:12:55 CDT 2015


On Sat, 28 Mar 2015 15:19:50 -0400, Gregory Szorc  
<gregory.szorc at gmail.com> wrote:

> On Thu, Mar 26, 2015 at 7:35 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>
>> On Wed, 25 Mar 2015 21:15:27 -0400, Matt Harbison wrote:
>> > On Mon, 23 Mar 2015 16:02:16 -0400, Matt Mackall <mpm at selenic.com>
>> wrote:
>> > > On Fri, 2015-03-20 at 18:36 -0400, Matt Harbison wrote:
>> > >> # HG changeset patch
>> > >> # User Matt Harbison <matt_harbison at yahoo.com>
>> > >> # Date 1426888879 14400
>> > >> #      Fri Mar 20 18:01:19 2015 -0400
>> > >> # Node ID 636f51ff2cfc9bbd665f1b3bf198d780a2afffe8
>> > >> # Parent  6b3aff8a06f1cf7057a9bdcdc5e301dde0801828
>> > >> config: introduce platform specific sections
>> > >
>> > > Please take a peek at Greg's patches where he's proposing using
>> > > [foo.bar] for something else. I can't say I'm excited by either
>> > > proposal.
>> > >
>> >
>> > Yep, I saw he hadn't ruled that out yet and pinged him on Monday.
>> >
>> > In the meantime, do you have any alternate ideas for this?  I wasn't  
>> sure
>> > how your
>> >
>> >    new.style =
>> >    old.style.path =
>> >
>> > suggestion would apply to this, since that probably requires  
>> knowledge of
>> > what is possible in each section.  ([extensions] for example allows a
>> > leading 'hgext.', so it isn't a general split on '.' and compare
>> > algorithm.)
>>
>> As the magic is done at config.parse() layer, how about %if like a C
>> preprocessor?
>>
>>   [systest]
>>   %if $platform == 'win32'  # or %ifeq(...,...) ?
>>   key = windowsvalue
>>   %elif $platform == 'linux'
>>   key = linuxvalue
>>   %endif
>>
>> (I don't know if mpm likes it. ;)
>>
>>
> I'm also not a huge fan of adopting [X.platform] for platform-specific
> bits. That prevents us from using [X.Y] sections for anything else,  
> which I
> think is a bit short-sighted.

Yeah, I was concerned about that too.  I only ignored it because I  
couldn't think of something else that needed to apply at a section level.   
And without a concrete second example, I tend to over-engineer the future  
compatibility aspects.  But it's a fair point.

> I kinda like the preprocessor idea. Although, the more I interact with
> preprocessors, the more I learn that it is often best to avoid them. Once
> you introduce a preprocessing step, any tool that parses the file needs  
> to
> know about preprocessing. And since Mercurial doesn't have a built-in
> config editor, this would further limit the ability for 3rd party tools  
> to
> rewrite Mercurial's mostly-ini config files. (The "mercurial-setup" tool  
> I
> wrote for Mozilla is one such tool.)

That was also a concern, even with the syntax I introduced.  Would an  
editing tool be smart enough to skip over the regular [systest] section to  
edit the platform specific section, and fall back if not present?  I  
suppose overwriting both isn't horrible, just undesirable.

> I'm a much bigger fan of sticking to defined data formats as much as
> possible. i.e. "use ini."
>
> Here are some alternatives.
>
> Introduce special options within sections to denote properties. e.g. all
> UPPERCASE options could be reserved for metadata:
>
> [systest]
> PLATFORM = win32
> key = windowsvalue
>
> [systest]
> PLATFORM = linux
> key = linuxvalue
>
> [systest]
> PLATFORM = linux, osx
> key = posixvalue

I assume the metadata is controlling for the entire section, not just the  
next line?  Given that config.py reads a line and updates the dictionary  
immediately, I wonder if 3rd party editors can screw this up when they  
don't recognize the metadata, and put other variables in the section  
first.  (A 3rd party editor reordering keys seems like a bad idea, but it  
hasn't really mattered to this point, other than maybe comments.)

> Or, put special syntax in the section title after a space.
>
> [systest platform=win32]
> key = windowsvalue
>
> [systest platform=linux,osx]
> key = posixvalue
>
> Not as powerful as preprocessing, yes. But it preserves ini  
> compatibility,
> which I don't think should be under-estimated.

I like this.  It isn't far off from what I originally proposed (split on '  
' instead of '.', then split again on '='), so it should be fairly  
simple.  I think I'll try this, and use Yuya's idea as a backup.

Thinking toward the future, should we disallow a second space, so that  
multiple properties can be specified in the future?  e.g.

[systest platform=win32 arch=amd64]

Is there any reason to care about quoting values on the right side of '=',  
maybe to allow for spaces in the value?  Are there existing string parsing  
methods that can be reused for consistency?

The only other thing I can think of that might be useful here is to be  
able to match a string based on regex or something, like the OS version.   
(There are a couple of things that work differently in 10.10 vs 10.6 that  
I've noticed with thg.)  Obviously '=' won't work for that, so are there  
characters that we should reserve after the first space for future uses?


> (FWIW, the %include directive supported in config files today already
> breaks ini compatibility.)


More information about the Mercurial-devel mailing list