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

Thu Apr 2 00:10:53 UTC 2015

On Tue, Mar 31, 2015 at 6:42 AM, Matt Mackall <mpm at selenic.com> wrote:

> On Mon, 2015-03-30 at 22:12 -0700, Gregory Szorc wrote:
> > 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.
>
> I'm not excited by any of the options here. The best option I've seen so
> far is #if which is nice and open-ended (and comment-like), and I think
> it's equivalent to all the other alternatives from an automated editing
> standpoint[1].
>
> That said, I don't think the marginal utility of the feature is high
> enough to justify it. Matt's the first person to ever ask for it that
> I'm aware of.
>
> [1] Doomed to fail, just like editing already-very-complex existing
> format.

FWIW, we're using configobj [1] for doing hgrc parsing and rewriting. It
preserves comments and ordering. The only time I've seen it choke is when
someone uses %include.

Maybe "progressive ui" will solve a lot of problems. But until there is a
better story for helping users configure an optimal config (this includes
server-specific suggestions), I'll continue to insist that the config
format is "portable" so those of us that don't have the power to rewrite
hgrc files on clients can make the hg experience suck less using config
editing tools.

[1] https://pypi.python.org/pypi/configobj/5.0.6
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150401/49f6a732/attachment.html>