This is an archive of the discontinued Mercurial Phabricator instance.

More serious question: Maybe we should stop passing around tuple of dictionnary and return an object holding all that data with a better API. What do you think ?

This revision now requires review to proceed.Jul 20 2020, 3:44 PM

I resigned (and commented) on the wrong revision.

This revision now requires changes to proceed.Jul 20 2020, 4:59 PM

pulkit added inline comments.Jul 21 2020, 9:07 AM

mercurial/localrepo.py
535–549	This code ideally should not be used outside this function. Hence a closure.

That is a bad reason to use a closure. Make the function private and move it at the module level.
(Mercurial used to be full of this strange closure and this was a pain)

pulkit updated this revision to Diff 22000.Jul 21 2020, 9:42 AM

marmoute requested changes to this revision.Jul 21 2020, 9:46 AM

marmoute added inline comments.

mercurial/localrepo.py
535–549	Still a closure, please make it not a closure.

This revision now requires changes to proceed.Jul 21 2020, 9:46 AM

pulkit updated this revision to Diff 22069.Jul 23 2020, 10:52 AM

pulkit added inline comments.Jul 23 2020, 10:56 AM

mercurial/localrepo.py
535–549	Fixed.

pulkit updated this revision to Diff 22172.Aug 1 2020, 10:36 AM

pulkit abandoned this revision.Aug 7 2020, 2:51 PM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/localrepo.py (24 lines)

Commit	Parents	Author	Summary	Date
7784ae4e7413	465d93c4dc00	Pulkit Goyal		Jul 17 2020, 5:37 AM

Status	Author	Revision
Closed	pulkit	D8660 upgrade: support running upgrade if repository has share-safe requirement
Closed	pulkit	D8659 config: add `--shared` flag to edit config file of shared source
Closed	pulkit	D8658 commands: use any() instead of `if a or b or c`
Closed	pulkit	D8656 localrepo: load the share source .hg/hgrc also in share-safe mode (API)
Abandoned	pulkit	D8655 localrepo: unify share checking and related code
Abandoned	pulkit	D8764 localrepo: deduplicate code around reading requires file
Closed	pulkit	D8633 share: introduce config option to store requires in .hg/store
Closed	pulkit	D8913 scmutil: introduce filterrequirements() to split reqs into wc and store ones
Closed	pulkit	D8952 remotefilelog: acquire lock before writing requirements on clone

Diff 22172

