# HG changeset patch # User Matt Harbison # Date 1539188728 14400 # Node ID 535fc8a2236542835d23dc37a1269e5079d680ba # 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 -r 3e61146d7b24 -r 535fc8a22365 hgext/lfs/__init__.py --- a/hgext/lfs/__init__.py Mon Oct 01 16:07:38 2018 -0400 +++ b/hgext/lfs/__init__.py Wed Oct 10 12:25:28 2018 -0400 @@ -229,13 +229,15 @@ 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 -r 3e61146d7b24 -r 535fc8a22365 tests/test-lfs-serve.t --- a/tests/test-lfs-serve.t Mon Oct 01 16:07:38 2018 -0400 +++ b/tests/test-lfs-serve.t Wed Oct 10 12:25:28 2018 -0400 @@ -220,6 +220,30 @@ $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