gh-102950: Implement PEP 706 – Filter for tarfile.extractall #102953
Merged
+1,786
−99
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
ethanfurman
requested changes
Mar 23, 2023
|
When you're done making the requested changes, leave the comment: |
eryksun
reviewed
Mar 23, 2023
|
@gpshead, did you want to take a look? |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ethanfurman: please review the changes made to this pull request. |
hroncok
reviewed
Apr 4, 2023
Doc/library/tarfile.rst
Outdated
|
|
||
| .. versionadded:: 3.12 | ||
|
|
||
| The *tar* format is designed to capture all details of a UNIX-like ecosystem, |
There was a problem hiding this comment.
Suggested change
| The *tar* format is designed to capture all details of a UNIX-like ecosystem, | |
| The *tar* format is designed to capture all details of a UNIX-like filesystem, |
There was a problem hiding this comment.
I also tend to say POSIX rather than UNIX at this point.
There was a problem hiding this comment.
POSIX is more precise, but this intentionally hand-wavy terminology: I think UNIX-like is more understandable to more people than POSIX-like.
gpshead
reviewed
Apr 5, 2023
|
If there are no objections, I plan to merge this around Wednesday. Please let me know if you're working on a review. |
hroncok
reviewed
Apr 17, 2023
Lib/tarfile.py
Outdated
Comment on lines
2309
to
2311
| except TarError as e: | ||
| if self.errorlevel > 0: | ||
| raise |
There was a problem hiding this comment.
After reading the docs, I thought only OSErrors will be raised with errorlevel == 1. This seems to raise even TarErrors with errorlevel 1 or 2. What do I miss?
There was a problem hiding this comment.
Oh dear. What a rabbit hole. Thanks for raising it, and for a very helpful private discussion.
This warrants an addition to the PEP: https://discuss.python.org/t/23149/26
Please voice your concerns there!
I've updated this PR with my preferred solution, so you can see the areas that need to change and so I get a CI check. It doesn't mean the discussion is over, of course.
bedevere-bot
removed
the
|
|
On WASI, all symlink tests are skipped for now. The mechanism to skip them is a bit of a shortcut that doesn't work here, so on WASI I'll skip the tarfile symlink tests explicitly. |
|
And aarch64 Debian clang showed an issue in tests: I assumed |
encukou
added
the
bedevere-bot
removed
the
|
The remaining buildbot failures are unrelated and happened on |
|
Hi, with this integrated, I see one new test failure on Solaris: The issue seems to be the fact that setting sticky bit on files as regular user doesn't do anything on Solaris, and from my limited testing, the same seems to be true on some other systems as well (I tried FreeBSD and |
|
Thanks for the report! The tests are intentionally pretty strict. I'll relax this one. (Edit: see #103831) |
encukou
added a commit
to encukou/cpython
that referenced
this pull request
Apr 25, 2023
encukou
added a commit
that referenced
this pull request
Apr 28, 2023
…H-102953) (GH-103832) See [Backporting & Forward Compatibility in PEP 706](https://peps.python.org/pep-0706/#backporting-forward-compatibility). - Backport b52ad18 - Backport c8c3956 - Remove the DeprecationWarning - Adjust docs - Remove new `__all__` entries
|
GH-104128 is a backport of this pull request to the 3.10 branch. |
|
GH-104327 is a backport of this pull request to the 3.10 branch. |
encukou
added a commit
that referenced
this pull request
May 10, 2023
…H-102953) (GH-104128) - Backport b52ad18 - Backport c8c3956 - Remove the DeprecationWarning - Adjust docs - Remove new `__all__` entries Co-authored-by: Petr Viktorin <[email protected]>
|
GH-104382 is a backport of this pull request to the 3.9 branch. |
ambv
pushed a commit
that referenced
this pull request
May 15, 2023
|
|
|
A |
|
GH-104548 is a backport of this pull request to the 3.8 branch. |
ambv
pushed a commit
that referenced
this pull request
May 17, 2023
|
GH-104583 is a backport of this pull request to the 3.7 branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
See PEP-706 for details.
I might have overengineered the tests a bit, moving boilerplate to a set of helpers. Hopefully the tests themselves are clear enough to make it worth it.