Code review comment for lp:~parthm/bzr/no-chown-if-bzrlog-exists

Revision history for this message
Martin Pool (mbp) wrote :

+ """Create a log file in while avoiding race.

some kind of typo here.

I would make the docstring be:

  Create the .bzr.log file.

  It inherits the ownership and permissions (masked by umask) from the containing
  directory to cope better with being run under sudo with $HOME still set to the
  user's homedir.

Also the fact that you have a comment next to every copy_ownership() invocation suggests it should be renamed to copy_ownership_from_directory.

To set the permissions you need to pass a third parameter to os.open(), which here should be 0666. This is separate from the O_APPEND etc mode.

Thanks!

review: Needs Fixing

« Back to merge proposal