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

[Gregory Szorc]

On Sat, Mar 28, 2015 at 6:12 PM, Matt Harbison <mharbison72 at gmail.com>
wrote:

> 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.)
>

I would think this metadata would apply to the whole section and order
wouldn't be matter. Mercurial's config parser would essentially do a
post-processing step to apply special metadata options.


>  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]
>

I don't know!


>
> 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?
>

I also don't know!


>
> 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?


Now you are starting to design a a custom config format. I would suggest
avoiding this if at all possible and to work around platform differences in
the core code or extension.

Just my $0.02.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150330/8e40ac64/attachment.html>