search

GoogleContainerTools / rules_distroless

Apache License 2.0
55 stars 34 forks source link

docs: improve documentation #109

Closed jjmaestro closed 1 week ago

jjmaestro commented 1 month ago

It's probably best and easiest to check the rendered markdown in my branch:

Both are very similar since I've copy-pasted the common part of the doc. I tried refactoring part of the doc to reuse it in both places but I couldn't use a template replacement in the apt.install macro docstring.

NOTE: I had to ~update aspect_bazel_lib and bazel_skylib plus~ add cache.bzl as a dependency in apt/private/BUILD.bzl to get bazel run //docs:update to work.

jjmaestro commented 1 month ago

@thesayyn there seems to be some checks awaiting approval :-? Can you approve them so they run? Thanks!!

jjmaestro commented 3 weeks ago

@thesayyn the failures I get in those checks match the weird failure I got while trying to generate the docs and why I added the cache.bzl dependency in apt/private/BUILD.bazel.

I'm not sure what's going on, It seems to only be failing in 6.4.0 though... if you can help me understand this 🙏 Meanwhile, I'll try to repro it locally as well.

jjmaestro commented 2 weeks ago

OK, I've dug a bit into this and I think the issue is that, somehow (still have to check why) I needed to add cache.bzl as a dependency when running with a modern Bazel but that doesn't exist in @bazel_tools for old Bazels, so it breaks in 6.4.0.

Now, here's the thing, cache.bzl didn't exist until three months ago! (https://github.com/bazelbuild/bazel/commit/bd63c6104951a5e1aac85ec8cc8e2d0c084390c1) so I'm not sure why / how is that even being required, I definitely need to check more on that.

But, once I remove it again and re-run the tests for Bazel 6.4.0, I get another error

ERROR: /src/workspace/examples/ubuntu_snapshot/BUILD.bazel:27:6: in tar rule //examples/ubuntu_snapshot:group: 
Traceback (most recent call last):
        File "/root/.cache/bazel/_bazel_root/a08c2e4811c846650b733c6fc815a920/external/aspect_bazel_lib~2.9.3/lib/private/tar.bzl", line 304, column 38, in _tar_impl
                src[DefaultInfo].files_to_run.repo_mapping_manifest
Error: 'FilesToRunProvider' value has no field or method 'repo_mapping_manifest'

And sure enough, when I check, that was added in https://github.com/bazelbuild/bazel/commit/3575211fb445175f56e5322ace67bbef865e2205 so again, very modern Bazel. Yet, aspect_bazel_lib v2.9.3 is using it, and declaring a bazel_compatibility = [">=6.0.0"].

And look what I found 😄 https://github.com/bazel-contrib/bazel-lib/issues/975 and https://github.com/bazel-contrib/bazel-lib/pull/976

So, I'm going to try and use 2.9.1 which doesn't have the bug and will see if I can debug the cache.bzl thing which is quite weird (to my inexperienced eye 😅 )

jjmaestro commented 2 weeks ago

OK, so there's something very weird going on when trying to make this work with 6.4.0 and I don't know how to fix it. @thesayyn does it make any sense to support 6.x when, unless I'm mistaken, GoogleContainerTools/rules_distroless has a .bazelversion of 7.2.0?

I'm happy to make a PR bumping the CI to 7.x, using the latest CI config (I've noticed it's hardcoded to a commit that e.g. prevents testing with 8.x.

Can you let me know? I'd really like to unblock and land this so I can rebase the rest of the PRs.

Thanks!!

jjmaestro commented 2 weeks ago

@thesayyn I've rebased the PR removing the changes to bazel_skylib and aspect_bazel_lib since I've seen #111 removed support for Bazel 6 so everything should work a-OK in Bazel 7+ :) I've also rebased this on top of #112 which I hope you'll accept, because the MODULE.bazel.lock is constantly changing and breaking the rebases, and I really don't see it adding any value.

Also, could you approve the CI run? Thanks!!