This is an archive of the discontinued Mercurial Phabricator instance.

config: read system hgrc in lexicographical order
ClosedPublic

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

Download Raw Diff

Details

Reviewers

Alphare
mharbison72

Group Reviewers

hg-reviewers

Commits

rHG9ac96b9fa76e: config: read system hgrc in lexicographical order

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

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

martinvonz created this revision.Nov 3 2020, 2:26 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 3 2020, 2:26 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

Alphare accepted this revision.Nov 5 2020, 5:30 AM

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.Nov 8 2020, 11:51 PM

In D9269#139641, @mharbison72 wrote:
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)Nov 9 2020, 11:41 AM

martinvonz updated this revision to Diff 23441.

martinvonz added a commit: rHG9ac96b9fa76e: config: read system hgrc in lexicographical order.Nov 9 2020, 2:10 PM

This revision was not accepted when it landed; it landed in state Needs Review.

Closed by commit rHG9ac96b9fa76e: config: read system hgrc in lexicographical order (authored by martinvonz). · Explain Why

This revision was automatically updated to reflect the committed changes.

Queued, thanks.

mercurial/scmwindows.py
34 ↗	(On Diff #23441)	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.Nov 9 2020, 3:21 PM

mercurial/scmwindows.py
34 ↗	(On Diff #23441)	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.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/rcutil.py (2 lines)
M			tests/test-config-env.py (18 lines)
M			tests/test-config-env.py.out (8 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	23410		Nov 3 2020, 2:26 PM	★	★
Diff 2	23441		Nov 9 2020, 11:41 AM	★	★
Diff 3	23443	rHG9ac96b9fa76eb6a92e9c70a6fb14b0bc3041580a	Nov 3 2020, 12:56 PM	★	★

Commit	Parents	Author	Summary	Date
82837101727f	b6f4a1df91bd	Martin von Zweigbergk		Nov 3 2020, 12:56 PM

Diff 23410

mercurial/rcutil.py

	for p in encoding.environ[b'HGRCPATH'].split(pycompat.ospathsep):			for p in encoding.environ[b'HGRCPATH'].split(pycompat.ospathsep):
	if not p:			if not p:
	continue			continue
	_rccomponents.extend((b'path', p) for p in _expandrcpath(p))			_rccomponents.extend((b'path', p) for p in _expandrcpath(p))
	else:			else:
	_rccomponents = [(b'resource', r) for r in default_rc_resources()]			_rccomponents = [(b'resource', r) for r in default_rc_resources()]

	normpaths = lambda paths: [			normpaths = lambda paths: [
	(b'path', os.path.normpath(p)) for p in paths			(b'path', os.path.normpath(p)) for p in sorted(paths)
	]			]
	_rccomponents.extend(normpaths(systemrcpath()))			_rccomponents.extend(normpaths(systemrcpath()))
	_rccomponents.append(envrc)			_rccomponents.append(envrc)
	_rccomponents.extend(normpaths(userrcpath()))			_rccomponents.extend(normpaths(userrcpath()))
	return _rccomponents			return _rccomponents


	def defaultpagerenv():			def defaultpagerenv():

tests/test-config-env.py


	testtmp = encoding.environ[b'TESTTMP']			testtmp = encoding.environ[b'TESTTMP']

	# prepare hgrc files			# prepare hgrc files
	def join(name):			def join(name):
	return os.path.join(testtmp, name)			return os.path.join(testtmp, name)


	with open(join(b'sysrc'), 'wb') as f:			with open(join(b'sysrc1'), 'wb') as f:
	f.write(b'[ui]\neditor=e0\n[pager]\npager=p0\n')			f.write(b'[ui]\neditor=e0\n[pager]\npager=p1\n')

	with open(join(b'userrc'), 'wb') as f:			with open(join(b'sysrc2'), 'wb') as f:
				f.write(b'[ui]\neditor=e0\n[pager]\npager=p2\n')

				with open(join(b'userrc1'), 'wb') as f:
	f.write(b'[ui]\neditor=e1')			f.write(b'[ui]\neditor=e1')

				with open(join(b'userrc2'), 'wb') as f:
				f.write(b'[ui]\neditor=e2')

	# replace rcpath functions so they point to the files above			# replace rcpath functions so they point to the files above
	def systemrcpath():			def systemrcpath():
	return [join(b'sysrc')]			return [join(b'sysrc2'), join(b'sysrc1')]


	def userrcpath():			def userrcpath():
	return [join(b'userrc')]			return [join(b'userrc2'), join(b'userrc1')]


	extensions.wrapfunction(rcutil, 'default_rc_resources', lambda orig: [])			extensions.wrapfunction(rcutil, 'default_rc_resources', lambda orig: [])

	rcutil.systemrcpath = systemrcpath			rcutil.systemrcpath = systemrcpath
	rcutil.userrcpath = userrcpath			rcutil.userrcpath = userrcpath

	# utility to print configs			# utility to print configs
	def printconfigs(env):			def printconfigs(env):
	encoding.environ = env			encoding.environ = env
	rcutil._rccomponents = None # reset cache			rcutil._rccomponents = None # reset cache
	ui = uimod.ui.load()			ui = uimod.ui.load()
	for section, name, value in ui.walkconfig():			for section, name, value in ui.walkconfig():
	source = ui.configsource(section, name)			source = ui.configsource(section, name)
	procutil.stdout.write(			procutil.stdout.write(
	b'%s.%s=%s # %s\n' % (section, name, value, util.pconvert(source))			b'%s.%s=%s # %s\n' % (section, name, value, util.pconvert(source))
	)			)
	procutil.stdout.write(b'\n')			procutil.stdout.write(b'\n')


	# environment variable overrides			# environment variable overrides
	printconfigs({})			printconfigs({})
	printconfigs({b'EDITOR': b'e2', b'PAGER': b'p2'})			printconfigs({b'EDITOR': b'e3', b'PAGER': b'p3'})

tests/test-config-env.py.out

	pager.pager=p0 # $TESTTMP/sysrc:4			pager.pager=p2 # $TESTTMP/sysrc2:4
	ui.editor=e1 # $TESTTMP/userrc:2			ui.editor=e2 # $TESTTMP/userrc2:2

	pager.pager=p2 # $PAGER			pager.pager=p3 # $PAGER
	ui.editor=e1 # $TESTTMP/userrc:2			ui.editor=e2 # $TESTTMP/userrc2:2