bazelbuild / bazel-central-registry

The central registry of Bazel modules for the Bzlmod external dependency system.
https://registry.bazel.build
Apache License 2.0
260 stars 339 forks source link

feat: libarchive 3.7.5.bcr1 for windows compression filter fixes #3225

Closed thesayyn closed 4 days ago

thesayyn commented 6 days ago

Fixes linking of various filters for windows making it on par with other platforms.

Currently, when compiling libarchive for windows, some define statements are missing from windows_config.h which results in libarchive shelling out to programs such as gzip xz when encountering archives with such compression algos.

Running bazel-bin/external/libarchive~/tar/bsdtar.exe -tf test.tar.gz

eg: version 3.7.5

bsdtar.exe: Error opening archive: Can't initialize filter; unable to run program "gzip -d"

version 3.7.5.bcr.1 (this patch):

.github/
.github/workflows/
.github/workflows/build.yaml
.github/workflows/release.yml
bazel-io commented 6 days ago

Hello @dzbarsky, @zaucy, modules you maintain (libarchive) have been updated in this PR. Please review the changes.

thesayyn commented 6 days ago

https://github.com/aspect-build/rules_js/issues/1739#issuecomment-2455809190

zaucy commented 5 days ago

Hm thats fair. Thanks for fixing and investigating the issue. The defines I generated were taken from a naive cmake build that certainly wouldn't have covered every environment on Windows. I noticed in the patch that the fuzz test had the 'manual' tag added. It's already skipped in the presubmit, it shouldn't need that manual tag.

test_read_truncated_filter skip presubmit tag removed - last time I ran it on the presubmit it took a very long time. I suppose when you fix the validation error we'll see how long it is!

test_write_filter_gzip_timestamp skip presubmit tag removed - glad to see that this test works now :)

As as side note - we should probably move to an overlay style patch for libarchive sooner than later since the patch is quite large and mostly additional. Would have probably made your adjustments much easier and easier for reviewing the changes.

thesayyn commented 5 days ago

As as side note - we should probably move to an overlay style patch for libarchive sooner than later since the patch is quite large and mostly additional. Would have probably made your adjustments much easier and easier for reviewing the changes.

Definitely agree. We need a better pattern here.

It's already skipped in the presubmit, it shouldn't need that manual tag.

I added it to skip it on local as well, happy to remove it if it needs to be removed.

thesayyn commented 5 days ago

This should be ready now, i updated the patch to exclude MODULE.bazel.lock and test.patch, :sigh:

thesayyn commented 5 days ago

we should probably move to an overlay style patch for libarchive sooner than later since the patch is quite large and mostly additional.

okay after lots of pain and suffering, i have overlay thing setup and good to go. PTAL @zaucy @dzbarsky