search

bazelbuild / rules_pkg

Bazel rules for creating packages of many types (zip, tar, deb, rpm, ...)
Apache License 2.0
216 stars 171 forks source link

//pkg:verify_archive.bzl verify_archive_test provided but unusable #731

Closed chickenandpork closed 11 months ago

chickenandpork commented 1 year ago

I was very thankful when I noticed that a standard-ish way to validate the contents of a tar file was available:

https://github.com/bazelbuild/rules_pkg/blob/6c2e32553b740aec1d6fb540fb224487b5ce26a2/pkg/verify_archive.bzl#L15

"""Rule to test that the content of an archive has particular properties.

This is available for integration testing, when people want to verify that all the files they expect are in an archive. Or possibly, they want to verify that some files do not appear.

The execution time is O(# expected patterns * size of archive). """

Unfortunately, I don't think it's usable (it's possible that external re-use of this test capability might not be the intended audience)

  1. template at https://github.com/bazelbuild/rules_pkg/blob/6c2e32553b740aec1d6fb540fb224487b5ce26a2/pkg/verify_archive.bzl#L80 is //pkg:... not @rules_pkg//pkg:... so it cannot be reached in a dependent project trying to use verify_archive_test
  2. template at item 1 is not included in the rules_pkg-0.9.1 archive
  3. test lib at https://github.com/bazelbuild/rules_pkg/blob/6c2e32553b740aec1d6fb540fb224487b5ce26a2/pkg/verify_archive.bzl#L125 is dev = [ "//pkg:... not @rules_pkg//pkg:... with similar barrier to point 1.

Tentatively resolved

Points 1 and 3 are easily resolved; for point 2, I'm not certain how the missing file should be added to the delivered tar file in the release.

aiuto commented 1 year ago

Yup. It's all easy to fix. I'll start a PR.

aiuto commented 1 year ago

But.... you may be looking at old code too. There is no verify_archive_test_lib any more.

chickenandpork commented 1 year ago

(thanks, BTW)

you may be looking at old code too

Yeah, I'm using the latest release, which is 0.9.1 (2023-03-28?) .. an eternity in internet days :)

What's the right.. right-er .. way to re-use your unittest logic going forward?

aiuto commented 1 year ago

I don't have a lot of tests for it inside rules_pkg yet. It's an ultra low priority task for me to convert current golden tests to use verify_archive.

The motivator was that some fixes to the leading './' on tar paths broke a lot of people inside Google who had brittle structural tests on built tar files. I needed a test that would let them see that the expected files were there, without being a strict compare of tar tvf outputs. When I migrate internal users to better tests, I can finally update to 0.9.x.

aiuto commented 11 months ago

FWIW.. https://github.com/bazelbuild/rules_pkg/pull/749 will add some features to verify_archive and show some more uses. The missing TPL file is fixed at head.