[merge] BzrDir controls file modes, not LockableFiles

[Ian Clatworthy]

Martin Pool wrote:

bb:tweak

> +  API CHANGES:
> +
> +    * The default permissions for creating new files and directories 
> +      should now be obtained from ``BzrDir._get_file_mode()`` and 
> +      ``_get_dir_mode()``, rather than from LockableFiles.
> +      (Martin Pool)
>  
>  IN DEVELOPMENT
>  --------------

The API CHANGES section needs to be put inside IN DEVELOPMENT,
not above it.


> @@ -70,13 +73,17 @@
>          os.mkdir('c')
>          os.chmod('c', 02750)
>          b = self.make_branch('c')
> -        self.assertEqualMode(02750, b.control_files._dir_mode)
> +        self.assertEqualMode(02750, b.bzrdir._get_dir_mode())
> +        self.assertEqualMode(02750, b.bzrdir._get_dir_mode())
> +        self.assertEqualMode(00640, b.control_files._file_mode)
>          self.assertEqualMode(00640, b.control_files._file_mode)

This change to the tests isn't right. You want to be testing
_dir_mode, _get_dir_mode, _filemode and _get_file_mode. Currently
you're testing only two of those and doing each one twice. :-)

> There is one open issue: at the moment LockableFiles has this:
> 
>     # If set to False (by a plugin, etc) BzrBranch will not set the
>     # mode on created files or directories
>     _set_file_mode = True
>     _set_dir_mode = True
> 
> I haven't duplicated this functionality, and I'm not sure it is still
> needed.  Does anyone actually use such a plugin?  If we want the option
> for users to turn off mode setting or force a particular mode maybe that
> would be better controlled by a config setting?

I'd suggest using annotate to find out when these lines were added.
If early on, they may well be yagni. If added explicitly later, you'd
imagine their author put them in for a particular reason.

Ian C.