sshpeer: enable+fix warning about sshpeers not being closed explicitly

I recommend looking at this with a diff that ignores indentation.

The test changes are because localrepo.close() updates some cache,
which appears happens earlier now on rollbacks or strips or something.

The http changes are because httppeer.close() prints stats with
--verbose.

Differential Revision: https://phab.mercurial-scm.org/D9999

hgext/infinitepush/__init__.py

@@ -704,6 +704,7 b' def _pull(orig, ui, repo, source=b"defau'
 
         if scratchbookmarks:
             other = hg.peer(repo, opts, source)
+            try:
             fetchedbookmarks = other.listkeyspatterns(
                 b'bookmarks', patterns=scratchbookmarks
             )
@@ -714,6 +715,8 b' def _pull(orig, ui, repo, source=b"defau'
                     )
                 scratchbookmarks[bookmark] = fetchedbookmarks[bookmark]
                 revs.append(fetchedbookmarks[bookmark])
+            finally:
+                other.close()
         opts[b'bookmark'] = bookmarks
         opts[b'rev'] = revs
 
@@ -848,10 +851,13 b' def _push(orig, ui, repo, dest=None, *ar'
         if common.isremotebooksenabled(ui):
             if bookmark and scratchpush:
                 other = hg.peer(repo, opts, destpath)
+                try:
                 fetchedbookmarks = other.listkeyspatterns(
                     b'bookmarks', patterns=[bookmark]
                 )
                 remotescratchbookmarks.update(fetchedbookmarks)
+                finally:
+                    other.close()
             _saveremotebookmarks(repo, remotescratchbookmarks, destpath)
     if oldphasemove:
         exchange._localphasemove = oldphasemove

hgext/narrow/narrowcommands.py

@@ -595,10 +595,14 b' def trackedcmd(ui, repo, remotepath=None'
         ui.status(_(b'comparing with %s\n') % util.hidepassword(url))
         remote = hg.peer(repo, opts, url)
 
+        try:
         # check narrow support before doing anything if widening needs to be
         # performed. In future we should also abort if client is ellipses and
         # server does not support ellipses
-        if widening and wireprototypes.NARROWCAP not in remote.capabilities():
+            if (
+                widening
+                and wireprototypes.NARROWCAP not in remote.capabilities()
+            ):
             raise error.Abort(_(b"server does not support narrow clones"))
 
         commoninc = discovery.findcommonincoming(repo, remote)
@@ -667,5 +671,7 b' def trackedcmd(ui, repo, remotepath=None'
                 newincludes,
                 newexcludes,
             )
+        finally:
+            remote.close()
 
     return 0

mercurial/commands.py

@@ -3820,9 +3820,12 b' def identify('
     output = []
     revs = []
 
+    peer = None
+    try:
     if source:
         source, branches = hg.parseurl(ui.expandpath(source))
-        peer = hg.peer(repo or ui, opts, source)  # only pass ui when no repo
+            # only pass ui when no repo
+            peer = hg.peer(repo or ui, opts, source)
         repo = peer.local()
         revs, checkout = hg.addbranchrevs(repo, peer, branches, None)
 
@@ -3946,6 +3949,9 b' def identify('
 
     fm.plain(b"%s\n" % b' '.join(output))
     fm.end()
+    finally:
+        if peer:
+            peer.close()
 
 
 @command(
@@ -4291,12 +4297,15 b' def incoming(ui, repo, source=b"default"'
             ui.expandpath(source), opts.get(b'branch')
         )
         other = hg.peer(repo, opts, source)
+        try:
         if b'bookmarks' not in other.listkeys(b'namespaces'):
             ui.warn(_(b"remote doesn't support bookmarks\n"))
             return 0
         ui.pager(b'incoming')
         ui.status(_(b'comparing with %s\n') % util.hidepassword(source))
         return bookmarks.incoming(ui, repo, other)
+        finally:
+            other.close()
 
     repo._subtoppath = ui.expandpath(source)
     try:
