Thanks for making this. I didn't really get back to my PR after it look a year to be reviewed.

What I did differently is that I made preserving the permissions the default. The reason for this is that the unzip command which comes with any Linux distribution, MacOS or BSD system will preserve permissions by default as well. So the user will probably expect that to be the default.

Thanks for making this. I didn't really get back to my PR after it look a year to be reviewed.

It's no problem

What I did differently is that I made preserving the permissions the default. The reason for this is that the unzip command which comes with any Linux distribution, MacOS or BSD system will preserve permissions by default as well. So the user will probably expect that to be the default.

On the BPO issue (#59999) I proposed not doing this to maintain backwards compatibility

Makes some sense, thanks for clarifying. Keep up the good work!

Running test_zipfile I get two errors:
FAIL: test_extract_preserve_none (test.test_zipfile.TestsPermissionExtraction.test_extract_preserve_none)

Traceback (most recent call last):
File "/home/dougal/Documents/GitHub/cpython/Lib/test/test_zipfile.py", line 2099, in test_extract_preserve_none
self.assertEqual(os.stat(filename).st_mode,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 33204 != 33188

FAIL: test_extractall_preserve_none (test.test_zipfile.TestsPermissionExtraction.test_extractall_preserve_none)
Traceback (most recent call last):
File "/home/dougal/Documents/GitHub/cpython/Lib/test/test_zipfile.py", line 2090, in test_extractall_preserve_none
self.assertEqual(os.stat(filename).st_mode,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 33204 != 33188

Ran 274 tests in 94.070s
FAILED (failures=2, skipped=3)

Ok, interesting. What OS and file system are you running this on? And, do the other test_extract tests pass, or were some of them skipped? And, do tests still fail on 6470201?

@MaxwellDupre

I am running Fedora 35. I did a clean start (compiling python from scratch just in case) and ran the test again and got the same result.
Ran 274 tests in 78.664s
FAILED (failures=2, skipped=3)

Other test_extract tests are ok ( except
test_extract_hackers_arcnames_windows_only (test.test_zipfile.ExtractTests.test_extract_hackers_arcnames_windows_only) ... skipped 'Requires \ as path separator.'
)

I don't know what you mean by 6470201, I didn't see any tests there.

I don't know what you mean by 6470201, I didn't see any tests there.

Oh of course, they didn't exist in that revision.

I've just checked and the default mode for created files differs on Fedora. On CI and my system it's 644 on Fedora it seems to be 664. I'll write the test so it checks against what the default for the system is instead of the 0o644 constant.

(Please follow the devguide: avoid force pushes, they create notifications with broken links for reviewers, and make it harder to see changes compared to previous time. Thanks!)

Seems another look is needed from a reviewer?

This PR is stale because it has been open for 30 days with no activity.