Page MenuHomePhabricator

config: read system hgrc in lexicographical order
ClosedPublic

Authored by martinvonz on Tue, Nov 3, 2:26 PM.

Details

Summary

This is similar to edbcf5b239f9 (config: read configs from directories
in lexicographical order, 2019-04-03). Apparently I forgot to sort the
system hgrc files there. That's fixed by this patch.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Tue, Nov 3, 2:26 PM
Alphare accepted this revision.Thu, Nov 5, 5:30 AM
mharbison72 requested changes to this revision.Sun, Nov 8, 11:51 PM
mharbison72 added a subscriber: mharbison72.

I like the idea of a stable order, but I think this is a BC. Specifically, hg help config.files says:

The names of these files depend on the system on which Mercurial is
installed. "*.rc" files from a single directory are read in alphabetical
order, later ones overriding earlier ones. Where multiple paths are given
below, settings from earlier paths override later ones.

 On Windows, the following files are consulted:

- "<repo>/.hg/hgrc" (per-repository)
- "%USERPROFILE%\.hgrc" (per-user)
- "%USERPROFILE%\Mercurial.ini" (per-user)
- "%HOME%\.hgrc" (per-user)
- "%HOME%\Mercurial.ini" (per-user)
- "HKEY_LOCAL_MACHINE\SOFTWARE\Mercurial" (per-system)
- "<install-dir>\hgrc.d\*.rc" (per-installation)
- "<install-dir>\Mercurial.ini" (per-installation)
- "%PROGRAMDATA%\Mercurial\hgrc" (per-system)
- "%PROGRAMDATA%\Mercurial\Mercurial.ini" (per-system)
- "%PROGRAMDATA%\Mercurial\hgrc.d\*.rc" (per-system)
- "<internal>/*.rc" (defaults)

I read that as %USERPROFILE%\.hgrc overrides %USERPROFILE%\Mercurial.ini, but .hgrc will sort first.

There's also a bit of ordering of the list in mercurial.scmwindows.systemrcpath(). So maybe the sorting needs to happen inside the platform functions, and only when scanning a directory?

This revision now requires changes to proceed.Sun, Nov 8, 11:51 PM

I like the idea of a stable order, but I think this is a BC. Specifically, hg help config.files says:

The names of these files depend on the system on which Mercurial is
installed. "*.rc" files from a single directory are read in alphabetical
order, later ones overriding earlier ones. Where multiple paths are given
below, settings from earlier paths override later ones.
 On Windows, the following files are consulted:
- "<repo>/.hg/hgrc" (per-repository)
- "%USERPROFILE%\.hgrc" (per-user)
- "%USERPROFILE%\Mercurial.ini" (per-user)
- "%HOME%\.hgrc" (per-user)
- "%HOME%\Mercurial.ini" (per-user)
- "HKEY_LOCAL_MACHINE\SOFTWARE\Mercurial" (per-system)
- "<install-dir>\hgrc.d\*.rc" (per-installation)
- "<install-dir>\Mercurial.ini" (per-installation)
- "%PROGRAMDATA%\Mercurial\hgrc" (per-system)
- "%PROGRAMDATA%\Mercurial\Mercurial.ini" (per-system)
- "%PROGRAMDATA%\Mercurial\hgrc.d\*.rc" (per-system)
- "<internal>/*.rc" (defaults)

I read that as %USERPROFILE%\.hgrc overrides %USERPROFILE%\Mercurial.ini, but .hgrc will sort first.
There's also a bit of ordering of the list in mercurial.scmwindows.systemrcpath(). So maybe the sorting needs to happen inside the platform functions, and only when scanning a directory?

Ohh, good catch! I'll move the sorting into the platform functions as you suggested. Thanks!

martinvonz edited the summary of this revision. (Show Details)Mon, Nov 9, 11:41 AM
martinvonz updated this revision to Diff 23441.
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

Queued, thanks.

mercurial/scmwindows.py
34

Not related to this patch, but I'm surprised that we don't care to ignore directories here (and in posix), and just key off of the name. IDK what happens when a directory instead of a file is tossed into the config list. rcutil._expandrcpath() similarly doesn't check.

martinvonz added inline comments.Mon, Nov 9, 3:21 PM
mercurial/scmwindows.py
34

Based on some manual testing I just did, it simply ignores paths that are directories. I haven't checked where in the code that happens.