bazel-central-registry icon indicating copy to clipboard operation
bazel-central-registry copied to clipboard

add libarchive 3.7.3 with tests and windows support

Open zaucy opened this issue 2 months ago • 5 comments

Based on https://github.com/zaucy/libarchive/tree/bazel-v3.7.3

  • adds tests
  • adds Windows support

zaucy avatar Apr 30 '24 18:04 zaucy

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

bazel-io avatar Apr 30 '24 18:04 bazel-io

@dzbarsky I would love your opinion on these changes since you were the original maintainer. If this works out I intend to subumit these changes for 3.7.2 as well.

zaucy avatar Apr 30 '24 18:04 zaucy

some of the tests take an especially long time on the GitHub free action runners, but are quite fast locally. If they run slowly on the BCR buildkite CI then I may disable them or exclude them in the presubmit.

zaucy avatar Apr 30 '24 18:04 zaucy

Thanks for submitting! I'm about to go on vacation for a week, I will do my best to find time to look before that

DavidZbarsky-at avatar May 01 '24 23:05 DavidZbarsky-at

The presubmit is mostly green after I've excluded some tests (with added notes for any future readers.)

However, there is a patch error only on ubuntu arm64 for some reason. If anyone has any insight on that, it would be very helpful.

zaucy avatar May 02 '24 16:05 zaucy

This fix is required to allow rules_js to work on windows. Can it be moved forward?

peakschris avatar May 17 '24 10:05 peakschris

Since v3.7.3 is already in the registry, is this PR still relevant? Happy to merge once the module maintainers tell me to.

fmeum avatar May 17 '24 10:05 fmeum

@dzbarsky we do still need this, right?

alexeagle avatar May 17 '24 12:05 alexeagle

We still need this PR and/or a 3.7.4 with these changes applied on top.

From my POV, it's ready to go in, but @zaucy can make the final call on whether to change the config.h setup or keep as is.

There's also some issue with the patch not applying cleanly on one platform that we haven't gotten to the bottom of https://github.com/bazelbuild/bazel-central-registry/pull/1916#discussion_r1594523428

It would be nice to fix this before landing, but I think we need some help.

@zaucy anything else I'm missing?

DavidZbarsky-at avatar May 17 '24 15:05 DavidZbarsky-at

yes currently https://github.com/bazelbuild/bazel-central-registry/pull/1916#discussion_r1594523428 is making me hesitant to say it's done. After that's resolved I'm happy to update this PR with 3.7.4.bcr.1 added as well.

zaucy avatar May 17 '24 15:05 zaucy