lfs: show a friendly message when pushing lfs to a server without lfs enabled
authorMatt Harbison <matt_harbison@yahoo.com>
Sat, 23 Dec 2017 17:49:12 -0500
changeset 35506 fa865878a849
parent 35505 2526579a30e0
child 35507 95a9be56c3bb
lfs: show a friendly message when pushing lfs to a server without lfs enabled Upfront disclaimer: I don't know anything about the wire protocol, and this was pretty much cargo-culted from largefiles, and then clonebundles, since it seems more modern. I was surprised that exchange.push() will ensure all of the proper requirements when exchanging between two local repos, but doesn't care when one is remote. All this new capability marker does is inform the client that the extension is enabled remotely. It may or may not contain commits with external blobs. Open issues: - largefiles uses 'largefiles=serve' for its capability. Someday I hope to be able to push lfs blobs to an `hg serve` instance. That will probably require a distinct capability. Should it change to '=serve' then? Or just add an 'lfs-serve' capability then? - The flip side of this is more complicated. It looks like largefiles adds an 'lheads' command for the client to signal to the server that the extension is loaded. That is then converted to 'heads' and sent through the normal wire protocol plumbing. A client using the 'heads' command directly is kicked out with a message indicating that the largefiles extension must be loaded. We could do similar with 'lfsheads', but then a repo with both largefiles and lfs blobs can't be pushed over the wire. Hopefully somebody with more wire protocol experience can think of something else. I see 'x-hgarg-1' on some commands in the tests, but not on heads, and didn't dig any further.
hgext/lfs/__init__.py
hgext/lfs/wrapper.py
tests/test-lfs-serve.t
--- a/hgext/lfs/__init__.py	Sun Dec 24 01:26:36 2017 -0500
+++ b/hgext/lfs/__init__.py	Sat Dec 23 17:49:12 2017 -0500
@@ -49,6 +49,7 @@
     scmutil,
     upgrade,
     vfs as vfsmod,
+    wireproto,
 )
 
 from . import (
@@ -169,6 +170,9 @@
                  'allsupportedversions',
                  wrapper.allsupportedversions)
 
+    wrapfunction(exchange, 'push', wrapper.push)
+    wrapfunction(wireproto, '_capabilities', wrapper._capabilities)
+
     wrapfunction(context.basefilectx, 'cmp', wrapper.filectxcmp)
     wrapfunction(context.basefilectx, 'isbinary', wrapper.filectxisbinary)
     context.basefilectx.islfs = wrapper.filectxislfs
--- a/hgext/lfs/wrapper.py	Sun Dec 24 01:26:36 2017 -0500
+++ b/hgext/lfs/wrapper.py	Sat Dec 23 17:49:12 2017 -0500
@@ -39,6 +39,13 @@
     versions.add('03')
     return versions
 
+def _capabilities(orig, repo, proto):
+    '''Wrap server command to announce lfs server capability'''
+    caps = orig(repo, proto)
+    # XXX: change to 'lfs=serve' when separate git server isn't required?
+    caps.append('lfs')
+    return caps
+
 def bypasscheckhash(self, text):
     return False
 
@@ -265,6 +272,20 @@
     """
     return uploadblobsfromrevs(pushop.repo, pushop.outgoing.missing)
 
+def push(orig, repo, remote, *args, **kwargs):
+    """bail on push if the extension isn't enabled on remote when needed"""
+    if 'lfs' in repo.requirements:
+        # If the remote peer is for a local repo, the requirement tests in the
+        # base class method enforce lfs support.  Otherwise, some revisions in
+        # this repo use lfs, and the remote repo needs the extension loaded.
+        if not remote.local() and not remote.capable('lfs'):
+            # This is a copy of the message in exchange.push() when requirements
+            # are missing between local repos.
+            m = _("required features are not supported in the destination: %s")
+            raise error.Abort(m % 'lfs',
+                              hint=_('enable the lfs extension on the server'))
+    return orig(repo, remote, *args, **kwargs)
+
 def writenewbundle(orig, ui, repo, source, filename, bundletype, outgoing,
                    *args, **kwargs):
     """upload LFS blobs added by outgoing revisions on 'hg bundle'"""
--- a/tests/test-lfs-serve.t	Sun Dec 24 01:26:36 2017 -0500
+++ b/tests/test-lfs-serve.t	Sat Dec 23 17:49:12 2017 -0500
@@ -124,10 +124,14 @@
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
 
-TODO: fail more gracefully here
-  $ hg push -q 2>&1 | grep '^[A-Z]' || true
-  Traceback (most recent call last): (lfsremote-off !)
-  ValueError: no common changegroup version (lfsremote-off !)
+#if lfsremote-off
+  $ hg push -q
+  abort: required features are not supported in the destination: lfs
+  (enable the lfs extension on the server)
+  [255]
+#else
+  $ hg push -q
+#endif
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
   $TESTTMP/server/.hg/requires:lfs (lfsremote-on !)