gh-59999: Add option to preserve permissions in ZipFile.extract

[dignissimus]

Co-authored by Alexey Boriskin

https://bugs.python.org/issue15795

TODO

• Documentation
  • Document constants
  • Document permission preservation parameters
  • Document new behaviour for archives created on windows
• Testing
  • Get current tests to pass
    • Work out why extracted files have mode 777
  • Add more tests for the other stat flags
    • Explicitly state file type by applying file-type masks
    • Re-add tests for files with mode 0
  • Check the permissions of extracted files against the umask
  • Extract test archive contents to a test directory
• Enumerations
  • Replace constants with enumerations
  • Document enumerations

[merwok]

If these were turned into an enum, it could be:

class PreserveMode(enum.Enum):
    NONE = auto()
    SAFE = auto()
    ALL = auto()

which would look ok I think (name not too long and not too generic/short/cryptic):

thing = myzip.extract(member, preserve_permissions=PreserveMode.SAFE)

But I would like a second opinion from a core dev!

[sharkwouter]

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.

[dignissimus]

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

[sharkwouter]

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

[MaxwellDupre]

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)

[dignissimus]

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

[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.

[dignissimus]

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.

[dignissimus]

Hopefully tests should pass on your end now.
@MaxwellDupre

[MaxwellDupre]

Ran 277 tests in 89.415s
OK (skipped=3)
Ok now.

[ugomancz]

Any news on this PR? It's been a while.