Re: [PATCH 2 of 6] lfs: add the ability to disable the usercache
> On Apr 9, 2018, at 11:43 AM, Yuya Nishihara wrote:
>
>> On Mon, 9 Apr 2018 11:28:54 -0400, Matt Harbison wrote:
>>
On Apr 9, 2018, at 9:21 AM, Yuya Nishihara wrote:
On Mon, 09 Apr 2018 00:26:42 -0400, Matt Harbison wrote:
# HG changeset patch
# User Matt Harbison
# Date 1523155211 14400
# Sat Apr 07 22:40:11 2018 -0400
# Node ID f4381233ecb960307d39459ea961a0af03df442b
# Parent 69ead78af94459ca41e529d0ebfb24bd1d11c32c
lfs: add the ability to disable the usercache
>>>
+class nullvfs(lfsvfs):
+def __init__(self):
+pass
+
+def exists(self, oid):
+return False
+
+def read(self, oid):
+raise IOError('%s: No such file or directory' % self.join(oid))
+
+def walk(self, path=None, onerror=None):
+return ('', [], [])
+
+def write(self, oid, data):
+pass
>>>
>>> I don't think this abstraction would work nicely. For example, local.read()
>>> expects something exists in usercache.
>>>
>>> def read(self, oid, verify=True):
>>> """Read blob from local blobstore."""
>>> if not self.vfs.exists(oid):
>>> blob = self._read(self.cachevfs, oid, verify)
>>
>> That initially gave me pause too. But vfs.read(oid) inside _read() will also
>> fail if the file doesn’t exist. So before reading from the store, all code
>> calls store.exists(oid). Since the nullvfs always says the file doesn’t
>> exist, the only way for a store to say it exists is if self.vfs (the repo
>> internal store) has it. That’s the desired behavior.
>>
>> The only thing I was puzzling over after seeing that was if nullvfs.read()
>> should raise an error or return an empty string- it should never be called.
>
> Can you run all lfs tests with the nullvfs to see if nullvfs is mostly
> working? I got some AttributeError.
Good catch. The problem is the self.join reference in the read() call when
generating the IOError- self.base that it uses doesn’t exist. The upload case
is not checking that the blob exists beforehand, because it wants an error if
it doesn’t. The revlog read will fetch it first if it doesn’t exist in the
store.
I think the abstraction still works though because if I point self.cachevfs to
self.vfs, the test case for not having the local blob (where this was blowing
up) fails in exactly the same way. The only difference is the full path isn’t
available, and the error code for a real vfs may be ENOENT or ENOTDIR because
of the path sharding.
I’ll fix up the IOError call. test-lfs.t and test-lfs-test-server.t rely on
the cache, but nothing else looked wrong and the other lfs tests run cleanly.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6] lfs: add the ability to disable the usercache
On Mon, 9 Apr 2018 11:28:54 -0400, Matt Harbison wrote:
>
> > On Apr 9, 2018, at 9:21 AM, Yuya Nishihara wrote:
> >
> >> On Mon, 09 Apr 2018 00:26:42 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison
> >> # Date 1523155211 14400
> >> # Sat Apr 07 22:40:11 2018 -0400
> >> # Node ID f4381233ecb960307d39459ea961a0af03df442b
> >> # Parent 69ead78af94459ca41e529d0ebfb24bd1d11c32c
> >> lfs: add the ability to disable the usercache
> >
> >> +class nullvfs(lfsvfs):
> >> +def __init__(self):
> >> +pass
> >> +
> >> +def exists(self, oid):
> >> +return False
> >> +
> >> +def read(self, oid):
> >> +raise IOError('%s: No such file or directory' % self.join(oid))
> >> +
> >> +def walk(self, path=None, onerror=None):
> >> +return ('', [], [])
> >> +
> >> +def write(self, oid, data):
> >> +pass
> >
> > I don't think this abstraction would work nicely. For example, local.read()
> > expects something exists in usercache.
> >
> >def read(self, oid, verify=True):
> >"""Read blob from local blobstore."""
> >if not self.vfs.exists(oid):
> >blob = self._read(self.cachevfs, oid, verify)
>
> That initially gave me pause too. But vfs.read(oid) inside _read() will also
> fail if the file doesn’t exist. So before reading from the store, all code
> calls store.exists(oid). Since the nullvfs always says the file doesn’t
> exist, the only way for a store to say it exists is if self.vfs (the repo
> internal store) has it. That’s the desired behavior.
>
> The only thing I was puzzling over after seeing that was if nullvfs.read()
> should raise an error or return an empty string- it should never be called.
Can you run all lfs tests with the nullvfs to see if nullvfs is mostly
working? I got some AttributeError.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6] lfs: add the ability to disable the usercache
> On Apr 9, 2018, at 9:21 AM, Yuya Nishihara wrote:
>
>> On Mon, 09 Apr 2018 00:26:42 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison
>> # Date 1523155211 14400
>> # Sat Apr 07 22:40:11 2018 -0400
>> # Node ID f4381233ecb960307d39459ea961a0af03df442b
>> # Parent 69ead78af94459ca41e529d0ebfb24bd1d11c32c
>> lfs: add the ability to disable the usercache
>
>> +class nullvfs(lfsvfs):
>> +def __init__(self):
>> +pass
>> +
>> +def exists(self, oid):
>> +return False
>> +
>> +def read(self, oid):
>> +raise IOError('%s: No such file or directory' % self.join(oid))
>> +
>> +def walk(self, path=None, onerror=None):
>> +return ('', [], [])
>> +
>> +def write(self, oid, data):
>> +pass
>
> I don't think this abstraction would work nicely. For example, local.read()
> expects something exists in usercache.
>
>def read(self, oid, verify=True):
>"""Read blob from local blobstore."""
>if not self.vfs.exists(oid):
>blob = self._read(self.cachevfs, oid, verify)
That initially gave me pause too. But vfs.read(oid) inside _read() will also
fail if the file doesn’t exist. So before reading from the store, all code
calls store.exists(oid). Since the nullvfs always says the file doesn’t exist,
the only way for a store to say it exists is if self.vfs (the repo internal
store) has it. That’s the desired behavior.
The only thing I was puzzling over after seeing that was if nullvfs.read()
should raise an error or return an empty string- it should never be called.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6] lfs: add the ability to disable the usercache
On Mon, 09 Apr 2018 00:26:42 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison
> # Date 1523155211 14400
> # Sat Apr 07 22:40:11 2018 -0400
> # Node ID f4381233ecb960307d39459ea961a0af03df442b
> # Parent 69ead78af94459ca41e529d0ebfb24bd1d11c32c
> lfs: add the ability to disable the usercache
> +class nullvfs(lfsvfs):
> +def __init__(self):
> +pass
> +
> +def exists(self, oid):
> +return False
> +
> +def read(self, oid):
> +raise IOError('%s: No such file or directory' % self.join(oid))
> +
> +def walk(self, path=None, onerror=None):
> +return ('', [], [])
> +
> +def write(self, oid, data):
> +pass
I don't think this abstraction would work nicely. For example, local.read()
expects something exists in usercache.
def read(self, oid, verify=True):
"""Read blob from local blobstore."""
if not self.vfs.exists(oid):
blob = self._read(self.cachevfs, oid, verify)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