@@ -4327,7 +4336,8 b' def init(ui, dest=b".", **opts):'
     Returns 0 on success.
     """
     opts = pycompat.byteskwargs(opts)
-    hg.peer(ui, opts, ui.expandpath(dest), create=True)
+    peer = hg.peer(ui, opts, ui.expandpath(dest), create=True)
+    peer.close()
 
 
 @command(
@@ -4963,12 +4973,15 b' def outgoing(ui, repo, dest=None, **opts'
     if opts.get(b'bookmarks'):
         dest = path.pushloc or path.loc
         other = hg.peer(repo, opts, dest)
+        try:
         if b'bookmarks' not in other.listkeys(b'namespaces'):
             ui.warn(_(b"remote doesn't support bookmarks\n"))
             return 0
         ui.status(_(b'comparing with %s\n') % util.hidepassword(dest))
         ui.pager(b'outgoing')
         return bookmarks.outgoing(ui, repo, other)
+        finally:
+            other.close()
 
     repo._subtoppath = path.pushloc or path.loc
     try:
@@ -5679,6 +5692,7 b' def push(ui, repo, dest=None, **opts):'
     revs, checkout = hg.addbranchrevs(repo, repo, branches, opts.get(b'rev'))
     other = hg.peer(repo, opts, dest)
 
+    try:
     if revs:
         revs = [repo[r].node() for r in scmutil.revrange(repo, revs)]
         if not revs:
@@ -5714,7 +5728,9 b' def push(ui, repo, dest=None, **opts):'
     finally:
         del repo._subtoppath
 
-    opargs = dict(opts.get(b'opargs', {}))  # copy opargs since we may mutate it
+        opargs = dict(
+            opts.get(b'opargs', {})
+        )  # copy opargs since we may mutate it
     opargs.setdefault(b'pushvars', []).extend(opts.get(b'pushvars', []))
 
     pushop = exchange.push(
@@ -5735,7 +5751,8 b' def push(ui, repo, dest=None, **opts):'
             result = 2
         elif not result and pushop.bkresult:
             result = 2
-
+    finally:
+        other.close()
     return result
 
 

mercurial/debugcommands.py

@@ -471,6 +471,7 b' def debugcapabilities(ui, path, **opts):'
     """lists the capabilities of a remote peer"""
     opts = pycompat.byteskwargs(opts)
     peer = hg.peer(ui, opts, path)
+    try:
     caps = peer.capabilities()
     ui.writenoi18n(b'Main capabilities:\n')
     for c in sorted(caps):
@@ -482,6 +483,8 b' def debugcapabilities(ui, path, **opts):'
             ui.write(b'  %s\n' % key)
             for v in values:
                 ui.write(b'    %s\n' % v)
+    finally:
+        peer.close()
 
 
 @command(
@@ -2615,12 +2618,17 b' def debugpeer(ui, path):'
     with ui.configoverride(overrides):
         peer = hg.peer(ui, {}, path)
 
+        try:
         local = peer.local() is not None
         canpush = peer.canpush()
 
         ui.write(_(b'url: %s\n') % peer.url())
         ui.write(_(b'local: %s\n') % (_(b'yes') if local else _(b'no')))
-        ui.write(_(b'pushable: %s\n') % (_(b'yes') if canpush else _(b'no')))
+            ui.write(
+                _(b'pushable: %s\n') % (_(b'yes') if canpush else _(b'no'))
+            )
+        finally:
+            peer.close()
 
 
 @command(
@@ -2723,6 +2731,7 b' def debugpushkey(ui, repopath, namespace'
     """
 
     target = hg.peer(ui, {}, repopath)
+    try:
     if keyinfo:
         key, old, new = keyinfo
         with target.commandexecutor() as e:
