[merge] BzrDir controls file modes, not LockableFiles

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
| 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?
|

It is probably better as a config setting. Someone was asking for it at the
time. I don't specifically remember what, but I *do* remember writing the
functionality. It's going to be *way* back in the archive though. I'm thinking
2-3 years.

| 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.
|


I would guess it is yagni at this point. There was a purpose, maybe to work
around the problems with openssh, etc.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkgzg70ACgkQJdeBCYSNAAPHOwCfUalbDtuKEZH13XHIzEoNNiOr
pN4An0FDlLEKePADuJbQc+h12SIVw4Aq
=f0Si
-----END PGP SIGNATURE-----