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.

Sign up for GitHub

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

Jump to bottom

Extract .zip files with recorded file timestamps (#15268) #15269

Open
wants to merge 1 commit into
base: develop2
Choose a base branch
from
Open

Extract .zip files with recorded file timestamps (#15268) #15269

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion conan/tools/files/files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import shutil
import subprocess
import sys
import time
from contextlib import contextmanager
from fnmatch import fnmatch
from shutil import which
Expand Down Expand Up @@ -341,18 +342,24 @@ def print_progress(_, __):
except Exception as e:
output.error(f"Error extract {file_.filename}\n{str(e)}", error_type="exception")
else: # duplicated for, to avoid a platform check for each zipped file
file_timestamps = []
for file_ in zip_info:
extracted_size += file_.file_size
print_progress(extracted_size, uncompress_size)
try:
z.extract(file_, full_path)
file_path = os.path.join(full_path, file_.filename)
ts = time.mktime(file_.date_time + (0, 0, -1))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this 0, 0, -1? This would need some comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_.date_time provides six timestamp fields, but time.mktime() needs three additional fields to make a struct_time.

I can add a comment to this effect.

file_timestamps.append((file_path, ts))
if keep_permissions:
# Could be dangerous if the ZIP has been created in a non nix system
# https://bugs.python.org/issue15795
perm = file_.external_attr >> 16 & 0xFFF
os.chmod(os.path.join(full_path, file_.filename), perm)
os.chmod(file_path, perm)
except Exception as e:
output.error(f"Error extract {file_.filename}\n{str(e)}", error_type="exception")
for file_path, ts in file_timestamps:
os.utime(file_path, (ts, ts))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the z.extract() will not automatically handle the zip timestamp? It doesn't make much sense that this has to be handled manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beats me. Seems like quite the oversight for the API.

Apparently not even .extractall() gets you that.

output.writeln("")


Expand Down