| Submitter | phabricator |
|---|---|
| Date | Aug. 9, 2017, 12:05 a.m. |
| Message ID | <differential-rev-PHID-DREV-3tnmsqav2vnbtidfgwzh-req@phab.mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/22774/ |
| State | Superseded |
| Headers | show |
Comments
lothiraldan added a comment. LGTM REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D288 To: durham, #hg-reviewers Cc: lothiraldan, mercurial-devel
indygreg requested changes to this revision. indygreg added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > bundlerepo.py:325-326 > + > + 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-", Please update this to reflect that it is no longer a closure. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D288 To: durham, #hg-reviewers, indygreg Cc: indygreg, lothiraldan, mercurial-devel
martinvonz added inline comments. INLINE COMMENTS > bundlerepo.py:322 > > + def _writetempbundle(self, read, suffix, header=''): > + """Write a temporary file to disk I think we usually use a "fn" prefix for function arguments. I think "readfn" would be clearer. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D288 To: durham, #hg-reviewers, indygreg Cc: martinvonz, indygreg, lothiraldan, mercurial-devel
krbullock added inline comments. INLINE COMMENTS > martinvonz wrote in bundlerepo.py:322 > I think we usually use a "fn" prefix for function arguments. I think "readfn" would be clearer. 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D288 To: durham, #hg-reviewers, indygreg Cc: krbullock, martinvonz, indygreg, lothiraldan, mercurial-devel
durham added a comment. 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). REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D288 To: durham, #hg-reviewers, indygreg Cc: krbullock, martinvonz, indygreg, lothiraldan, mercurial-devel
Patch
diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py --- a/mercurial/bundlerepo.py +++ b/mercurial/bundlerepo.py @@ -264,24 +264,6 @@ class bundlerepository(localrepo.localrepository): 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 try: localrepo.localrepository.__init__(self, ui, path) @@ -314,17 +296,18 @@ msg = _('Unsupported changegroup version: %s') raise error.Abort(msg % version) if self.bundle.compressed(): - cgstream = _writetempbundle(part.read, - ".cg%sun" % version) + cgstream = self._writetempbundle(part.read, + ".cg%sun" % version) if cgstream is None: raise error.Abort(_('No changegroups found')) cgstream.seek(0) self.bundle = changegroup.getunbundler(version, cgstream, 'UN') 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, bundlename, self.vfs) @@ -336,6 +319,25 @@ phases.retractboundary(self, None, phases.draft, [ctx.node() for ctx in self[self.firstnewrev:]]) + def _writetempbundle(self, 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") + @localrepo.unfilteredpropertycache def _phasecache(self): return bundlephasecache(self, self._phasedefaults)