[MERGE][bug #259855] Handle when Transport.stat() returns empty permissions
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> === 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> They're always set to be consistent with the base directory,
john> assuming that this transport allows setting modes.
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.
john> + def test_mode_0(self):
How about test_invalid_chmod_bits or test_unavailable_chmod_bits
or test_invalid_permissions ?
More information about the bazaar