add libarchive 3.7.3 with tests and windows support #1916
Conversation
zaucy
force-pushed
the
add-libarchive-3.7.3-and-tests
branch
from
5459986
to
df6dc6b
Compare
|
Hello @dzbarsky, modules you maintain (libarchive) have been updated in this PR. Please review the changes. |
|
@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
changed the title
|
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
force-pushed
the
add-libarchive-3.7.3-and-tests
branch
from
552e852
to
9c8711b
Compare
alexeagle
added
the
presubmit-auto-run
|
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 |
|
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
force-pushed
the
add-libarchive-3.7.3-and-tests
branch
from
593e84d
to
9530ab5
Compare
| @@ -0,0 +1,15 @@ | |||
| module( | |||
| name = "libarchive", | |||
| version = "3.7.3", | |||
There was a problem hiding this comment.
would you mind doing this on 3.7.4? That's the latest version in the BCR now
There was a problem hiding this comment.
I was thinking of doing 3.7.4 as a follow up once we felt this patch was good, but how about I just include both 3.7.3 and 3.7.4.bcr.1 in this PR? :)
There was a problem hiding this comment.
whatever's easier for you :)
There was a problem hiding this comment.
once all the other conversations are resolved I'll add 3.7.4.bcr.1 👍
| - debian10 | ||
| - debian11 | ||
| - ubuntu2004 | ||
| # - ubuntu2004_arm64 |
There was a problem hiding this comment.
how did you generate the patch? I've seen patches generated with git diff work but git show not work in the past. Maybe it would help to trim the header lines?
There was a problem hiding this comment.
I just used git diff. I have no idea why its not working. This is how I'm currently generating it https://github.com/zaucy/libarchive/blob/bazel-v3.7.3/bazel/update_bcr.nu#L37
There was a problem hiding this comment.
hmm, maybe @Wyverald has an idea?
|
This fix is required to allow rules_js to work on windows. Can it be moved forward? |
|
Since v3.7.3 is already in the registry, is this PR still relevant? Happy to merge once the module maintainers tell me to. |
|
@dzbarsky we do still need this, right? |
|
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 #1916 (comment) It would be nice to fix this before landing, but I think we need some help. @zaucy anything else I'm missing? |
|
yes currently #1916 (comment) is making me hesitant to say it's done. After that's resolved I'm happy to update this PR with |
Based on https://github.com/zaucy/libarchive/tree/bazel-v3.7.3