-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Compression plugin #18314
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
Compression plugin #18314
Changes from 27 commits
4cd2811
156e036
c7b5dff
be30ba2
1295d4f
cc4d3ad
e245a93
f094d9a
fee5fab
452f774
c1a7320
a240c8c
cf7de74
557f8e0
f82d764
549e086
ac1fc45
181a736
3b32ec2
b52bce2
91da7a4
e96bd3c
a024baf
989fde9
f986651
8009bed
0db1a7b
dcbb29e
28e9148
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,4 +1,6 @@ | ||||||
| import errno | ||||||
| from pathlib import Path | ||||||
| import tempfile | ||||||
| import gzip | ||||||
| import hashlib | ||||||
| import os | ||||||
|
|
@@ -11,10 +13,13 @@ | |||||
|
|
||||||
| from contextlib import contextmanager | ||||||
|
|
||||||
| from conan.api.output import ConanOutput | ||||||
| from conan.errors import ConanException | ||||||
|
|
||||||
| _DIRTY_FOLDER = ".dirty" | ||||||
|
|
||||||
| # Name (without extension) of the tar file to be created by the compression plugin | ||||||
| COMPRESSED_PLUGIN_TAR_NAME = "__conan_plugin_compressed_contents__" | ||||||
|
|
||||||
| def set_dirty(folder): | ||||||
| dirty_file = os.path.normpath(folder) + _DIRTY_FOLDER | ||||||
|
|
@@ -256,15 +261,46 @@ def mkdir(path): | |||||
| os.makedirs(path) | ||||||
|
|
||||||
|
|
||||||
| def tar_extract(fileobj, destination_dir): | ||||||
| def tar_extract(fileobj, destination_dir, compression_plugin=None, conf=None): | ||||||
| if compression_plugin: | ||||||
| _tar_extract_with_plugin(fileobj, destination_dir, compression_plugin, conf) | ||||||
| return | ||||||
|
|
||||||
| the_tar = tarfile.open(fileobj=fileobj) | ||||||
| # NOTE: The errorlevel=2 has been removed because it was failing in Win10, it didn't allow to | ||||||
| # "could not change modification time", with time=0 | ||||||
| # the_tar.errorlevel = 2 # raise exception if any error | ||||||
| the_tar.extraction_filter = (lambda member, path: member) # fully_trusted, avoid Py3.14 break | ||||||
| the_tar.extractall(path=destination_dir) | ||||||
| the_tar.close() | ||||||
|
|
||||||
| if list(Path(destination_dir).glob(f"{COMPRESSED_PLUGIN_TAR_NAME}.*")): | ||||||
| raise ConanException(f"Error while extracting {os.path.basename(fileobj.name)}.\n" | ||||||
| "This file has been compressed using a `compression` plugin.\n" | ||||||
| "If your organization uses this plugin, ensure it is correctly installed on your environment.") | ||||||
|
|
||||||
| def _tar_extract_with_plugin(fileobj, destination_dir, compression_plugin, conf): | ||||||
| """First remove tar.gz wrapper and then call the plugin to extract""" | ||||||
| with tempfile.TemporaryDirectory() as temp_dir: | ||||||
|
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. Conan doesn't really use system TemporaryDirectories for anything, it tries to do everything in the Conan cache, in cache folders, so if something goes wrong the files are there, and also there is no "leakage" of pacakges, recipes or code in anywhere else beside the Conan cache, that can be easily wiped.
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. Okay, I didn't like the idea of messing up the conan cache. Do we have temporal folder in our conan cache? conan/conan/api/subapi/cache.py Line 170 in f986651
(do not look at the blame, it was me!) but this case is different as it is only a pckglist extraction + removal. I could move it also to the conan cache and may consider creating a
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. Yes, I understand the concern. |
||||||
| t1 = time.time() | ||||||
| the_tar = tarfile.open(fileobj=fileobj) | ||||||
| the_tar.extraction_filter = (lambda member, path: member) # fully_trusted, avoid Py3.14 break | ||||||
| the_tar.extractall(path=temp_dir) | ||||||
| extracted_file = the_tar.getnames()[0] | ||||||
| the_tar.close() | ||||||
| # Check if the tar was compressed with the compression plugin by checking the existence of | ||||||
| # our constant COMPRESSED_PLUGIN_TAR_NAME (without extension as extension is added by the plugin) | ||||||
| if list(Path(temp_dir).glob(f"{COMPRESSED_PLUGIN_TAR_NAME}.*")): | ||||||
| # Get the only extracted file: the plugin tar | ||||||
|
Contributor
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. In reference to #18259, we'd eventually like to have our own additional files added to the top-level. Don't assume here that it's the only file. I don't think this requires anything other than clarifying the comment.
Suggested change
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. Greetings @mrjoel, With this incoming compression plugin you could do whatever you need with the files. The thing is that, to avoid issues with the compressed file extension (which could be whatever the user decides), we have decided to wrap that compressed result file into a constant I'll put an example. Without the plugin: With the plugin, using standard: As you can see, when decompressing the Answering your suggestion: there will always be one file, the unwrapped compressed file from the plugin!
Contributor
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. @perseoGI Yep, I'm tracking all of that from the diff. My main concern was just including the assumption and comment that the outer wrapper would only contain a single file. For #18259 we'd really like it to end up being the following, where we add the custom additional files. We don't expect Conan to know or do anything about it, but do want it to not cause issues. I'm slightly sad that custom files in the direct wrapper won't be compressed at all, but so be it, we mainly expect files of a few hundred KBs (the HTML graph reports). As a separate question, why
Contributor
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. Further, this approach of requiring the compression plugin to be enabled in a user's Conan home means that it is considered for all projects they may be working. We'd like to be able to provide our conan package archives in a non-interference way, not requiring users of our archive to have to use our compression plugin for everything. Would you consider an approach where an embedded |
||||||
| plugin_tar_path = os.path.join(temp_dir, extracted_file) | ||||||
| ConanOutput().debug(f"Unwrapped in {time.time() - t1} time") | ||||||
| t1 = time.time() | ||||||
| compression_plugin.tar_extract(archive_path=plugin_tar_path, dest_dir=destination_dir, conf=conf) | ||||||
| ConanOutput().debug(f"Extracted in {time.time() - t1} time on plugin") | ||||||
| else: | ||||||
| # The tar was not compressed using the plugin, copy files to destination | ||||||
| from conan.tools.files import copy | ||||||
| ConanOutput().debug(f"Extracted in {time.time() - t1} time built in") | ||||||
| copy(None, pattern="*", src=temp_dir, dst=destination_dir) | ||||||
|
perseoGI marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| def merge_directories(src, dst): | ||||||
| from conan.tools.files import copy | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -158,9 +158,13 @@ def test_cache_save_excluded_folders(): | |||||
|
|
||||||
| # exclude source | ||||||
| c.run("cache save * --no-source") | ||||||
| # Check default compression function is being used and not compression.py plugin one | ||||||
| assert "Compressing conan_cache_save.tgz\n" in c.out | ||||||
|
||||||
| assert "Compressing conan_cache_save.tgz\n" in c.out | |
| assert "Compressing conan_cache_save.tgz" in c.out |
Uh oh!
There was an error while loading. Please reload this page.