mercurial/localrepo.py

	# functions defined in loaded extensions are called.			# functions defined in loaded extensions are called.
	#			#
	# The function receives a set of requirement strings that the repository			# The function receives a set of requirement strings that the repository
	# is capable of opening. Functions will typically add elements to the			# is capable of opening. Functions will typically add elements to the
	# set to reflect that the extension knows how to handle that requirements.			# set to reflect that the extension knows how to handle that requirements.
	featuresetupfuncs = set()			featuresetupfuncs = set()


				def _readrequires(vfs):
				try:
				requirements = set(vfs.read(b'requires').splitlines())
				except IOError as e:
				if e.errno != errno.ENOENT:
				raise
				requirements = set()
				return requirements


	def makelocalrepository(baseui, path, intents=None):			def makelocalrepository(baseui, path, intents=None):
	"""Create a local repository object.			"""Create a local repository object.

	Given arguments needed to construct a local repository, this function			Given arguments needed to construct a local repository, this function
	performs various early repository loading functionality (such as			performs various early repository loading functionality (such as
	reading the ``.hg/requires`` and ``.hg/hgrc`` files), validates that			reading the ``.hg/requires`` and ``.hg/hgrc`` files), validates that
	the repository can be opened, derives a type suitable for representing			the repository can be opened, derives a type suitable for representing
	that repository, and returns an instance of it.			that repository, and returns an instance of it.
	except ValueError as e:			except ValueError as e:
	# Can be raised on Python 3.8 when path is invalid.			# Can be raised on Python 3.8 when path is invalid.
	raise error.Abort(			raise error.Abort(
	_(b'invalid path %s: %s') % (path, pycompat.bytestr(e))			_(b'invalid path %s: %s') % (path, pycompat.bytestr(e))
	)			)

	raise error.RepoError(_(b'repository %s not found') % path)			raise error.RepoError(_(b'repository %s not found') % path)

	# .hg/requires file contains a newline-delimited list of			# .hg/requires file contains a newline-delimited list of
	# features/capabilities the opener (us) must have in order to use			# features/capabilities the opener (us) must have in order to use
	# the repository. This file was introduced in Mercurial 0.9.2,			# the repository. This file was introduced in Mercurial 0.9.2,
	# which means very old repositories may not have one. We assume			# which means very old repositories may not have one. We assume
	# a missing file translates to no requirements.			# a missing file translates to no requirements.
	try:			requirements = _readrequires(hgvfs)
	requirements = set(hgvfs.read(b'requires').splitlines())
	except IOError as e:
	if e.errno != errno.ENOENT:
	raise
	requirements = set()

	# if .hg/requires contains the sharesafe requirement, it means			# if .hg/requires contains the sharesafe requirement, it means
	# there exists a `.hg/store/requires` too and we should read it			# there exists a `.hg/store/requires` too and we should read it
	if SHARESAFE_REQUIREMENT in requirements:			if SHARESAFE_REQUIREMENT in requirements:
	if hgvfs.exists(b'sharedpath'):			if hgvfs.exists(b'sharedpath'):
	# This is a shared repo			# This is a shared repo
	sharedpath = hgvfs.read(b'sharedpath').rstrip(b'\n')			sharedpath = hgvfs.read(b'sharedpath').rstrip(b'\n')
	if b'relshared' in requirements:			if b'relshared' in requirements:
	sharedpath = hgvfs.join(sharedpath)			sharedpath = hgvfs.join(sharedpath)
				marmouteUnsubmitted Not Done This does not seems to need to be a closure. Can we make it a normal function? marmoute: This does not seems to need to be a closure. Can we make it a normal function?
				pulkitAuthorUnsubmitted Done This code ideally should not be used outside this function. Hence a closure. pulkit: This code ideally should not be used outside this function. Hence a closure.
				marmouteUnsubmitted Not Done Still a closure, please make it not a closure. marmoute: Still a closure, please make it not a closure.
				pulkitAuthorUnsubmitted Done Fixed. pulkit: Fixed.
	storevfs = vfsmod.vfs(vfsmod.vfs(sharedpath).join(b'store'))			storevfs = vfsmod.vfs(vfsmod.vfs(sharedpath).join(b'store'))
	else:			else:
	storevfs = vfsmod.vfs(hgvfs.join(b'store'), cacheaudited=True)			storevfs = vfsmod.vfs(hgvfs.join(b'store'), cacheaudited=True)
	try:			requirements \|= _readrequires(storevfs)
	store_requirements = set(storevfs.read(b'requires').splitlines())
	requirements \|= store_requirements
	except IOError as e:
	if e.errno != errno.ENOENT:
	raise

	# The .hg/hgrc file may load extensions or contain config options			# The .hg/hgrc file may load extensions or contain config options
	# that influence repository construction. Attempt to load it and			# that influence repository construction. Attempt to load it and
	# process any new extensions that it may have pulled in.			# process any new extensions that it may have pulled in.
	if loadhgrc(ui, wdirvfs, hgvfs, requirements):			if loadhgrc(ui, wdirvfs, hgvfs, requirements):
	afterhgrcload(ui, wdirvfs, hgvfs, requirements)			afterhgrcload(ui, wdirvfs, hgvfs, requirements)
	extensions.loadall(ui)			extensions.loadall(ui)
	extensions.populateui(ui)			extensions.populateui(ui)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	21953		Jul 17 2020, 10:20 AM	★	★
Diff 2	22000		Jul 21 2020, 9:42 AM	★	★
Diff 3	22069		Jul 23 2020, 10:52 AM	★	★
Diff 4	22172		Aug 1 2020, 10:36 AM	★	★

This is an archive of the discontinued Mercurial Phabricator instance.

localrepo: deduplicate code around reading requires fileAbandonedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 22172

mercurial/localrepo.py

localrepo: deduplicate code around reading requires file
AbandonedPublic

Revision Contents
Changeset List