Patchwork [v3] hg: allow usage of XDG_CONFIG_HOME and $HOME/.config/hg/hgrc

login
register
mail settings
Submitter David Demelier
Date Feb. 10, 2017, 8:19 a.m.
Message ID <f0e8567b-26fd-81d2-9b02-7d6f68fbbd05@gmail.com>
Download mbox | patch
Permalink /patch/18391/
State Changes Requested
Headers show

Comments

David Demelier - Feb. 10, 2017, 8:19 a.m.
Le 09/02/2017 à 17:22, Jun Wu a écrit :
> The XDG specification says:
>
>   All paths set in these environment variables must be absolute. If an
>   implementation encounters a relative path in any of these variables it
>   should consider the path invalid and ignore it.
>
> So it needs to be checked before use.

Yes, I forgot that detail, thanks!

>
>> +        path = os.path.join(home, 'hg/hgrc')
>> +        if os.path.isfile(path):
>> +            return [path]
>> +        else:
>> +            return [os.path.expanduser('~/.hgrc')]
>
> This introduces a race condition:
>
>   - hg: userrcpath() returns ~/.hgrc, but not $XDG_CONFIG_HOME/hg/hgrc
>   - outside: somebody renames ~/.hgrc to $XDG_CONFIG_HOME/hg/hgrc
>   - hg: user config file will be missed when actually reading the files
>

A bit paranoia but real.

Is the following applicable for a v4?

I've updated the code so that hg config --edit still create a ~/.hgrc 
file first if no file is found.

Using XDG_CONFIG_HOME as first choice would require creating the 
directories before the file.

I've updated the test to specify an absolute path as XDG_CONFIG_HOME as 
well.

# HG changeset patch
# User David Demelier <demelier.david@gmail.com>
# Date 1486485215 -3600
#      Tue Feb 07 17:33:35 2017 +0100
# Node ID 438873a0f0b7b7871ae1fb950ec9924f52871878
# Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
hg: allow usage of XDG_CONFIG_HOME/hg/hgrc

Modern applications must use the following paths to store configuration 
files:

   - $XDG_CONFIG_HOME/hg/hgrc

For backward compatibility, ~/.hgrc is still tested first.

See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
Raffaele Salmaso - Feb. 10, 2017, 9:08 a.m.
Hi,

On Fri, Feb 10, 2017 at 9:19 AM, David Demelier <demelier.david@gmail.com>
wrote:
>
> I've updated the code so that hg config --edit still create a ~/.hgrc file
> first if no file is found.
>
Why?
If you support this specs, I expect it does the right thing by default (or
at least ask, if any)

A lot of things can go wrong even for $HOME/.hgrc, so I don't think it is a
bigger problem creating a directory first

> Using XDG_CONFIG_HOME as first choice would require creating the
> directories before the file.
>
Augie Fackler - Feb. 10, 2017, 9:03 p.m.
On Fri, Feb 10, 2017 at 09:19:14AM +0100, David Demelier wrote:
> Le 09/02/2017 à 17:22, Jun Wu a écrit :
>> The XDG specification says:
>> 
>> All paths set in these environment variables must be absolute. If an
>> implementation encounters a relative path in any of these variables it
>> should consider the path invalid and ignore it.
>> 
>> So it needs to be checked before use.
> 
> Yes, I forgot that detail, thanks!
> 
>> 
>>> +        path = os.path.join(home, 'hg/hgrc')
>>> +        if os.path.isfile(path):
>>> +            return [path]
>>> +        else:
>>> +            return [os.path.expanduser('~/.hgrc')]
>> 
>> This introduces a race condition:
>> 
>> - hg: userrcpath() returns ~/.hgrc, but not $XDG_CONFIG_HOME/hg/hgrc
>> - outside: somebody renames ~/.hgrc to $XDG_CONFIG_HOME/hg/hgrc
>> - hg: user config file will be missed when actually reading the files
>> 
> 
> A bit paranoia but real.
> 
> Is the following applicable for a v4?
> 
> I've updated the code so that hg config --edit still create a ~/.hgrc file
> first if no file is found.
> 
> Using XDG_CONFIG_HOME as first choice would require creating the directories
> before the file.
> 
> I've updated the test to specify an absolute path as XDG_CONFIG_HOME as
> well.
> 
> # HG changeset patch
> # User David Demelier <demelier.david@gmail.com>
> # Date 1486485215 -3600
> #      Tue Feb 07 17:33:35 2017 +0100
> # Node ID 438873a0f0b7b7871ae1fb950ec9924f52871878
> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
> hg: allow usage of XDG_CONFIG_HOME/hg/hgrc
> 
> Modern applications must use the following paths to store configuration
> files:
> 
>  - $XDG_CONFIG_HOME/hg/hgrc
> 
> For backward compatibility, ~/.hgrc is still tested first.
> 
> See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
> 
> diff -r 1f51b4658f21 -r 438873a0f0b7 mercurial/help/config.txt
> --- a/mercurial/help/config.txt Thu Feb 02 14:19:48 2017 +0100
> +++ b/mercurial/help/config.txt Tue Feb 07 17:33:35 2017 +0100
> @@ -56,6 +56,7 @@
> 
>   - ``<repo>/.hg/hgrc`` (per-repository)
>   - ``$HOME/.hgrc`` (per-user)
> +  - ``${XDG_CONFIG_HOME:-$HOME/.config}/hg/hgrc`` (per-user)

