This is an archive of the discontinued Mercurial Phabricator instance.

Differential D288

bundlerepo: move temp bundle creation to a separate function
ClosedPublic

Authored by durham on Aug 8 2017, 8:05 PM.

Download Raw Diff

Details

Reviewers

indygreg

Group Reviewers

hg-reviewers

Commits

rHG702a26fec3e2: bundlerepo: move temp bundle creation to a separate function

Summary

A future patch will refactor certain parts of bundlerepo initiatlization such
that we need to create temp bundles from another function. Let's move this to
another function to support that.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

durham created this revision.Aug 8 2017, 8:05 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 8 2017, 8:05 PM

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

durham added a child revision: D289: bundle2: seek part back during iteration.Aug 8 2017, 8:05 PM

LGTM

indygreg requested changes to this revision.Aug 14 2017, 9:40 PM

indygreg added a subscriber: indygreg.

indygreg added inline comments.

mercurial/bundlerepo.py
325–326	Please update this to reflect that it is no longer a closure.

This revision now requires changes to proceed.Aug 14 2017, 9:40 PM

martinvonz added a subscriber: martinvonz.Aug 23 2017, 1:47 PM

martinvonz added inline comments.

mercurial/bundlerepo.py
322	I think we usually use a "fn" prefix for function arguments. I think "readfn" would be clearer.

krbullock added a subscriber: krbullock.Aug 23 2017, 2:30 PM

krbullock added inline comments.

mercurial/bundlerepo.py
322	I agree, but as it is, this is simple code movement. Usually we like renames to be done in separate changes. Updating the docstring as @indygreg suggests is appropriate, though.

durham edited edge metadata.Aug 23 2017, 3:35 PM

durham updated this revision to Diff 1215.

I fixed the comment and renamed read to readfn. I know we usually do renames as separate from code moves, but this was a tiny rename (only one use).

