search

bazel-contrib / bazel-lib

Common useful functions for writing BUILD files and Starlark macros/rules
Apache License 2.0
140 stars 90 forks source link

fix: go back to bsdtar-prebuilt 3.7.4 but with a release with a working windows binary #969

Closed gregmagolan closed 1 month ago

gregmagolan commented 1 month ago

Fixes tar extract regression in 2.8.0 due to bump to bsdtar-prebuilt 3.7.5 in https://github.com/bazel-contrib/bazel-lib/pull/940.

bsdtar-prebuilt 3.7.5 (which depends on libarchive 3.7.5) has a regression that showed up in testing of our internal silo repo when attempting to bump bazel-lib. previously working tests with tar extracts starting to fail with invalid tar header. I also tested with a test bsdtar-prebuilt 3.7.7 release, built with libarchive 3.7.7, to see if there was a regression in libarchive 3.7.5 (one such regression was mentioned in https://github.com/libarchive/libarchive/releases/tag/v3.7.6) but also hit the same failure. Another possibility is BCR patches for libarchive 3.7.5 that were copied over from 3.7.4 may not be correct for 3.7.5.

In any case, to get back to a working version of tar in bazel-lib, this change goes back to safety by switching to a bsdtar-prebuilt 3.7.4 release with a working Windows binary: https://github.com/aspect-build/bsdtar-prebuilt/releases/tag/v3.7.4-4.

jbedard commented 1 month ago

Is there any way to test this here in bazel-lib?

gregmagolan commented 1 month ago

Is there any way to test this here in bazel-lib?

I'm not sure what it is about those particular tests in silo that causes tar extract to fail. Would be good to track it down and make a minimal repro test in the future. For now I tested this change with a patch in silo and verified it fixes the issue.

gregmagolan commented 1 month ago

FYSA @alexeagle @dzbarsky

alexeagle commented 1 month ago

Isn't this just going to break again the next time we upgrade libarchive?

gregmagolan commented 1 month ago

Isn't this just going to break again the next time we upgrade libarchive?

Likely yes. We have no idea why it was broken on silo. We need a repro outside of silo for this so we can figure out why it was built wrong so we can fix it without having to test in silo.