The only issue I see offhand is that this doesn't address Kevin's
(righteous, IMO) concern around having people parse shell variable
expansion. It's worth more prose to make it clear.

>   - ``<install-root>/etc/mercurial/hgrc`` (per-installation)
>   - ``<install-root>/etc/mercurial/hgrc.d/*.rc`` (per-installation)
>   - ``/etc/mercurial/hgrc`` (per-system)
> diff -r 1f51b4658f21 -r 438873a0f0b7 mercurial/scmposix.py
> --- a/mercurial/scmposix.py     Thu Feb 02 14:19:48 2017 +0100
> +++ b/mercurial/scmposix.py     Tue Feb 07 17:33:35 2017 +0100
> @@ -40,8 +40,15 @@
> def userrcpath():
>     if pycompat.sysplatform == 'plan9':
>         return [encoding.environ['home'] + '/lib/hgrc']
> +    elif pycompat.sysplatform == 'darwin':
> +        return [os.path.expanduser('~/.hgrc')]
>     else:
> -        return [os.path.expanduser('~/.hgrc')]
> +        confighome = encoding.environ.get('XDG_CONFIG_HOME')
> +        if confighome is None or not os.path.isabs(confighome):
> +            confighome = os.path.expanduser('~/.config')
> +
> +        return [os.path.expanduser('~/.hgrc'),
> +                os.path.join(confighome, 'hg', 'hgrc')]
> 
> def termsize(ui):
>     try:
> diff -r 1f51b4658f21 -r 438873a0f0b7 tests/test-xdg.t
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/test-xdg.t  Tue Feb 07 17:33:35 2017 +0100
> @@ -0,0 +1,11 @@
> +#if no-windows no-osx
> +
> +  $ mkdir -p xdgconf/hg
> +  $ echo '[ui]' > xdgconf/hg/hgrc
> +  $ echo 'username = foobar' >> xdgconf/hg/hgrc
> +  $ XDG_CONFIG_HOME=$PWD/xdgconf; export XDG_CONFIG_HOME
> +  $ unset HGRCPATH
> +  $ hg config ui.username
> +  foobar
> +
> +#endif
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
David Demelier - Feb. 13, 2017, 2:43 p.m.
Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
> Hi,
>
> On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
> <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
>
>     I've updated the code so that hg config --edit still create a
>     ~/.hgrc file first if no file is found.
>
> Why?
> If you support this specs, I expect it does the right thing by default
> (or at least ask, if any)
>
> A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
> is a bigger problem creating a directory first
>

I have no problem with this but I would like to know if everybody agree 
with that.
Jun Wu - Feb. 13, 2017, 5:37 p.m.
Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
> Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
> > Hi,
> >
> > On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
> > <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
> >
> >     I've updated the code so that hg config --edit still create a
> >     ~/.hgrc file first if no file is found.
> >
> > Why?
> > If you support this specs, I expect it does the right thing by default
> > (or at least ask, if any)
> >
> > A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
> > is a bigger problem creating a directory first
> >
> 
> I have no problem with this but I would like to know if everybody agree 
> with that.
> 

I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
~/.hgrc does not exist.
Augie Fackler - Feb. 13, 2017, 7:25 p.m.
On Mon, Feb 13, 2017 at 09:37:48AM -0800, Jun Wu wrote:
> Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
> > Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
> > > Hi,
> > >
> > > On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
> > > <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
> > >
> > >     I've updated the code so that hg config --edit still create a
> > >     ~/.hgrc file first if no file is found.
> > >
> > > Why?
> > > If you support this specs, I expect it does the right thing by default
> > > (or at least ask, if any)
> > >
> > > A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
> > > is a bigger problem creating a directory first
> > >
> >
> > I have no problem with this but I would like to know if everybody agree
> > with that.
> >
>
> I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
> ~/.hgrc does not exist.

I defer to people with opinions about linux on the desktop.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Feb. 14, 2017, 1:34 p.m.
On Mon, 13 Feb 2017 09:37:48 -0800, Jun Wu wrote:
> Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
> > Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
> > > Hi,
> > >
> > > On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
> > > <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
> > >
> > >     I've updated the code so that hg config --edit still create a
> > >     ~/.hgrc file first if no file is found.
> > >
> > > Why?
> > > If you support this specs, I expect it does the right thing by default
> > > (or at least ask, if any)
> > >
> > > A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
> > > is a bigger problem creating a directory first
> > >
> > 
> > I have no problem with this but I would like to know if everybody agree 
> > with that.
> > 
> 
> I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
> ~/.hgrc does not exist.

-1 for compatibility with old versions. Since old hg won't read .config/hg/hgrc,
creating it by default could confuse users.
David Demelier - Feb. 14, 2017, 2:53 p.m.
Le 14/02/2017 à 14:34, Yuya Nishihara a écrit :
> On Mon, 13 Feb 2017 09:37:48 -0800, Jun Wu wrote:
>> Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
>>> Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
>>>> Hi,
>>>>
>>>> On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
>>>> <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
>>>>
>>>>     I've updated the code so that hg config --edit still create a
>>>>     ~/.hgrc file first if no file is found.
>>>>
>>>> Why?
>>>> If you support this specs, I expect it does the right thing by default
>>>> (or at least ask, if any)
>>>>
>>>> A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
>>>> is a bigger problem creating a directory first
>>>>
>>>
>>> I have no problem with this but I would like to know if everybody agree
>>> with that.
>>>
>>
>> I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
>> ~/.hgrc does not exist.
>
> -1 for compatibility with old versions. Since old hg won't read .config/hg/hgrc,
> creating it by default could confuse users.
>

So if I understand correctly, that means:

1. the user installs a recent version of Mercurial
2. the user init a config using hg config --edit
3. the user downgrades Mercurial
Yuya Nishihara - Feb. 14, 2017, 3:07 p.m.
On Tue, 14 Feb 2017 15:53:24 +0100, David Demelier wrote:
> Le 14/02/2017 à 14:34, Yuya Nishihara a écrit :
> > On Mon, 13 Feb 2017 09:37:48 -0800, Jun Wu wrote:
> >> Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
> >>> Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
> >>>> Hi,
> >>>>
> >>>> On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
> >>>> <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
> >>>>
> >>>>     I've updated the code so that hg config --edit still create a
> >>>>     ~/.hgrc file first if no file is found.
> >>>>
> >>>> Why?
> >>>> If you support this specs, I expect it does the right thing by default
> >>>> (or at least ask, if any)
> >>>>
> >>>> A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
> >>>> is a bigger problem creating a directory first
> >>>>
> >>>
> >>> I have no problem with this but I would like to know if everybody agree
> >>> with that.
> >>>
> >>
> >> I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
> >> ~/.hgrc does not exist.
> >
> > -1 for compatibility with old versions. Since old hg won't read .config/hg/hgrc,
> > creating it by default could confuse users.
> >
> 
> So if I understand correctly, that means:
> 
> 1. the user installs a recent version of Mercurial
> 2. the user init a config using hg config --edit
> 3. the user downgrades Mercurial

Yes. Alternatively,

 3. the user switch to another host sharing HOME
David Demelier - Feb. 14, 2017, 3:17 p.m.
Le 14/02/2017 à 16:07, Yuya Nishihara a écrit :
>>> -1 for compatibility with old versions. Since old hg won't read .config/hg/hgrc,
>>> creating it by default could confuse users.
>>>
>>
>> So if I understand correctly, that means:
>>
>> 1. the user installs a recent version of Mercurial
>> 2. the user init a config using hg config --edit
>> 3. the user downgrades Mercurial
>
> Yes. Alternatively,
>
>  3. the user switch to another host sharing HOME
>

I've sent a v4 that still use ~/.hgrc as default when using hg config 
--edit :-)

Regards,
Sean Farley - Feb. 21, 2017, 11:54 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Tue, 14 Feb 2017 15:53:24 +0100, David Demelier wrote:
>> Le 14/02/2017 à 14:34, Yuya Nishihara a écrit :
>> > On Mon, 13 Feb 2017 09:37:48 -0800, Jun Wu wrote:
>> >> Excerpts from David Demelier's message of 2017-02-13 15:43:45 +0100:
>> >>> Le 10/02/2017 à 10:08, Raffaele Salmaso a écrit :
>> >>>> Hi,
>> >>>>
>> >>>> On Fri, Feb 10, 2017 at 9:19 AM, David Demelier
>> >>>> <demelier.david@gmail.com <mailto:demelier.david@gmail.com>> wrote:
>> >>>>
>> >>>>     I've updated the code so that hg config --edit still create a
>> >>>>     ~/.hgrc file first if no file is found.
>> >>>>
>> >>>> Why?
>> >>>> If you support this specs, I expect it does the right thing by default
>> >>>> (or at least ask, if any)
>> >>>>
>> >>>> A lot of things can go wrong even for $HOME/.hgrc, so I don't think it
>> >>>> is a bigger problem creating a directory first
>> >>>>
>> >>>
>> >>> I have no problem with this but I would like to know if everybody agree
>> >>> with that.
>> >>>
>> >>
>> >> I'm +1 on creating the directory and editing XDG_CONFIG_HOME/hg/hgrc if
>> >> ~/.hgrc does not exist.
>> >
>> > -1 for compatibility with old versions. Since old hg won't read .config/hg/hgrc,
>> > creating it by default could confuse users.
>> >
>> 
>> So if I understand correctly, that means:
>> 
>> 1. the user installs a recent version of Mercurial
>> 2. the user init a config using hg config --edit
>> 3. the user downgrades Mercurial
>
> Yes. Alternatively,
>
>  3. the user switch to another host sharing HOME

Or in my too frequent case:

3. the user activates a different virtualenv

Patch

diff -r 1f51b4658f21 -r 438873a0f0b7 mercurial/help/config.txt
--- a/mercurial/help/config.txt Thu Feb 02 14:19:48 2017 +0100
+++ b/mercurial/help/config.txt Tue Feb 07 17:33:35 2017 +0100
@@ -56,6 +56,7 @@ 

    - ``<repo>/.hg/hgrc`` (per-repository)
    - ``$HOME/.hgrc`` (per-user)
+  - ``${XDG_CONFIG_HOME:-$HOME/.config}/hg/hgrc`` (per-user)
    - ``<install-root>/etc/mercurial/hgrc`` (per-installation)
    - ``<install-root>/etc/mercurial/hgrc.d/*.rc`` (per-installation)
    - ``/etc/mercurial/hgrc`` (per-system)
diff -r 1f51b4658f21 -r 438873a0f0b7 mercurial/scmposix.py
--- a/mercurial/scmposix.py     Thu Feb 02 14:19:48 2017 +0100
+++ b/mercurial/scmposix.py     Tue Feb 07 17:33:35 2017 +0100
@@ -40,8 +40,15 @@ 
  def userrcpath():
      if pycompat.sysplatform == 'plan9':
          return [encoding.environ['home'] + '/lib/hgrc']
+    elif pycompat.sysplatform == 'darwin':
+        return [os.path.expanduser('~/.hgrc')]
      else:
-        return [os.path.expanduser('~/.hgrc')]
+        confighome = encoding.environ.get('XDG_CONFIG_HOME')
+        if confighome is None or not os.path.isabs(confighome):
+            confighome = os.path.expanduser('~/.config')
+
+        return [os.path.expanduser('~/.hgrc'),
+                os.path.join(confighome, 'hg', 'hgrc')]

  def termsize(ui):
      try:
diff -r 1f51b4658f21 -r 438873a0f0b7 tests/test-xdg.t
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-xdg.t  Tue Feb 07 17:33:35 2017 +0100
@@ -0,0 +1,11 @@ 
+#if no-windows no-osx
+
+  $ mkdir -p xdgconf/hg
+  $ echo '[ui]' > xdgconf/hg/hgrc
+  $ echo 'username = foobar' >> xdgconf/hg/hgrc
+  $ XDG_CONFIG_HOME=$PWD/xdgconf; export XDG_CONFIG_HOME
+  $ unset HGRCPATH
+  $ hg config ui.username
+  foobar
+
+#endif