@@ -2741,8 +2750,11 b' def debugpushkey(ui, repopath, namespace'
     else:
         for k, v in sorted(pycompat.iteritems(target.listkeys(namespace))):
             ui.write(
-                b"%s\t%s\n" % (stringutil.escapestr(k), stringutil.escapestr(v))
-            )
+                    b"%s\t%s\n"
+                    % (stringutil.escapestr(k), stringutil.escapestr(v))
+                )
+    finally:
+        target.close()
 
 
 @command(b'debugpvec', [], _(b'A B'))
@@ -4095,6 +4107,7 b' def debugwhyunstable(ui, repo, rev):'
 def debugwireargs(ui, repopath, *vals, **opts):
     opts = pycompat.byteskwargs(opts)
     repo = hg.peer(ui, opts, repopath)
+    try:
     for opt in cmdutil.remoteopts:
         del opts[opt[1]]
     args = {}
@@ -4108,6 +4121,8 b' def debugwireargs(ui, repopath, *vals, *'
     ui.write(b"%s\n" % res1)
     if res1 != res2:
         ui.warn(b"%s\n" % res2)
+    finally:
+        repo.close()
 
 
 def _parsewirelangblocks(fh):

mercurial/hg.py

@@ -678,6 +678,9 b' def clone('
         srcpeer = source.peer()  # in case we were called with a localrepo
         branches = (None, branch or [])
         origsource = source = srcpeer.url()
+    srclock = destlock = cleandir = None
+    destpeer = None
+    try:
     revs, checkout = addbranchrevs(srcpeer, srcpeer, branches, revs)
 
     if dest is None:
@@ -696,9 +699,13 b' def clone('
     destvfs = vfsmod.vfs(dest, expandpath=True)
     if destvfs.lexists():
         if not destvfs.isdir():
-            raise error.InputError(_(b"destination '%s' already exists") % dest)
+                raise error.InputError(
+                    _(b"destination '%s' already exists") % dest
+                )
         elif destvfs.listdir():
-            raise error.InputError(_(b"destination '%s' is not empty") % dest)
+                raise error.InputError(
+                    _(b"destination '%s' is not empty") % dest
+                )
 
     createopts = {}
     narrow = False
@@ -792,7 +799,9 b' def clone('
 
         # TODO this is a somewhat arbitrary restriction.
         if narrow:
-            ui.status(_(b'(pooled storage not supported for narrow clones)\n'))
+                ui.status(
+                    _(b'(pooled storage not supported for narrow clones)\n')
+                )
             sharepath = None
 
         if sharepath:
@@ -809,9 +818,8 b' def clone('
                 stream=stream,
             )
 
-    srclock = destlock = cleandir = None
     srcrepo = srcpeer.local()
-    try:
+
         abspath = origsource
         if islocal(origsource):
             abspath = os.path.abspath(util.urllocalpath(origsource))
@@ -1052,6 +1060,8 b' def clone('
             shutil.rmtree(cleandir, True)
         if srcpeer is not None:
             srcpeer.close()
+        if destpeer and destpeer.local() is None:
+            destpeer.close()
     return srcpeer, destpeer
 
 
