Patchwork [02,of,16] vfs: add a read only vfs

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 2, 2013, 1:09 a.m.
Message ID <f347244a1d604ed96f3a.1357088964@yamac.lan>
Download mbox | patch
Permalink /patch/347/
State Superseded, archived
Commit c38a62af000e3bd4a760066656751a8132c54c2c
Headers show

Comments

Pierre-Yves David - Jan. 2, 2013, 1:09 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
# Date 1356664856 -3600
# Node ID f347244a1d604ed96f3a4231e1bb1868fb60c533
# Parent  13d23b0b1ef6751240408123218108899f0f1881
vfs: add a read only vfs

This read only wrapper is intended to be used bundle repo. See follow up commit
for details.
Kevin Bullock - Jan. 3, 2013, 10:55 p.m.
On Jan 1, 2013, at 7:09 PM, Pierre-Yves David wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1356664856 -3600
> # Node ID f347244a1d604ed96f3a4231e1bb1868fb60c533
> # Parent  13d23b0b1ef6751240408123218108899f0f1881
> vfs: add a read only vfs
> 
> This read only wrapper is intended to be used bundle repo. See follow up commit
> for details.
> 
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -376,10 +376,22 @@ class filtervfs(abstractvfs, auditvfs):
>         else:
>             return self.vfs.join(path)
> 
> filteropener = filtervfs
> 
> +class readonlyvfs(abstractvfs, auditvfs):
> +    '''Wrapper vfs preventing any writing.'''
> +
> +    def __init__(self, vfs):
> +        auditvfs.__init__(self, vfs)
> +
> +    def __call__(self, path, mode='r', *args, **kw):
> +        if mode not in ('r', 'rb'):
> +            raise IOError('Permission denied')

Misleading message. It's not a filesystem constraint preventing us from writing to 'path', it's a caller violating the expectations of the object using readonlyvfs.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Kevin Bullock - Jan. 3, 2013, 10:57 p.m.
On Jan 3, 2013, at 4:55 PM, Kevin Bullock wrote:

> On Jan 1, 2013, at 7:09 PM, Pierre-Yves David wrote:
> 
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1356664856 -3600
>> # Node ID f347244a1d604ed96f3a4231e1bb1868fb60c533
>> # Parent  13d23b0b1ef6751240408123218108899f0f1881
>> vfs: add a read only vfs
>> 
>> This read only wrapper is intended to be used bundle repo. See follow up commit
>> for details.
>> 
>> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
>> --- a/mercurial/scmutil.py
>> +++ b/mercurial/scmutil.py
>> @@ -376,10 +376,22 @@ class filtervfs(abstractvfs, auditvfs):
>>        else:
>>            return self.vfs.join(path)
>> 
>> filteropener = filtervfs
>> 
>> +class readonlyvfs(abstractvfs, auditvfs):
>> +    '''Wrapper vfs preventing any writing.'''
>> +
>> +    def __init__(self, vfs):
>> +        auditvfs.__init__(self, vfs)
>> +
>> +    def __call__(self, path, mode='r', *args, **kw):
>> +        if mode not in ('r', 'rb'):
>> +            raise IOError('Permission denied')
> 
> Misleading message. It's not a filesystem constraint preventing us from writing to 'path', it's a caller violating the expectations of the object using readonlyvfs.


?and actually, we shouldn't be using IOError for this either.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130103/175ea149/attachment.html>
Matt Mackall - Jan. 3, 2013, 10:59 p.m.
On Thu, 2013-01-03 at 16:55 -0600, Kevin Bullock wrote:
> On Jan 1, 2013, at 7:09 PM, Pierre-Yves David wrote:
> 
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> > # Date 1356664856 -3600
> > # Node ID f347244a1d604ed96f3a4231e1bb1868fb60c533
> > # Parent  13d23b0b1ef6751240408123218108899f0f1881
> > vfs: add a read only vfs
> > 
> > This read only wrapper is intended to be used bundle repo. See follow up commit
> > for details.
> > 
> > diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> > --- a/mercurial/scmutil.py
> > +++ b/mercurial/scmutil.py
> > @@ -376,10 +376,22 @@ class filtervfs(abstractvfs, auditvfs):
> >         else:
> >             return self.vfs.join(path)
> > 
> > filteropener = filtervfs
> > 
> > +class readonlyvfs(abstractvfs, auditvfs):
> > +    '''Wrapper vfs preventing any writing.'''
> > +
> > +    def __init__(self, vfs):
> > +        auditvfs.__init__(self, vfs)
> > +
> > +    def __call__(self, path, mode='r', *args, **kw):
> > +        if mode not in ('r', 'rb'):
> > +            raise IOError('Permission denied')
> 
> Misleading message. It's not a filesystem constraint preventing us
> from writing to 'path', it's a caller violating the expectations of
> the object using readonlyvfs.

Indeed, under no circumstances should we be spoofing operating
system-level errors. Debugging is already hard enough; users routinely
believe that 'permission denied' is something Mercurial is inflicting on
them.

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -376,10 +376,22 @@  class filtervfs(abstractvfs, auditvfs):
         else:
             return self.vfs.join(path)
 
 filteropener = filtervfs
 
+class readonlyvfs(abstractvfs, auditvfs):
+    '''Wrapper vfs preventing any writing.'''
+
+    def __init__(self, vfs):
+        auditvfs.__init__(self, vfs)
+
+    def __call__(self, path, mode='r', *args, **kw):
+        if mode not in ('r', 'rb'):
+            raise IOError('Permission denied')
+        return self.vfs(path, mode, *args, **kw)
+
+
 def canonpath(root, cwd, myname, auditor=None):
     '''return the canonical path of myname, given cwd and root'''
     if util.endswithsep(root):
         rootsep = root
     else: