[PATCH 6 of 6] vfs: also audit rename

Boris FELD boris.feld at octobus.net
Sun Dec 2 15:30:28 UTC 2018


On 28/11/2018 13:04, Yuya Nishihara wrote:
> On Mon, 26 Nov 2018 19:22:48 +0100, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.feld at octobus.net>
>> # Date 1498961267 -7200
>> #      Sun Jul 02 04:07:47 2017 +0200
>> # Node ID afdc73b20bd1faeec1b278ef0a2ab8d1dda71eb8
>> # Parent  cc7d970d99eb242dbe2d8e792a9212aabd06911f
>> # EXP-Topic vfs.audit-rename
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r afdc73b20bd1
>> vfs: also audit rename
> Queued the first 4 patches, thanks.
>
>> diff --git a/mercurial/vfs.py b/mercurial/vfs.py
>> --- a/mercurial/vfs.py
>> +++ b/mercurial/vfs.py
>> @@ -436,6 +436,10 @@ class vfs(abstractvfs):
>>  
>>          return fp
>>  
>> +    def rename(self, src, dst, *args, **kwargs):
>> +        self._auditpath(dst, 'w')
>> +        return super(vfs, self).rename(src, dst, *args, **kwargs)
> Can you move this to abstractvfs.rename()?
>
> I don't think it's worth to override rename() just for _auditpath(). Instead,
> abstractvfs can implement _auditpath() that raises exception.

I'm not sure what are your concerns and what would be the point of
moving that one layer up. Currently, the _auditpath logic is exclusive
to vfs. It comes from the `vfs.__call__` method that the `abstractvfs`
class do not have. What makes it worthy to move it in `abstractvfs`?

(I'm not opposed to doing so, I'm trying to understand the logic and
responsibility of each class)
>
> Also, 'dst' is likely to be relative in abstractvfs.rename(), so we won't
> probably need to convert absolute 'dst' back to relative path.
What's your reasoning here? We directly call the underlying `rename`
method in the `vfs` class, and we know for sure that they can receive
absolute paths.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list