@@ -1253,6 +1263,8 b' def _incoming('
     """
     source, branches = parseurl(ui.expandpath(source), opts.get(b'branch'))
     other = peer(repo, opts, source)
+    cleanupfn = other.close
+    try:
     ui.status(_(b'comparing with %s\n') % util.hidepassword(source))
     revs, checkout = addbranchrevs(repo, other, branches, opts.get(b'rev'))
 
@@ -1261,7 +1273,7 b' def _incoming('
     other, chlist, cleanupfn = bundlerepo.getremotechanges(
         ui, repo, other, revs, opts[b"bundle"], opts[b"force"]
     )
-    try:
+
         if not chlist:
             ui.status(_(b"no changes found\n"))
             return subreporecurse()
@@ -1320,6 +1332,7 b' def _outgoing(ui, repo, dest, opts):'
         revs = [repo[rev].node() for rev in scmutil.revrange(repo, revs)]
 
     other = peer(repo, opts, dest)
+    try:
     outgoing = discovery.findcommonoutgoing(
         repo, other, revs, force=opts.get(b'force')
     )
@@ -1327,6 +1340,9 b' def _outgoing(ui, repo, dest, opts):'
     if not o:
         scmutil.nochangesfound(repo.ui, repo, outgoing.excluded)
     return o, other
+    except:  # re-raises
+        other.close()
+        raise
 
 
 def outgoing(ui, repo, dest, opts):
@@ -1341,6 +1357,7 b' def outgoing(ui, repo, dest, opts):'
 
     limit = logcmdutil.getlimit(opts)
     o, other = _outgoing(ui, repo, dest, opts)
+    try:
     if not o:
         cmdutil.outgoinghooks(ui, repo, other, opts, o)
         return recurse()
@@ -1362,6 +1379,8 b' def outgoing(ui, repo, dest, opts):'
     cmdutil.outgoinghooks(ui, repo, other, opts, o)
     recurse()
     return 0  # exit code is zero since we found outgoing changes
+    finally:
+        other.close()
 
 
 def verify(repo, level=None):

mercurial/revset.py

@@ -1841,9 +1841,12 b' def outgoing(repo, subset, x):'
     if revs:
         revs = [repo.lookup(rev) for rev in revs]
     other = hg.peer(repo, {}, dest)
+    try:
     repo.ui.pushbuffer()
     outgoing = discovery.findcommonoutgoing(repo, other, onlyheads=revs)
     repo.ui.popbuffer()
+    finally:
+        other.close()
     cl = repo.changelog
     o = {cl.rev(r) for r in outgoing.missing}
     return subset & o

mercurial/sshpeer.py

@@ -175,10 +175,7 b' def _cleanuppipes(ui, pipei, pipeo, pipe'
         # to deadlocks due to a peer get gc'ed in a fork
         # We add our own stack trace, because the stacktrace when called
         # from __del__ is useless.
-        if False:  # enabled in next commit
-            ui.develwarn(
-                b'missing close on SSH connection created at:\n%s' % warn
-            )
+        ui.develwarn(b'missing close on SSH connection created at:\n%s' % warn)
 
 
 def _makeconnection(ui, sshcmd, args, remotecmd, path, sshenv=None):

mercurial/subrepo.py

@@ -716,13 +716,17 b' class hgsubrepo(abstractsubrepo):'
                     _(b'sharing subrepo %s from %s\n')
                     % (subrelpath(self), srcurl)
                 )
+                peer = getpeer()
+                try:
                 shared = hg.share(
                     self._repo._subparent.baseui,
-                    getpeer(),
+                        peer,
                     self._repo.root,
                     update=False,
                     bookmarks=False,
                 )
+                finally:
+                    peer.close()
                 self._repo = shared.local()
             else:
                 # TODO: find a common place for this and this code in the
@@ -743,14 +747,18 b' class hgsubrepo(abstractsubrepo):'
                     _(b'cloning subrepo %s from %s\n')
                     % (subrelpath(self), util.hidepassword(srcurl))
                 )
+                peer = getpeer()
+                try:
                 other, cloned = hg.clone(
                     self._repo._subparent.baseui,
                     {},
-                    getpeer(),
+                        peer,
                     self._repo.root,
                     update=False,
                     shareopts=shareopts,
                 )
+                finally:
+                    peer.close()
                 self._repo = cloned.local()
             self._initrepo(parentrepo, source, create=True)
             self._cachestorehash(srcurl)
@@ -760,7 +768,11 b' class hgsubrepo(abstractsubrepo):'
                 % (subrelpath(self), util.hidepassword(srcurl))
             )
             cleansub = self.storeclean(srcurl)
-            exchange.pull(self._repo, getpeer())
+            peer = getpeer()
+            try:
+                exchange.pull(self._repo, peer)
+            finally:
+                peer.close()
             if cleansub:
                 # keep the repo clean after pull
                 self._cachestorehash(srcurl)
@@ -845,7 +857,10 b' class hgsubrepo(abstractsubrepo):'
             % (subrelpath(self), util.hidepassword(dsturl))
         )
         other = hg.peer(self._repo, {b'ssh': ssh}, dsturl)
+        try:
         res = exchange.push(self._repo, other, force, newbranch=newbranch)
+        finally:
+            other.close()
 
         # the repo is now clean
         self._cachestorehash(dsturl)

tests/remotefilelog-getflogheads.py

@@ -21,7 +21,10 b' def getflogheads(ui, repo, path):'
     dest = repo.ui.expandpath(b'default')
     peer = hg.peer(repo, {}, dest)
 
+    try:
     flogheads = peer.x_rfl_getflogheads(path)
+    finally:
+        peer.close()
 
     if flogheads:
         for head in flogheads:

tests/test-acl.t

@@ -361,6 +361,7 b' Empty [acl.allow]'
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" not allowed on "foo/file.txt" (changeset "ef1ea85a6374")
   no rollback information available
   0:6675d58eff77
@@ -808,7 +809,6 b' fred is not blocked from moving bookmark'
   acl: acl.deny.bookmarks not enabled
   acl: bookmark access granted: "ef1ea85a6374b77d6da9dcda9541f498f2d17df7" on bookmark "moving-bookmark"
   bundle2-input-bundle: 7 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   invalid branch cache (served.hidden): tip differs
   added 1 changesets with 1 changes to 1 files
@@ -900,6 +900,7 b' fred is not allowed to move bookmarks'
   bundle2-input-bundle: 7 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on bookmark "moving-bookmark" (changeset "ef1ea85a6374b77d6da9dcda9541f498f2d17df7")
   no rollback information available
   0:6675d58eff77
@@ -985,7 +986,6 b' barney is allowed everywhere'
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1073,6 +1073,7 b' wilma can change files with a .txt exten'
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "wilma" not allowed on "quux/file.py" (changeset "911600dab2ae")
   no rollback information available
   0:6675d58eff77
@@ -1322,7 +1323,6 b' acl.config can set only [acl.allow]/[acl'
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1499,6 +1499,7 b' no one is allowed inside foo/Bar/'
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on "foo/Bar/file.txt" (changeset "f9cafe1212c8")
   no rollback information available
   0:6675d58eff77
@@ -1583,7 +1584,6 b' OS-level groups'
   bundle2-input-part: "phase-heads" supported
   bundle2-input-part: total payload size 24
   bundle2-input-bundle: 5 parts total
-  truncating cache/rbc-revs-v1 to 8
   updating the branch cache
   added 3 changesets with 3 changes to 3 files
   bundle2-output-bundle: "HG20", 1 parts total
@@ -1671,6 +1671,7 b' OS-level groups'
   bundle2-input-bundle: 5 parts total
   transaction abort!
   rollback completed
+  truncating cache/rbc-revs-v1 to 8
   abort: acl: user "fred" denied on "foo/Bar/file.txt" (changeset "f9cafe1212c8")
   no rollback information available
   0:6675d58eff77

tests/test-http.t

@@ -382,6 +382,7 b' test http authentication'
   devel-peer-request:   16 bytes of commands arguments in headers
   devel-peer-request:   finished in *.???? seconds (200) (glob)
   received listkey for "phases": 15 bytes
+  (sent 9 HTTP requests and 3898 bytes; received 920 bytes in responses)
   $ hg rollback -q
 
   $ sed 's/.*] "/"/' < ../access.log

tests/test-lfs-serve.t

@@ -462,6 +462,7 b' lfs content, and the extension enabled.'
   remote: adding manifests
   remote: adding file changes
   remote: added 1 changesets with 1 changes to 1 files
+  (sent 8 HTTP requests and 3526 bytes; received 961 bytes in responses) (?)
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
   $TESTTMP/server/.hg/requires:lfs