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

login
register
mail settings
Submitter David Demelier
Date Feb. 8, 2017, 4:41 p.m.
Message ID <5ecf752f0a79482fe2ce.1486572110@localhost.localdomain>
Download mbox | patch
Permalink /patch/18351/
State Changes Requested
Headers show

Comments

David Demelier - Feb. 8, 2017, 4:41 p.m.
# HG changeset patch
# User David Demelier <demelier.david@gmail.com>
# Date 1486485215 -3600
#      Tue Feb 07 17:33:35 2017 +0100
# Node ID 5ecf752f0a79482fe2cecfa73c5a733dab29fb88
# Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
hg: allow usage of XDG_CONFIG_HOME and $HOME/.config/hg/hgrc

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

  - $XDG_CONFIG_HOME/hg/hgrc
  - $HOME/.config/hg/hgrc (if XDG_CONFIG_HOME is not set)

For convenience, these paths are now evaluated first and the old $HOME/.hgrc is
used as a fallback.

See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
Jun Wu - Feb. 9, 2017, 4:22 p.m.
Excerpts from David Demelier's message of 2017-02-08 17:41:50 +0100:
> # HG changeset patch
> # User David Demelier <demelier.david@gmail.com>
> # Date 1486485215 -3600
> #      Tue Feb 07 17:33:35 2017 +0100
> # Node ID 5ecf752f0a79482fe2cecfa73c5a733dab29fb88
> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
> hg: allow usage of XDG_CONFIG_HOME and $HOME/.config/hg/hgrc
> 
> Modern applications must use the following paths to store configuration files:
> 
>   - $XDG_CONFIG_HOME/hg/hgrc
>   - $HOME/.config/hg/hgrc (if XDG_CONFIG_HOME is not set)
> 
> For convenience, these paths are now evaluated first and the old $HOME/.hgrc is
> used as a fallback.
> 
> See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html 
> 
> diff -r 1f51b4658f21 -r 5ecf752f0a79 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
> @@ -55,6 +55,8 @@
>    On Unix, the following files are consulted:
>  
>    - ``<repo>/.hg/hgrc`` (per-repository)
> +  - ``$XDG_CONFIG_HOME/hg/hgrc`` (per-user)
> +  - ``$HOME/.config/hg/hgrc`` (per-user)

This does not match the code logic. All files in the list should be loaded,
while the code only picks one of the three "per-user" files.

I think it's better to always include ~/.hgrc, even if $XDG_CONFIG_HOME
exists (also check the comment below). So the text here could be:

  - ${XDG_CONFIG_HOME:-$HOME/.config}/hg/hgrc
  - $HOME/.hgrc

>    - ``$HOME/.hgrc`` (per-user)
>    - ``<install-root>/etc/mercurial/hgrc`` (per-installation)
>    - ``<install-root>/etc/mercurial/hgrc.d/*.rc`` (per-installation)
> diff -r 1f51b4658f21 -r 5ecf752f0a79 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,16 @@
>  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')]
> +        home = encoding.environ.get('XDG_CONFIG_HOME',
> +            os.path.expanduser('~/.config'))

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.

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

We can return both paths regardless of their existence, and avoid accessing
the filesystem in this function:

  confighome = encoding.environ.get('XDG_CONFIG_HOME')
  if 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 5ecf752f0a79 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=xdgconf; export XDG_CONFIG_HOME
> +  $ unset HGRCPATH
> +  $ hg config ui.username
> +  foobar
> +
> +#endif
Augie Fackler - Feb. 10, 2017, 2:50 a.m.
On Thu, Feb 09, 2017 at 08:22:02AM -0800, Jun Wu wrote:
> Excerpts from David Demelier's message of 2017-02-08 17:41:50 +0100:
>> # HG changeset patch
>> # User David Demelier <demelier.david@gmail.com>
>> # Date 1486485215 -3600
>> #      Tue Feb 07 17:33:35 2017 +0100
>> # Node ID 5ecf752f0a79482fe2cecfa73c5a733dab29fb88
>> # Parent  1f51b4658f21bbb797e922d155c1046eddccf91d
>> hg: allow usage of XDG_CONFIG_HOME and $HOME/.config/hg/hgrc
>> 
>> Modern applications must use the following paths to store configuration files:
>> 
>>  - $XDG_CONFIG_HOME/hg/hgrc
>>  - $HOME/.config/hg/hgrc (if XDG_CONFIG_HOME is not set)
>> 
>> For convenience, these paths are now evaluated first and the old $HOME/.hgrc is
>> used as a fallback.
>> 
>> See https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
>> 
>> diff -r 1f51b4658f21 -r 5ecf752f0a79 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
>> @@ -55,6 +55,8 @@
>>   On Unix, the following files are consulted:
>> 
>>   - ``<repo>/.hg/hgrc`` (per-repository)
>> +  - ``$XDG_CONFIG_HOME/hg/hgrc`` (per-user)
>> +  - ``$HOME/.config/hg/hgrc`` (per-user)
> 
> This does not match the code logic. All files in the list should be loaded,
> while the code only picks one of the three "per-user" files.
> 
> I think it's better to always include ~/.hgrc, even if $XDG_CONFIG_HOME
> exists (also check the comment below). So the text here could be:
> 
>  - ${XDG_CONFIG_HOME:-$HOME/.config}/hg/hgrc
>  - $HOME/.hgrc
> 
>>   - ``$HOME/.hgrc`` (per-user)
>>   - ``<install-root>/etc/mercurial/hgrc`` (per-installation)
>>   - ``<install-root>/etc/mercurial/hgrc.d/*.rc`` (per-installation)
>> diff -r 1f51b4658f21 -r 5ecf752f0a79 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,16 @@
>> 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')]
>> +        home = encoding.environ.get('XDG_CONFIG_HOME',
>> +            os.path.expanduser('~/.config'))
> 
> 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.
> 
>> +        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

Hm, that's nasty.

> 
> We can return both paths regardless of their existence, and avoid accessing
> the filesystem in this function:

We should make sure to define the load-order semantics clearly in that
case - I'd like it to be very clear which one takes precedence. Also,
'hg config --edit' should make sure to open the one that exists,
should only one of these paths exist.

> 
>  confighome = encoding.environ.get('XDG_CONFIG_HOME')
>  if 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 5ecf752f0a79 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=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

Patch

diff -r 1f51b4658f21 -r 5ecf752f0a79 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
@@ -55,6 +55,8 @@ 
   On Unix, the following files are consulted:
 
   - ``<repo>/.hg/hgrc`` (per-repository)
+  - ``$XDG_CONFIG_HOME/hg/hgrc`` (per-user)
+  - ``$HOME/.config/hg/hgrc`` (per-user)
   - ``$HOME/.hgrc`` (per-user)
   - ``<install-root>/etc/mercurial/hgrc`` (per-installation)
   - ``<install-root>/etc/mercurial/hgrc.d/*.rc`` (per-installation)
diff -r 1f51b4658f21 -r 5ecf752f0a79 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,16 @@ 
 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')]
+        home = encoding.environ.get('XDG_CONFIG_HOME',
+            os.path.expanduser('~/.config'))
+        path = os.path.join(home, 'hg/hgrc')
+        if os.path.isfile(path):
+            return [path]
+        else:
+            return [os.path.expanduser('~/.hgrc')]
 
 def termsize(ui):
     try:
diff -r 1f51b4658f21 -r 5ecf752f0a79 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=xdgconf; export XDG_CONFIG_HOME
+  $ unset HGRCPATH
+  $ hg config ui.username
+  foobar
+
+#endif