Replace archiver dependency #2564
Conversation
As github.com/mholt/archiver has been archived, this dependency has been replaced by github.com/mholt/archives as suggested in its README.
| OverwriteExisting: true, | ||
| ImplicitTopLevelFolder: false, | ||
| } | ||
| func Zip(ctx context.Context, sourcePath, destinationFile string) error { |
There was a problem hiding this comment.
If it is added the context as parameter, it requires to update different functions in the base code.
The main issue is that elastic-package check has some difference and it does not have any context. So, it requires to create a new one if that happens:
https://github.com/elastic/elastic-package/pull/2564/files#diff-5794c40aa4c9301aaf9c39eacf7d3c8511f2511cb24bdd95e0a08704ba583a5fR89
There was a problem hiding this comment.
Another option could be create the context inside this method, so the signature does not change.
WDYT?
There was a problem hiding this comment.
I think it'd be better to add support for context, it seems to be used in this package for cancellation.
| z := archives.Zip{ | ||
| SelectiveCompression: true, | ||
| ContinueOnError: false, | ||
| } |
There was a problem hiding this comment.
New struct from archives has fewer options than the previous dependency:
https://github.com/mholt/archives/blob/4393d8832e1d36b9e188cba6d6e2fd3396362ae6/zip.go#L60-L78
There was a problem hiding this comment.
Compression looks similar to the old CompressionLevel, or at least seems to be used to control compression level/algorithm. We should check if we are compressing at the same level with old and new implementation, at least confirm that we are not only archiving without compression.
There was a problem hiding this comment.
Umm, I have checked and it looks like both implementations do the same, they are storing without compression 🤔
This is what file says, compression method store:
Zip archive data, at least v2.0 to extract, compression method=store
There was a problem hiding this comment.
Comparing the zip file resulting of using elastic-package v0.111.0 and the one from this PR with the file command, both zip files have the same output and the same size:
$ elastic-package build -v
...
$ file /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zipuser/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip: Zip archive data, at least v2.0 to extract, compression method=store
$ ls -l /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip
-rw-rw-r-- 1 user user 1305990 may 6 16:17 /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip
$ elastic-package-v0.111.0 build -v
...
$ file /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip
/home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip: Zip archive data, at least v2.0 to extract, compression method=store
$ ls -l /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zip
-rw-rw-r-- 1 user user 1305990 may 6 16:18 /home/user/Coding/work/integrations/build/packages/elastic_package_registry-0.3.1.zipLooking at the code, it looks like there is no compression:
- Previously, it was set
CompressionLevel: flate.DefaultCompression, so it looks like there is no compression.
https://github.com/mholt/archiver/blob/cc194d2e4af2dc09a812aa0ff61adc4813ea6c69/zip.go#L371-L389 - In both deps, if
zipfile extension is used andSelectiveCompressionis true, the method used isStorein both cases: - Looking at the compression methods:
- Previously, the
FileMethodfield from zip struct was not set, so IIUC this should beStoremethod. - In the new dep,
Compressionfield is not set neither.- And these are the compression methods available: https://github.com/mholt/archives/blob/4393d8832e1d36b9e188cba6d6e2fd3396362ae6/zip.go#L333-L341
- Previously, the
Do you know if we could do some other test to ensure that the resulting zip files keep the same behavior?
cmd/build.go
Outdated
| ctx := cmd.Context() | ||
| if ctx == nil { | ||
| // FIXME: when executging elastic-package check , context returned is nil | ||
| ctx = context.Background() |
There was a problem hiding this comment.
If this is not added, executing elastic-package check results in a panic, since ctx is nil in that case.
There was a problem hiding this comment.
We should rather fix cobraext.ComposeCommands, to receive and use the parent cmd, or at least its context.
There was a problem hiding this comment.
I'll give a try to fix that function to set the context from its "parent".
There was a problem hiding this comment.
With the changes introduced in a79d5db looks like it works now as expected.
Tested by adding in one package an unknown category and the lint stage fails when running elastic-package check command:
$ elastic-package check -v
2025/05/06 16:41:18 DEBUG Enable verbose logging
2025/05/06 16:41:18 DEBUG Distribution built without a version tag, can't determine release chronology. Please consider using official releases at https://github.com/elastic/elastic-package/releases
Lint the package
2025/05/06 16:41:18 DEBUG Check if README.md is up-to-date
2025/05/06 16:41:18 DEBUG Generate README.md file (package: /home/user/Coding/work/integrations/packages/elastic_package_registry)
2025/05/06 16:41:18 DEBUG Template file for README.md found: /home/user/Coding/work/integrations/packages/elastic_package_registry/_dev/build/docs/README.md
2025/05/06 16:41:18 DEBUG Using links definitions file: /home/user/Coding/work/integrations/links_table.yml
2025/05/06 16:41:18 DEBUG Render README.md file (package: /home/user/Coding/work/integrations/packages/elastic_package_registry, templatePath: /home/user/Coding/work/integrations/packages/elastic_package_registry/_dev/build/docs/README.md)
2025/05/06 16:41:18 DEBUG Read existing README.md file (package: /home/user/Coding/work/integrations/packages/elastic_package_registry)
Error: checking package failed: linting package failed: found 1 validation error:
1. file "/home/user/Coding/work/integrations/packages/elastic_package_registry/manifest.yml" is invalid: field categories.1: categories.1 must be one of the following: "advanced_analytics_ueba", "analytics_engine", "application_observability", "app_search", "auditd", "authentication", "aws", "azure", "big_data", "cdn_security", "cloud", "cloudsecurity_cdr", "config_management", "connector", "connector_client", "connector_package", "containers", "content_source", "crawler", "credential_management", "crm", "custom", "custom_logs", "database_security", "datastore", "dns_security", "edr_xdr", "elasticsearch_sdk", "elastic_stack", "email_security", "enterprise_search", "firewall_security", "google_cloud", "iam", "ids_ips", "infrastructure", "java_observability", "kubernetes", "language_client", "languages", "load_balancer", "message_queue", "monitoring", "native_search", "network", "network_security", "notification", "observability", "os_system", "process_manager", "productivity", "productivity_security", "proxy_security", "sdk_search", "security", "siem", "stream_processing", "support", "threat_intel", "ticketing", "version_control", "virtualization", "vpn_security", "vulnerability_management", "web", "web_application_firewall", "websphere", "workplace_search"
⏳ Build in-progress, with failures
Failed CI StepsHistory
cc @mrodm |
* main: chore: [updatecli] Update latest snapshot to 9.1.0-4348ed5d (elastic#2582) [CI] Allow to run each cloud cleanup step independently in DRY_RUN false mode (elastic#2581) buildkite: use GCP OIDC (elastic#2569) Replace archiver dependency (elastic#2564) github-action: add catalog-validate for GitHub actions (elastic#2576) chore: [updatecli] Update latest snapshot to 9.1.0-62329fee (elastic#2580) Chore(deps): bump golang.org/x/tools from 0.32.0 to 0.33.0 (elastic#2577) Chore(deps): bump gotest.tools/gotestsum from 1.12.1 to 1.12.2 (elastic#2578)
As github.com/mholt/archiver has been archived, this dependency has been replaced by github.com/mholt/archives as suggested in its README.