Currently shutils chown still can't do a recursive chown. It would be nice to have this instead of having to do the looping dance we put our selves through each time we need recursion. Ruby's FileUtils already have this.

See also issue 12191, where there was a brief discussion of this.

Wasn’t there a python-ideas discussion on this?  If someone could find a link and summarize use cases it would be great.

http://mail.python.org/pipermail/python-dev/2011-May/111661.html

Following Nick’s opinion in the thread, I’d prefer a distinct function.

First time posting, how about something like this. Just added a os.walk method to do the chown recursively.

See also #13229.

Tigger: Could you provide a patch for Python 3.3? (more info at http://docs.python.org/devguide/)

Hi Éric,

Apologies for the late response, attached is the patch. Let me know if
I need any changes or anything else.

Best,
Tigger

On Sat, Nov 5, 2011 at 11:01 AM, Éric Araujo <report@bugs.python.org> wrote:
>
> Éric Araujo <merwok@netwok.org> added the comment:
>
> Tigger: Could you provide a patch for Python 3.3? (more info at http://docs.python.org/devguide/)
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue13033>
> _______________________________________

Thanks for the patch.  Before you do any more work, do other core developers agree that this function is a good addition or is it obsoleted by the generic improved-walk-with-callback that Nick is working on?

Doc/library/shutil.rst:

I don’t think the note directives are needed.  The doc is not big, I assume people will read all of it and see the caveats.  (I’ll also want to group some small paragraphs.)

Lib/shutil.py:

+def chowntree(path, user=None, group=None, followlinks=False):
+ [...]
+    The dictionary _modified_items, will keep track of the old ownership details,
What _modified_items dictionary?

Apart from a few stylistic violations which can be fixed by the committer, the function looks good.

Lib/test/test_shutil.py:

Looks good and needs more tests.  Currently it only calls chowntree on a directory without children, so it does not test that the chown is indeed recursive.

I believe the current "check_chown" could be passed by a no-op (since the file will be owned by the current user even *before* the call to chowntree). Testing this properly is actually rather difficult (since the only uid and gid we can rely on are those of the current process).

More significantly, I don't agree with the proposed error handling (i.e. attempting to roll back to the original state). Instead, I think it would be more appropriate to follow the rmtree ignore_errors/onerror style so that uses can either unconditionally ignore errors (including dir listing errors) or else tailor the error handling themselves. Any custom error handling should also cover the actual chown operation, not just directory listing errors inside os.walk.

I think, like walkdir itself, there's enough under the hood here that the idea requires some baking time outside the standard library. How do you feel about migrating this discussion over to the walkdir issue tracker as a higher level API proposal there? (https://bitbucket.org/ncoghlan/walkdir/issues).

I had a couple of other minor comments, although they're largely irrelevant given the more significant comments above:

There's a gratuitous inconsistency in the type-checking for uid/gid (one uses "isinstance(uid, str)", the other "not isinstance(gid, int)". Neither is a particular good check for the "None, integer or string" case anyway. It would be better to just try the following in order:

- "is None"
- operator.index
- _get_uid/gid (as appropriate)

The dict initialisation and error handler definition may as well move inside the if statement.