-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for zstd compression #14706
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
Changes from 5 commits
278ea02
682b0e3
396815c
f0b7813
a33394d
bbed1a0
ea5d948
db87f56
6a109f4
e5765e6
ff29efc
0c58aa8
79afdae
890c454
b8f4a57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| import os | ||
| import shutil | ||
| import tarfile | ||
| import time | ||
| import zstandard | ||
| from typing import List | ||
|
|
||
| from requests.exceptions import ConnectionError | ||
|
|
@@ -14,7 +17,7 @@ | |
| from conans.model.package_ref import PkgReference | ||
| from conans.model.recipe_ref import RecipeReference | ||
| from conans.util.files import rmdir, human_size | ||
| from conans.paths import EXPORT_SOURCES_TGZ_NAME, EXPORT_TGZ_NAME, PACKAGE_TGZ_NAME | ||
| from conans.paths import EXPORT_SOURCES_TGZ_NAME, EXPORT_TGZ_NAME, PACKAGE_TGZ_NAME, PACKAGE_TZSTD_NAME | ||
| from conans.util.files import mkdir, tar_extract | ||
|
|
||
|
|
||
|
|
@@ -148,14 +151,21 @@ def _get_package(self, layout, pref, remote, scoped_output, metadata): | |
| metadata, only_metadata=False) | ||
| zipped_files = {k: v for k, v in zipped_files.items() if not k.startswith(METADATA)} | ||
| # quick server package integrity check: | ||
| for f in ("conaninfo.txt", "conanmanifest.txt", "conan_package.tgz"): | ||
| for f in ("conaninfo.txt", "conanmanifest.txt"): | ||
| if f not in zipped_files: | ||
| raise ConanException(f"Corrupted {pref} in '{remote.name}' remote: no {f}") | ||
| accepted_package_files = [PACKAGE_TZSTD_NAME, PACKAGE_TGZ_NAME] | ||
| package_file = next((f for f in zipped_files if f in accepted_package_files), None) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically, a package could contain both compressed artifacts, but it will prioritize and only download the zstd one if existing? Wouldn't it be a bit less confusing to not allow to have both compressed formats artifacts in the same package?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A package is only supposed to contain one. Let's say an organization switches to zstd compression on Jan 1 2025. The expectation would be that packages produced before then would have |
||
| if not package_file: | ||
| raise ConanException(f"Corrupted {pref} in '{remote.name}' remote: no {accepted_package_files} found") | ||
| self._signer.verify(pref, download_pkg_folder, zipped_files) | ||
|
|
||
| tgz_file = zipped_files.pop(PACKAGE_TGZ_NAME, None) | ||
| package_file = zipped_files.pop(package_file, None) | ||
| package_folder = layout.package() | ||
| uncompress_file(tgz_file, package_folder, scope=str(pref.ref)) | ||
| t1 = time.time() | ||
| uncompress_file(package_file, package_folder, scope=str(pref.ref)) | ||
| duration = time.time() - t1 | ||
| scoped_output.debug(f"Decompressed {package_file} in {duration} seconds") | ||
| mkdir(package_folder) # Just in case it doesn't exist, because uncompress did nothing | ||
| for file_name, file_path in zipped_files.items(): # copy CONANINFO and CONANMANIFEST | ||
| shutil.move(file_path, os.path.join(package_folder, file_name)) | ||
|
|
@@ -255,8 +265,15 @@ def uncompress_file(src_path, dest_folder, scope=None): | |
| if big_file: | ||
| hs = human_size(filesize) | ||
| ConanOutput(scope=scope).info(f"Decompressing {hs} {os.path.basename(src_path)}") | ||
|
|
||
| with open(src_path, mode='rb') as file_handler: | ||
| tar_extract(file_handler, dest_folder) | ||
| if src_path.endswith(".tar.zst"): | ||
| dctx = zstandard.ZstdDecompressor() | ||
|
grossag marked this conversation as resolved.
Outdated
|
||
| stream_reader = dctx.stream_reader(file_handler) | ||
| with tarfile.open(fileobj=stream_reader, mode='r|') as the_tar: | ||
| the_tar.extractall(dest_folder) | ||
| else: | ||
| tar_extract(file_handler, dest_folder) | ||
| except Exception as e: | ||
| error_msg = "Error while extracting downloaded file '%s' to %s\n%s\n"\ | ||
| % (src_path, dest_folder, str(e)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
| from conans.errors import ConanException, NotFoundException, PackageNotFoundException, \ | ||
| RecipeNotFoundException, AuthenticationException, ForbiddenException | ||
| from conans.model.package_ref import PkgReference | ||
| from conans.paths import EXPORT_SOURCES_TGZ_NAME | ||
| from conans.paths import EXPORT_SOURCES_TGZ_NAME, PACKAGE_TGZ_NAME, PACKAGE_TZSTD_NAME | ||
| from conans.util.dates import from_iso8601_to_timestamp | ||
| from conans.util.thread import ExceptionThread | ||
|
|
||
|
|
@@ -81,8 +81,12 @@ def get_package(self, pref, dest_folder, metadata, only_metadata): | |
| result = {} | ||
| # Download only known files, but not metadata (except sign) | ||
| if not only_metadata: # Retrieve package first, then metadata | ||
| accepted_files = ["conaninfo.txt", "conan_package.tgz", "conanmanifest.txt", | ||
| "metadata/sign"] | ||
| accepted_package_files = [PACKAGE_TZSTD_NAME, PACKAGE_TGZ_NAME] | ||
| accepted_files = ["conaninfo.txt", "conanmanifest.txt", "metadata/sign"] | ||
| for f in accepted_package_files: | ||
| if f in server_files: | ||
| accepted_files = [f] + accepted_files | ||
| break | ||
|
Comment on lines
+84
to
+89
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we assumed there can only be 1 compressed artifact in one of the formats, this would be simplified?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I think I'm missing what you are saying here. I don't have the context about if/how these accepted files changed over time. But Artifactory would only have .tgz or .tzst, not both. If that means we can simplify this a bit, that's fine with me. |
||
| files = [f for f in server_files if any(f.startswith(m) for m in accepted_files)] | ||
| # If we didn't indicated reference, server got the latest, use absolute now, it's safer | ||
| urls = {fn: self.router.package_file(pref, fn) for fn in files} | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.