-
Notifications
You must be signed in to change notification settings - Fork 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
base: develop2
Are you sure you want to change the base?
Changes from all 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 |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
from conans.errors import ConanException, NotFoundException, PackageNotFoundException, \ | ||
RecipeNotFoundException, AuthenticationException, ForbiddenException | ||
from conans.model.package_ref import PkgReference | ||
from conan.internal.paths import EXPORT_SOURCES_TGZ_NAME | ||
from conan.internal.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
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? 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} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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
.tgz
extension and packages produced after then would have.tzst
extension. I would like to avoid producing both because it would result in unnecessary storage usage in Artifactory.