chown when the file already exists?

[Parth Malwankar]

On Thu, Mar 25, 2010 at 10:37 PM, John Arbash Meinel
<john at arbash-meinel.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> I'm just going to submit this to pqm, but bzr is broken on Windows.
> Specifically the patch:
>
> === modified file 'bzrlib/osutils.py'
> - --- bzrlib/osutils.py   2010-03-11 06:35:52 +0000
> +++ bzrlib/osutils.py   2010-03-25 17:04:08 +0000
> @@ -1812,8 +1812,9 @@
>     If src is None, the containing directory is used as source. If chown
>     fails, the error is ignored and a warning is printed.
>     """
> - -    has_chown = getattr(os, 'chown')
> - -    if has_chown is None: return
> +    chown = getattr(os, 'chown', None)
> +    if chown is None:
> +        return
>
>     if src == None:
>         src = os.path.dirname(dst)
> @@ -1822,7 +1823,7 @@
>
>     try:
>         s = os.stat(src)
> - -        os.chown(dst, s.st_uid, s.st_gid)
> +        chown(dst, s.st_uid, s.st_gid)
>     except OSError, e:
>         trace.warning("Unable to copy ownership from '%s' to '%s':
> IOError: %s." % (src, dst, e))
>
>
> The big problem is "has_chown = getattr()" raises an exception if you
> only use the 2 parameter form. However, this showed another issue.
>
> Why is "bzr status" thinking about calling chown? Digging into it, I see
> it is being called from:
>
> def open_with_ownership()
>
> Which is being called for opening .bzr.log... However, if we aren't
> creating the file, should we be changing its ownership?
>

Hi John,

This issue was introduced by me in the mp for bug #376388 [1].
Missed out on the importance of 'a' :-)

Specifically, in trace.py:
- bzr_log_file = open(_bzr_log_filename, 'at', buffering=0) # unbuffered
+ buffering = 0 # unbuffered
+ bzr_log_file = osutils.open_with_ownership(_bzr_log_filename, 'at', buffering)

I am not sure whats a good way to handle this. We could go back to
using open() and chown only if the log file didn't exist before the open using a
flag to check this.

I could do an mp for this. Please let me know if you have any other suggestions.

Regards,
Parth

[1] https://code.launchpad.net/~parthm/bzr/376388-dot-bazaar-ownership/+merge/19691