-
Notifications
You must be signed in to change notification settings - Fork 203
Bazel rule: detect "oci_layout" and "oci_tarball" output groups #523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bazel rule: detect "oci_layout" and "oci_tarball" output groups #523
Conversation
Summary of ChangesHello @malt3, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request enhances the container_structure_test Bazel rule by adding support for oci_layout and oci_tarball output groups, which is useful for compatibility with rules_img. The implementation correctly checks for these output groups and falls back to the previous behavior of using a single file from DefaultInfo. The overall change is good, but there is an opportunity to refactor the new logic to improve its clarity and reduce code duplication.
With rules_oci, the output of "oci_image" and "oci_image_index" is always a tree artifact containing the full oci layout. rules_img is an upcoming Bazel ruleset for building container images that tries to avoid unnecessary work. To that end, rules_img will only materialize a full oci layout tree artifact if a downstream action needs it. This is done through the use of two special output groups: - oci_layout contains a tree artifact of the full oci layout - oci_tarball contains is a tar file containing the oci layout With this change, container_structure_test is compatible with both rulesets. Users can continue to pass a single file or can choose to use the aforementioned output groups. This results in a better user experience for rules_img users who otherwise need to create a special intermediate filegroup to convert from OutputGroupInfo to DefaultInfo.
3d033ce to
9f64f9d
Compare
With rules_oci, the output of "oci_image" and "oci_image_index" is always a tree artifact containing the full oci layout. rules_img is an upcoming Bazel ruleset for building container images that tries to avoid unnecessary work. To that end, rules_img will only materialize a full oci layout tree artifact if a downstream action needs it. This is done through the use of two special output groups:
With this change, container_structure_test is compatible with both rulesets. Users can continue to pass a single file or can choose to use the aforementioned output groups.
This results in a better user experience for rules_img users who otherwise need to create a special intermediate filegroup to convert from OutputGroupInfo to DefaultInfo.