[PATCH STABLE] lfs: avoid a potential variable reference before assignment error in cmdserver

[Matt Harbison]

# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1539188728 14400
#      Wed Oct 10 12:25:28 2018 -0400
# Branch stable
# Node ID 51e0af18304005dc62d1debe1bba6614140056ab
# Parent  3e61146d7b243078101ba1a554d90490fc915f71
lfs: avoid a potential variable reference before assignment error in cmdserver

A coworker hit this once yesterday when pulling in thg (a retry worked), and
then I hit it with strip after a pull.  I had a difficult time recreating a test
for this (at least one of the tricks was to not use '-R', which seems to cause
reposetup() to be called for each command), so I'm not sure how large of a
window there actually is for this.  Calling reposetup() *after* the requirement
is added will skip the hook entirely.

The other issue I had was adding a couple `ui.status()` lines around the check
that installs the hook.  On Windows, the cmdserver process ballooned to 1.6GB
and hung.  Changing that to `ui.warn()` avoided the hang.  It also hung on
macOS, but without the large memory usage.

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -229,13 +229,15 @@ def reposetup(ui, repo):
 
     if 'lfs' not in repo.requirements:
         def checkrequireslfs(ui, repo, **kwargs):
-            if 'lfs' not in repo.requirements:
-                last = kwargs.get(r'node_last')
-                _bin = node.bin
-                if last:
-                    s = repo.set('%n:%n', _bin(kwargs[r'node']), _bin(last))
-                else:
-                    s = repo.set('%n', _bin(kwargs[r'node']))
+            if 'lfs' in repo.requirements:
+                return 0
+
+            last = kwargs.get(r'node_last')
+            _bin = node.bin
+            if last:
+                s = repo.set('%n:%n', _bin(kwargs[r'node']), _bin(last))
+            else:
+                s = repo.set('%n', _bin(kwargs[r'node']))
             match = repo.narrowmatch()
             for ctx in s:
                 # TODO: is there a way to just walk the files in the commit?
diff --git a/tests/test-lfs-serve.t b/tests/test-lfs-serve.t
--- a/tests/test-lfs-serve.t
+++ b/tests/test-lfs-serve.t
@@ -220,6 +220,30 @@ should have an 'lfs' requirement after i
   $TESTTMP/client3_pull/.hg/requires:lfs (lfsremote-on !)
   $TESTTMP/server/.hg/requires:lfs (lfsremote-on !)
 
+Test that the commit/changegroup requirement check hook can be run multiple
+times.
+
+  $ hg clone -qr 0 http://localhost:$HGPORT $TESTTMP/cmdserve_client3
+
+  $ cd ../cmdserve_client3
+
+  >>> from __future__ import absolute_import
+  >>> from hgclient import check, readchannel, runcommand
+  >>> @check
+  ... def addrequirement(server):
+  ...     readchannel(server)
+  ...     # change the repo in a way that adds the lfs requirement
+  ...     runcommand(server, ['pull', '-qu'])
+  ...     # Now cause the requirement adding hook to fire again, without going
+  ...     # through reposetup() again.
+  ...     with open('file.txt', 'wb') as fp:
+  ...         fp.write('data')
+  ...     runcommand(server, ['ci', '-Aqm', 'non-lfs'])
+  *** runcommand pull -qu
+  *** runcommand ci -Aqm non-lfs
+
+  $ cd ../client
+
 The difference here is the push failed above when the extension isn't
 enabled on the server.
   $ hg identify http://localhost:$HGPORT