Skip to content
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

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

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

thesayyn
Copy link
Contributor

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
Copy link
Member

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

@thesayyn
Copy link
Contributor Author

@zaucy
Copy link
Contributor

zaucy commented Nov 20, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@alexeagle alexeagle self-requested a review November 20, 2024 19:25
@alexeagle alexeagle changed the title feat: libarchive 1.7.5.bcr1 for windows compression filter fixes feat: libarchive 3.7.5.bcr1 for windows compression filter fixes Nov 20, 2024
@thesayyn thesayyn force-pushed the main branch 11 times, most recently from 240aff9 to 961a71f Compare November 20, 2024 22:13
@thesayyn
Copy link
Contributor Author

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

dzbarsky
dzbarsky previously approved these changes Nov 20, 2024
Copy link
Contributor

@dzbarsky dzbarsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, so much easier to read :)

bazel-io
bazel-io previously approved these changes Nov 20, 2024
Copy link
Member

@bazel-io bazel-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @bazelbuild/bcr-maintainers, all modules in this PR have been approved by their maintainers. Please take a final look to merge this PR.

@bazel-io bazel-io dismissed dzbarsky’s stale review November 21, 2024 00:33

Require module maintainers' approval for newly pushed changes.

@bazel-io bazel-io dismissed their stale review November 21, 2024 00:33

Require module maintainers' approval for newly pushed changes.

Copy link
Member

@bazel-io bazel-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @bazelbuild/bcr-maintainers, all modules in this PR have been approved by their maintainers. Please take a final look to merge this PR.

@meteorcloudy meteorcloudy added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Nov 21, 2024
@alexeagle alexeagle merged commit e651d70 into bazelbuild:main Nov 21, 2024
18 checks passed
alexeagle added a commit to aspect-build/bsdtar-prebuilt that referenced this pull request Dec 11, 2024
alexeagle added a commit to aspect-build/bsdtar-prebuilt that referenced this pull request Dec 11, 2024
* chore: release newest entry from BCR

Pick up bazelbuild/bazel-central-registry#3225 for windows compression filter fixes

* upgrade Bazel, it's required
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants