[MERGE][bug #259855] Handle when Transport.stat() returns empty permissions

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Aug 22 10:15:53 BST 2008


>>>>> "john" == John Arbash Meinel <john at arbash-meinel.com> writes:

    john> At the moment the FTP Transport doesn't check
    john> permission bits, but it *does* implement .stat().
    john> The existing code tried to do a stat() and if it raised
    john> NotImplemented it fell back to *not* setting permission
    john> bits. Since FTP now implements stat()

A bit of context here: revno 3514 fixed ftp transport so that it
handles the 'mode' parameter when provided.

Doing so, I failed to realized that revno 3416 (BzrDir controls
file modes, not LockableFiles) had put a new requirement on
transport.stat(): the mode bits are now used to ensure a
consistent scheme across .bzr directory and its children.

I think more transport tests are needed here and the stat()
contract needs to be clarified:

- it provides a way to identify directories vs files (needed by
  listdir()), this is mandatory for writable transports

- it provides a way to acquire chmod bits.

If we make the later mandatory for writable transports, webdav
will never be able to fulfill the contract.


    john> it causes bzr to fall back to the 0700 mode (we force
    john> user rwx because we assume that is always required.)

    john> Anyway, this patch just notices when the stat returns
    john> 0000 for permissions and treats it as a stat failure.

I think that's the right fix since this is the only place I know
of where the chmod bits are needed and the fallback (not
implemented being one case, returning 000 for chmod bits being
the other) should be handled.

I intend to rework the permissions tests as part of bug #260131,
feedback on the above remarks welcome.

Some minor remarks below:

    john> John
    john> =:->


<snip/>
    john> === modified file 'bzrlib/bzrdir.py'
    john> --- bzrlib/bzrdir.py	2008-07-31 08:35:25 +0000
    john> +++ bzrlib/bzrdir.py	2008-08-21 16:12:57 +0000
    john> @@ -637,7 +637,7 @@
 
    john>      def _find_creation_modes(self):
    john>          """Determine the appropriate modes for files and directories.
    john> -        
    john> +
    john>          They're always set to be consistent with the base directory,
    john>          assuming that this transport allows setting modes.
    john>          """
    john> @@ -656,9 +656,14 @@
    john>              # directories and files are read-write for this user. This is
    john>              # mostly a workaround for filesystems which lie about being able to
    john>              # write to a directory (cygwin & win32)
    john> -            self._dir_mode = (st.st_mode & 07777) | 00700
    john> -            # Remove the sticky and execute bits for files
    john> -            self._file_mode = self._dir_mode & ~07111
    john> +            if (st.st_mode & 07777 == 00000):
    john> +                # FTP allows stat but does not return dir/file modes

True and there should be a way to fix it for ftp, but not for webdav.

<snip/>

    john> +    def test_mode_0(self):

How about test_invalid_chmod_bits or test_unavailable_chmod_bits
or test_invalid_permissions ?

BB:tweak

        Vincent



More information about the bazaar mailing list