Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.10] gh-102950: Implement PEP 706 – Filter for tarfile.extractall (GH-102953) #104128

Merged
merged 7 commits into from
May 10, 2023

Conversation

mcepl
Copy link
Contributor

@mcepl mcepl commented May 3, 2023

Manual port of gh-102953 to 3.10.

Please, review.

@encukou
Copy link
Member

encukou commented May 3, 2023

Thanks for getting to this sooner than I did!
But see https://github.com/python/cpython/pull/103832/commits for the changes needed in the backport.

@mcepl
Copy link
Contributor Author

mcepl commented May 3, 2023

I was basing it on this 3.11 PR, I know better than using master branch for this. But yes, I will re-check, that everything in that PR has been included.

@encukou
Copy link
Member

encukou commented May 3, 2023

Ah, sorry! I just looked at the versionchanged:: 3.12 and gave a quick comment. Will do a proper review later.

@mcepl
Copy link
Contributor Author

mcepl commented May 3, 2023

Yes, you were right, I missed those, not sure how I have managed to do that.

@mcepl
Copy link
Contributor Author

mcepl commented May 3, 2023

I don't understand those Windows failures … shouldn’t it be covered by ae62b3a ?

Doc/library/shutil.rst Outdated Show resolved Hide resolved
@encukou
Copy link
Member

encukou commented May 4, 2023

No. ae62b3a is for Solaris or BSD, supported platforms (per PEP 11) don't need it.

This change made me get a Windows VM, so I'll investigate there. Can't promise it'll be this week though.

@encukou
Copy link
Member

encukou commented May 9, 2023

It's related to a change made while backporting, going from

        if mode is not None and os_helper.can_chmod():

to just

        if mode is not None:

The can_chmod() determines whether chmod can set all the mode bits. According to the docs, it's only limited on Windows, so I'll hardcode that. (It's also limited on WASI, but in 3.10 tests we don't care.)

I can't push fixes to your repo so I opened #104327.

@mcepl
Copy link
Contributor Author

mcepl commented May 9, 2023

I can't push fixes to your repo so I opened #104327.

Sorry about that, I don’t know how to set it up with the existing PR.

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 9, 2023
@mcepl
Copy link
Contributor Author

mcepl commented May 9, 2023

Couldn’t you just make PR to https://github.com/openSUSE-Python/cpython/tree/PEP706-for-py310 ?

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 7ab6233 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 9, 2023
@encukou
Copy link
Member

encukou commented May 9, 2023

I don't think I could run CPython buildbots on another repo.
But since you've updated this PR quickly, I'll run them here.

@encukou
Copy link
Member

encukou commented May 10, 2023

The red checks are unrelated: Python 3.10 doesn't build on Wasm, and RtD's OpenSSL is too old for urllib3.

@encukou encukou merged commit 425065b into python:3.10 May 10, 2023
@encukou
Copy link
Member

encukou commented May 10, 2023

Thanks for the backport!

@mcepl mcepl deleted the PEP706-for-py310 branch May 10, 2023 12:27
@mcepl
Copy link
Contributor Author

mcepl commented May 10, 2023

So, now 3.9! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants