[PATCH 07 of 11 V2] ui: use scmutil.rccomponents to load configs (BC)

Ryan McElroy rm at fb.com
Wed Mar 22 12:28:42 EDT 2017


On 3/22/17 4:01 PM, Jun Wu wrote:
> Excerpts from Ryan McElroy's message of 2017-03-22 11:37:00 +0000:
>> On 3/22/17 7:50 AM, Jun Wu wrote:
>>> # HG changeset patch
>>> # User Jun Wu <quark at fb.com>
>>> # Date 1489462886 25200
>>> #      Mon Mar 13 20:41:26 2017 -0700
>>> # Node ID add83f47bf3a51edbd58aa0fb6e571d186bdae6e
>>> # Parent  df768455486ff51b4047fd3a8dfbef158516d2aa
>>> ui: use scmutil.rccomponents to load configs (BC)
>>>
>>> This is BC because system configs won't be able to override $EDITOR, $PAGER.
>>> The new behavior is arguably more rational.
>>>
>>> diff --git a/mercurial/ui.py b/mercurial/ui.py
>>> --- a/mercurial/ui.py
>>> +++ b/mercurial/ui.py
>>> @@ -212,6 +212,18 @@ class ui(object):
>>>            u = cls()
>>>            # we always trust global config files
>>> -        for f in scmutil.rcpath():
>>> -            u.readconfig(f, trust=True)
>>> +        for (t, f) in scmutil.rccomponents():
>>> +            if t == 'path':
>>> +                u.readconfig(f, trust=True)
>>> +            elif t == 'items':
>>> +                sections = set()
>>> +                for section, name, value, source in f:
>>> +                    # do not set ocfg
>>> +                    u._tcfg.set(section, name, value, source)
>>> +                    u._ucfg.set(section, name, value, source)
>>> +                    sections.add(section)
>>> +                for section in sections:
>>> +                    u.fixconfig(section=section)
>>> +            else:
>>> +                raise error.ProgrammingError('unexpected rccomponent: %s' % t)
>>>            return u
>> Seeing this makes me not like this "union type" (rccomponents) that
>> you've created. Can we do this differently for more cleanliness?
> The code here is temporary and will be rewritten once the immutable config
> becomes a thing.  I will add a comment but not rewrite the temporary code.
>
>> Consider instead of having rccomponents be a parsed config "source"
>> (either a file or environment for now). Then this patch simply combines
>> them in order. This gives us the nice property of reading each config
>> file only once, which was a patch series that dsop sent but had BC
>> issues, but doing it this way means there are no BC issues. Of course,
>> it also cleans up this code so we don't have the type check in there.
> If I understand what you suggest, I think it could have issues with
> "%unset".
>
> Although that may be solved, but the immutable config will be a thing, so
> I'm not interested in improving to-be-deprecated code.

I think unset could be accomplished with a "sentinel object" that we 
compare against to know if we should unset a value while applying 
partial configs.

However, since you have a plan to improve config overall and you have a 
good track record of follow-through, I'll officially drop my objections 
to this approach.

>> Essentially, envconfig() can be generalized into a "parsedconfig"
>> container, and then it's just about ordering for the final config from
>> there.
>>
>> I think this would make the code cleaner -- thoughts?
>>
>>>    
>>> diff --git a/tests/test-config-env.py b/tests/test-config-env.py
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/tests/test-config-env.py
>>> @@ -0,0 +1,48 @@
>>> +# Test the config layer generated by environment variables
>>> +
>>> +from __future__ import absolute_import, print_function
>>> +
>>> +import os
>>> +
>>> +from mercurial import (
>>> +    encoding,
>>> +    scmutil,
>>> +    ui as uimod,
>>> +)
>>> +
>>> +testtmp = encoding.environ['TESTTMP']
>>> +
>>> +# prepare hgrc files
>>> +def join(name):
>>> +    return os.path.join(testtmp, name)
>>> +
>>> +with open(join('sysrc'), 'w') as f:
>>> +    f.write('[ui]\neditor=e0\n[pager]\npager=p0\n')
>>> +
>>> +with open(join('userrc'), 'w') as f:
>>> +    f.write('[ui]\neditor=e1')
>>> +
>>> +# replace rcpath functions so they point to the files above
>>> +def systemrcpath():
>>> +    return [join('sysrc')]
>>> +
>>> +def userrcpath():
>>> +    return [join('userrc')]
>>> +
>>> +scmutil.systemrcpath = systemrcpath
>>> +scmutil.userrcpath = userrcpath
>>> +os.path.isdir = lambda x: False # hack: do not load default.d/*.rc
>>> +
>>> +# utility to print configs
>>> +def printconfigs(env):
>>> +    encoding.environ = env
>>> +    scmutil._rccomponents = None # reset cache
>>> +    ui = uimod.ui.load()
>>> +    for section, name, value in ui.walkconfig():
>>> +        source = ui.configsource(section, name)
>>> +        print('%s.%s=%s # %s' % (section, name, value, source))
>>> +    print('')
>>> +
>>> +# environment variable overrides
>>> +printconfigs({})
>>> +printconfigs({'EDITOR': 'e2', 'PAGER': 'p2'})
>>> diff --git a/tests/test-config-env.py.out b/tests/test-config-env.py.out
>>> new file mode 100644
>>> --- /dev/null
>>> +++ b/tests/test-config-env.py.out
>>> @@ -0,0 +1,6 @@
>>> +pager.pager=p0 # $TESTTMP/sysrc:4
>>> +ui.editor=e1 # $TESTTMP/userrc:2
>>> +
>>> +pager.pager=p2 # $PAGER
>>> +ui.editor=e1 # $TESTTMP/userrc:2
>>> +
>>> diff --git a/tests/test-config.t b/tests/test-config.t
>>> --- a/tests/test-config.t
>>> +++ b/tests/test-config.t
>>> @@ -165,2 +165,16 @@ edit failure
>>>      abort: edit failed: false exited with status 1
>>>      [255]
>>> +
>>> +config affected by environment variables
>>> +
>>> +  $ EDITOR=e1 VISUAL=e2 hg config --debug | grep 'ui\.editor'
>>> +  $VISUAL: ui.editor=e2
>>> +
>>> +  $ VISUAL=e2 hg config --debug --config ui.editor=e3 | grep 'ui\.editor'
>>> +  --config: ui.editor=e3
>>> +
>>> +  $ PAGER=p1 hg config --debug | grep 'pager\.pager'
>>> +  $PAGER: pager.pager=p1
>>> +
>>> +  $ PAGER=p1 hg config --debug --config pager.pager=p2 | grep 'pager\.pager'
>>> +  --config: pager.pager=p2
>>>



More information about the Mercurial-devel mailing list