bazel-contrib / rules_oci

Bazel rules for building OCI containers
Apache License 2.0
292 stars 153 forks source link

Align oci_image's env attribute with Bazel common attribute standards #688

Open andyscott opened 1 month ago

andyscott commented 1 month ago

TLDR

For the 2.0.0 release can we rename oci_image_rule's env attribute to env_file so that third party tooling can safely assume all env attributes on rules are dictionaries? The oci_image macro can retain the env attribute as is.

Background

rules_oci's oci_image_rule rule declares the env attribute as a file input. This breaks with the standard set out by Bazel's common definitions which suggests that env should be a dictionary (at least for binary and test rules).

For end users this discrepancy is handled by the oci_image macro, which accepts the dictionary and silently creates the file for the rule invocation.

Tooling can be broken by this discrepancy. For example: IntelliJ's latest (early release) Bazel plugin breaks when it tries to serialize the env dictionary with to_proto:

ERROR: /Users/andy/git/.../BUILD.bazel:115:9: in @@bazelbsp_aspect//aspects:core.bzl%bsp_target_info_aspect aspect on oci_image rule //my/thing:thing_image: 
Traceback (most recent call last):
    File "/private/var/tmp/_bazel_andy/6984c3fb2f491c925a6b0ec8d6b97e37/external/bazelbsp_aspect/aspects/core.bzl", line 191, column 64, in _bsp_target_info_aspect_impl
        ctx.actions.write(info_file, create_struct(**info).to_proto())
Error in to_proto: in struct field .env: in Target field .actions: at list index 0: got Action, want string, int, float, bool, or struct

(relevant code bit here)

thesayyn commented 1 day ago

This sounds like a bug in IntelliJ plugin, as env attribute only applies to runnable targets which oci_image isn't.