Roundup Robot added the comment:
New changeset e5a53d75dc19 by Zachary Ware in branch 'default':
Issue #23193: Skip numeric_owner tests on platforms where they don't make sense
https://hg.python.org/cpython/rev/e5a53d75dc19
--
___
Python tracker
Roundup Robot added the comment:
New changeset 6b70f16d585a by Eric V. Smith in branch 'default':
Issue 23193: Add numeric_owner to tarfile.TarFile.extract() and
tarfile.TarFile.extractall().
https://hg.python.org/cpython/rev/6b70f16d585a
--
nosy: +python-dev
Eric V. Smith added the comment:
Thanks everyone for their help, especially Michael for the original patch.
--
resolution: - fixed
stage: patch review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: -haypo
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23193
___
___
Python-bugs-list
Eric V. Smith added the comment:
Thanks for your review, Berker. I've updated the code with most of your
suggestions, although some of them were mooted by some restructuring I did.
A couple of questions/issues:
- I'm not sure where we stand on keyword-only arguments. I certainly agree that
Eric V. Smith added the comment:
I added numeric_owner to the self.chown() call when adding directories. I'm
reasonably sure this is correct.
I added tests for dirs, although they need some cleaning up to be simpler and
cleaner. I'll do that cleanup shortly, but I want to check this in before
Eric V. Smith added the comment:
Other than Misc/NEWS, I think this is the final version of this patch.
--
Added file:
http://bugs.python.org/file38981/tarfile-numeric-owner-with-tests-3.diff
___
Python tracker rep...@bugs.python.org
Berker Peksag added the comment:
* +.. method:: TarFile.extractall(path=., members=None, numeric_owner=False)
numeric_owner can be a keyword-only argument.
* TarFile.extract and TarFile.extractall docs need a versionchanged directive.
* It would be nice to add an entry to
Eric V. Smith added the comment:
Updated patch with a few minor doc tweaks.
The one substantive change I did make was to add numeric_owner to the call to
self.chown() when setting directory owners. I believe this is correct, but I
need to convince myself and to write a test.
--
Added
Eric V. Smith added the comment:
Note that this change will break code that subclasses TarFile and overrides
chown(), as suggested in msg233915. I'm not too concerned about that, since
chown() is not documented. Ideally it would be renamed to _chown(), but that's
probably a separate issue.
Changes by Eric V. Smith e...@trueblade.com:
--
stage: test needed - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23193
___
___
Michael Vogt added the comment:
Thanks everyone for the comments and feedback!
Attached is a updated patch with tests and a documentation update.
Feedback is very welcome. I decided to skip the test on systems where root is
not uid,gid=0. I could also mock that of course if you prefer it
Lars Gustäbel added the comment:
I would argue that a serious alternative to this patch is to simply override
the TarFile.chown() method in a subclass. However, I'm not sure if this expects
too much of the user.
--
___
Python tracker
Eric V. Smith added the comment:
Ignore my review comment on pwd and grp being None. I see that there is a test
for it (at least grp), and they're not available on Windows.
--
___
Python tracker rep...@bugs.python.org
Changes by Eric V. Smith e...@trueblade.com:
--
assignee: - eric.smith
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23193
___
___
R. David Murray added the comment:
If it weren't for the fact that this feature is something that the tar command
provides, I'd agree (the chown method is relatively short). However, since tar
*does* provide this feature, it seems reasonable for us to support it as well.
Call me +0.5 :)
Eric V. Smith added the comment:
I don't think we want to encourage the type of coupling that arises from
subclassing, especially when when overriding an undocumented method. I'm +1 on
the change. I'll review the patch. Michael: can you write the tests, and
hopefully docs?
--
stage:
Eric V. Smith added the comment:
I think Michael is asking if the proposed change would ever be accepted. If the
answer is no, not even if you write the tests and update the documentation,
then there's no sense putting the work into this. That seems like a reasonable
question to me.
I think
R. David Murray added the comment:
I concur that this is a reasonable feature request, and it is not one that can
be satisfied without modifying the tarfile module (that is, you can't write a
simple wrapper to tarfile to get the functionality desired without cutting and
pasting the entire
Berker Peksag added the comment:
The patch also needs documentation update for TarFile.extract():
https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extract
The tarfile documentation is located at Doc/library/tarfile.rst.
--
nosy: +berker.peksag
Changes by Serhiy Storchaka storch...@gmail.com:
--
nosy: +lars.gustaebel, serhiy.storchaka
stage: - patch review
versions: +Python 3.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23193
STINNER Victor added the comment:
(...) if there is a chance that this patch goes in I'm happy to write the
required test (mocking os.chown()) for this to go in.
We don't accept changes without test. So you must write a test.
Implementing the feature in Python makes sense.
--
nosy:
New submission from Michael Vogt:
Please consider adding a option to extract a tarfile with the uid/gid instead
of the lookup for uname/gname in the tarheader (i.e. what tar --numeric-owner
provides).
One use-case is if you unpack a chroot tarfile that contains a
/etc/{passwd,group} file
23 matches
Mail list logo
