Patchwork [1,of,4,V3] lfs: special case the null:// usercache instead of treating it as a url

login
register
mail settings
Submitter Matt Harbison
Date April 11, 2018, 11:31 p.m.
Message ID <11de91e25707a7acb205.1523489502@MATT7H-PC.attotech.com>
Download mbox | patch
Permalink /patch/30746/
State Accepted
Headers show

Comments

Matt Harbison - April 11, 2018, 11:31 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1523482195 14400
#      Wed Apr 11 17:29:55 2018 -0400
# Node ID 11de91e25707a7acb2052fe87fe7aacde88b65ca
# Parent  8478b198af9cb96463582be895f0b9a06747a13c
lfs: special case the null:// usercache instead of treating it as a url

The previous code worked on Windows, but not on Unix, and a pending patch's test
failed.  The url being used was something like "/tmp/.../client1/null://",
courtesy of ui.configpath().  Looking at the doc comment, this seems like it's
maybe not the right function to call (why should a relative cache path be
expanded relative to the repo root or config file?), but largefiles has been
using it since 8b8dd13295db (Oct 2011).  It was introduced in 1b591f9b7fd2 (Jan
2011) without comment or callers.  A grep over the whole history shows that only
largefiles used it until lfs and infinitepush came along recently.

It looks like if the `if not os.path.isabs(v) or "://" not in v` in configpath()
is changed to an 'and', both Linux and Windows are happy.  I'm guessing that
"://" is to pick off URLs, so that seems reasonable.  But I'm not sure why it
isn't explicitly "file://", and I thought that "file://foo" is relative anyway.
(At least, there are doctests for file:///tmp in util.url.)  There is no mention
of this setting in the help, but it is referenced on the wiki page for
largefiles.  (There's no mention that this is intended to be a URL, and the
example uses an absolute path.)

I don't want this blocking the rest of the lfs server discovery stuff.  It was
also wrong to allow a file:// URL here, but not in largefiles.
Yuya Nishihara - April 12, 2018, 11:43 a.m.
On Wed, 11 Apr 2018 19:31:42 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1523482195 14400
> #      Wed Apr 11 17:29:55 2018 -0400
> # Node ID 11de91e25707a7acb2052fe87fe7aacde88b65ca
> # Parent  8478b198af9cb96463582be895f0b9a06747a13c
> lfs: special case the null:// usercache instead of treating it as a url

Queued, thanks.

Patch

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -174,6 +174,9 @@  configitem('experimental', 'lfs.serve',
 configitem('experimental', 'lfs.user-agent',
     default=None,
 )
+configitem('experimental', 'lfs.disableusercache',
+    default=False,
+)
 configitem('experimental', 'lfs.worker-enable',
     default=False,
 )
diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -118,14 +118,12 @@  class local(object):
     def __init__(self, repo):
         fullpath = repo.svfs.join('lfs/objects')
         self.vfs = lfsvfs(fullpath)
-        usercache = util.url(lfutil._usercachedir(repo.ui, 'lfs'))
-        if usercache.scheme in (None, 'file'):
-            self.cachevfs = lfsvfs(usercache.localpath())
-        elif usercache.scheme == 'null':
+
+        if repo.ui.configbool('experimental', 'lfs.disableusercache'):
             self.cachevfs = nullvfs()
         else:
-            raise error.Abort(_('unknown lfs cache scheme: %s')
-                              % usercache.scheme)
+            usercache = lfutil._usercachedir(repo.ui, 'lfs')
+            self.cachevfs = lfsvfs(usercache)
         self.ui = repo.ui
 
     def open(self, oid):
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
@@ -33,8 +33,9 @@  side.  If that *is* enabled, the subsequ
 for flag '0x2000'!" if the extension is only loaded on one side (possibly also
 masked by the Internal Server Error message).
   $ cat >> $HGRCPATH <<EOF
+  > [experimental]
+  > lfs.disableusercache = True
   > [lfs]
-  > usercache = null://
   > threshold=10
   > [web]
   > allow_push=*