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

Wed Apr 1 21:23:28 CDT 2015

On Wed, 01 Apr 2015 20:10:53 -0400, Gregory Szorc  
<gregory.szorc at gmail.com> wrote:

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

That might help some.  Certainly some of the settings in that series I've  
enabled by default.

Basically, I took the hgrc files on 4 of the machines I use most often,  
took out the junk that has accumulated, and split the common stuff into a  
baseline file that everyone in the department should be using.  (Some  
progressive ui things, but also corporate friendly things- eol and  
keyring, and some specific revset aliases).  The rest is user specific  
stuff, some portable (ui.username, etc), some not so much (merge tool,  
extdiff tool, alias for it).

The thing is, I've got 2 OS X systems, 3 Windows systems, 2 Linux systems  
and 2 FreeBSD systems.  It would be nice to only have to maintain the one  
personal hgrc, instead of 9 separate platform specific copies.  Sync with  
visual diff tool.  I don't see a better way to do it.

Maybe it's not a huge problem in practice, so I'll let it go for now.  I  
thought I found a couple other people asking for it before I started, but  
only found one when I went back to look yesterday[0].  If it becomes a  
hassle, I'll bring it up again with better justification.

--Matt

[0] http://stackoverflow.com/questions/6301864/cross-platform-hgrc-solution

> 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