Closed by commit rHG702a26fec3e2: bundlerepo: move temp bundle creation to a separate function (authored by durham). · Explain WhyAug 23 2017, 4:31 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/bundlerepo.py (44 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	658		Aug 8 2017, 8:05 PM	★	★
Diff 2	1215		Aug 23 2017, 3:35 PM	★	★
Diff 3	1227	rHG702a26fec3e2e85f05af3ebeaf0fc037027ad2b6	Aug 23 2017, 3:34 PM	★	★

Status	Author	Revision
Closed	durham	D290 bundlerepo: move bundle2 part handling out to a function
Closed	durham	D289 bundle2: seek part back during iteration
Closed	durham	D288 bundlerepo: move temp bundle creation to a separate function

Diff 658

mercurial/bundlerepo.py

	fname = chunkdata['filename']			fname = chunkdata['filename']
	bundlefilespos[fname] = bundle.tell()			bundlefilespos[fname] = bundle.tell()
	for chunk in iter(lambda: bundle.deltachunk(None), {}):			for chunk in iter(lambda: bundle.deltachunk(None), {}):
	pass			pass
	return bundlefilespos			return bundlefilespos

	class bundlerepository(localrepo.localrepository):			class bundlerepository(localrepo.localrepository):
	def __init__(self, ui, path, bundlename):			def __init__(self, ui, path, bundlename):
	def _writetempbundle(read, suffix, header=''):
	"""Write a temporary file to disk

	This is closure because we need to make sure this tracked by
	self.tempfile for cleanup purposes."""
	fdtemp, temp = self.vfs.mkstemp(prefix="hg-bundle-",
	suffix=".hg10un")
	self.tempfile = temp

	with os.fdopen(fdtemp, pycompat.sysstr('wb')) as fptemp:
	fptemp.write(header)
	while True:
	chunk = read(2**18)
	if not chunk:
	break
	fptemp.write(chunk)

	return self.vfs.open(self.tempfile, mode="rb")
	self._tempparent = None			self._tempparent = None
	try:			try:
	localrepo.localrepository.__init__(self, ui, path)			localrepo.localrepository.__init__(self, ui, path)
	except error.RepoError:			except error.RepoError:
	self._tempparent = tempfile.mkdtemp()			self._tempparent = tempfile.mkdtemp()
	localrepo.instance(ui, self._tempparent, 1)			localrepo.instance(ui, self._tempparent, 1)
	localrepo.localrepository.__init__(self, ui, self._tempparent)			localrepo.localrepository.__init__(self, ui, self._tempparent)
	self.ui.setconfig('phases', 'publish', False, 'bundlerepo')			self.ui.setconfig('phases', 'publish', False, 'bundlerepo')
	"multiple changegroups")			"multiple changegroups")
	cgstream = part			cgstream = part
	version = part.params.get('version', '01')			version = part.params.get('version', '01')
	legalcgvers = changegroup.supportedincomingversions(self)			legalcgvers = changegroup.supportedincomingversions(self)
	if version not in legalcgvers:			if version not in legalcgvers:
	msg = _('Unsupported changegroup version: %s')			msg = _('Unsupported changegroup version: %s')
	raise error.Abort(msg % version)			raise error.Abort(msg % version)
	if self.bundle.compressed():			if self.bundle.compressed():
	cgstream = _writetempbundle(part.read,			cgstream = self._writetempbundle(part.read,
	".cg%sun" % version)			".cg%sun" % version)

	if cgstream is None:			if cgstream is None:
	raise error.Abort(_('No changegroups found'))			raise error.Abort(_('No changegroups found'))
	cgstream.seek(0)			cgstream.seek(0)

	self.bundle = changegroup.getunbundler(version, cgstream, 'UN')			self.bundle = changegroup.getunbundler(version, cgstream, 'UN')

	elif self.bundle.compressed():			elif self.bundle.compressed():
	f = _writetempbundle(self.bundle.read, '.hg10un', header='HG10UN')			f = self._writetempbundle(self.bundle.read, '.hg10un',
				header='HG10UN')
	self.bundlefile = self.bundle = exchange.readbundle(ui, f,			self.bundlefile = self.bundle = exchange.readbundle(ui, f,
	bundlename,			bundlename,
	self.vfs)			self.vfs)

	# dict with the mapping 'filename' -> position in the bundle			# dict with the mapping 'filename' -> position in the bundle
	self.bundlefilespos = {}			self.bundlefilespos = {}

	self.firstnewrev = self.changelog.repotiprev + 1			self.firstnewrev = self.changelog.repotiprev + 1
	phases.retractboundary(self, None, phases.draft,			phases.retractboundary(self, None, phases.draft,
	[ctx.node() for ctx in self[self.firstnewrev:]])			[ctx.node() for ctx in self[self.firstnewrev:]])

				def _writetempbundle(self, read, suffix, header=''):
				martinvonzUnsubmitted Done I think we usually use a "fn" prefix for function arguments. I think "readfn" would be clearer. martinvonz: I think we usually use a "fn" prefix for function arguments. I think "readfn" would be clearer.
				krbullockUnsubmitted Done I agree, but as it is, this is simple code movement. Usually we like renames to be done in separate changes. Updating the docstring as @indygreg suggests is appropriate, though. krbullock: I agree, but as it is, this is simple code movement. Usually we like renames to be done in…
				"""Write a temporary file to disk

				This is closure because we need to make sure this tracked by
				self.tempfile for cleanup purposes."""
				indygregUnsubmitted Done Please update this to reflect that it is no longer a closure. indygreg: Please update this to reflect that it is no longer a closure.
				fdtemp, temp = self.vfs.mkstemp(prefix="hg-bundle-",
				suffix=".hg10un")
				self.tempfile = temp

				with os.fdopen(fdtemp, pycompat.sysstr('wb')) as fptemp:
				fptemp.write(header)
				while True:
				chunk = read(2**18)
				if not chunk:
				break
				fptemp.write(chunk)

				return self.vfs.open(self.tempfile, mode="rb")

	@localrepo.unfilteredpropertycache			@localrepo.unfilteredpropertycache
	def _phasecache(self):			def _phasecache(self):
	return bundlephasecache(self, self._phasedefaults)			return bundlephasecache(self, self._phasedefaults)

	@localrepo.unfilteredpropertycache			@localrepo.unfilteredpropertycache
	def changelog(self):			def changelog(self):
	# consume the header if it exists			# consume the header if it exists
	self.bundle.changelogheader()			self.bundle.changelogheader()