Skip to content
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

Use build settings (or equivalent) for arbitrary string attributes #14859

Open
pauldraper opened this issue Feb 17, 2022 · 13 comments
Open

Use build settings (or equivalent) for arbitrary string attributes #14859

pauldraper opened this issue Feb 17, 2022 · 13 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team type: feature request

Comments

@pauldraper
Copy link
Contributor

pauldraper commented Feb 17, 2022

Surprisingly, there is no good way to set string attributes to an arbitrary value from the command line.

select can configure virtually any rule attribute. However, the value must come from an enumerated list.

label_flag can use any value specified on the command line, but it only works for label attributes.

build_setting can use any value specified on the command line, but it requires the rule author to accept a label and interpret the providers.

For example, suppose I have:

docker_image(
  name = "example",
  tag = "" # TODO
)

I want to set the tag on the command line, but I have to convince rule authors to never use attr.string().

(FYI, for the specific case of docker_image, rules_docker does in fact support "make expansions" of the tag attribute, using the the undocumented ctx.version_file API in conjunction with workspace status. But you could imagine many cases where users may want to have a "dynamic" string attribute that the rule authors didn't image.)

@brandjon
Copy link
Member

brandjon commented May 2, 2022

Sounds like the request is for there to be a string-valued equivalent to label_setting / label_flag. (This seems like a slippery slope to doing the same for all attr types -- maybe there's a more general API we could use?)

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Configurability Issues for Configurability team untriaged and removed untriaged team-Build-Language P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels May 2, 2022
@pauldraper
Copy link
Contributor Author

pauldraper commented May 3, 2022

Sounds like the request is for there to be a string-valued equivalent to label_setting / label_flag

Yeah, sort of. The difference being that it isn't monomorphic; it's not "forwarding" the value. So that leads to some consideration of how it's actually consumed.

This seems like a slippery slope to doing the same for all attr types

I'm not really sure what the "slippery slope" leads down to. Configurable builds?


What would really be great is a global "environment variable"-type API.

tag = env.get("DOCKER_TAG", "")

docker_image(
  name = "example",
  tag = tag,
)

Bazel's currently configurability seems almost purposefully difficult. Something like this would be useful in an infinite number of scenarios, without deliberate cooperation of rule authors and users.

@aranguyen
Copy link
Contributor

What about using varref? I’m not super familiar with it but from the documentation it seems that they can be used in rules that are subject to “Make” variable substitution.

@aranguyen aranguyen added awaiting-user-response Awaiting a response from the author and removed untriaged labels May 16, 2022
@benjaminp
Copy link
Collaborator

varref has never been supported by Bazel.

@pauldraper
Copy link
Contributor Author

FYI, for the specific case of docker_image, rules_docker does in fact support "make expansions" of the tag attribute, using the the undocumented ctx.version_file API in conjunction with workspace status. But you could imagine many cases where users may want to have a "dynamic" string attribute that the rule authors didn't image.

@kkpattern
Copy link
Contributor

This feature would be very useful for us. For example, assume we are developing an iOS app framework. We can provide an ios_application target for users to build the iOS app.

ios_application(
    name = "app",
    build_id = ""
)

However, we don't know the bundle id of the app users wants to build beforehand. If we can pass the value of a string flag to a string attribute:

ios_application(
    name = "app",
    build_id = get_flag_value(":bundle_id"),
)

then users can easily provide the bundle id like this:

bazel build //app:app. --//app:bundle_id=com.xxx.yy

@gregestren gregestren added untriaged and removed awaiting-user-response Awaiting a response from the author labels Feb 15, 2023
@gregestren gregestren added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 13, 2023
@Louai-Abdelsalam
Copy link

Louai-Abdelsalam commented Apr 3, 2023

@pauldraper as far as I understand your requirement, it looks like --define does what you need.

Usage is something along the lines of doing --define <some key>=<some val> on the cli, and referencing the <some key> in a given rule's attr like a make variable (i.e $(<some key)).

Keep in mind though that the variable gets resolved either in the analysis stage or in the execution stage (I don't remember which), meaning that you can't e.g process its value in a macro for example.

For some reason this flag is barely documented though, at least in the context of this use case. It seems significantly more documented in the context of build_setting and config_setting. Heck even the make variable page only mentions custom make variables (which I think this usage of --define qualifies as) as doable only through "downstream targets consuming make variables created by upstream targets" (or something along those lines). So either the documentation is wrong, or I'm misunderstanding something here.

Anyway, hope this helps.

@fmeum
Copy link
Collaborator

fmeum commented Apr 4, 2023

I created bazelbuild/bazel-skylib#440 to make the string- and int-valued build settings defined there usable wherever --define is, using (and generating) Make variables.

@pauldraper
Copy link
Contributor Author

For some reason this flag is barely documented

--define is "legacy".

it looks like --define does what you need.

It seems this still requires cooperation from rule authors to do the substitution.

@pauldraper
Copy link
Contributor Author

pauldraper commented Apr 6, 2023

I don't see why it's a hard or bad idea to have a variable available even as early as the loading phase.


Currently, my best workaround is to write a tools/bazel that writes a Starlark file.

All the ad-hoc customizability I could want.

tools/bazel

#!/usr/bin/env bash
tmp="$(mktemp)"

(echo -n 'VARS = '; jq -n '$ENV') > "$tmp"

if diff -q "$tmp" vars.bzl > /dev/null; then
  rm "$tmp"
else
  mv "$tmp" vars.bzl
fi

exec "$BAZEL_REAL" "$@"

BUILD.bazel

load("@//:vars.bzl", VARS)

example_rule(
  name = "example",
  example = VARS.get("EXAMPLE"),
)

Then

EXAMPLE=example bazel build //:example

@gregestren
Copy link
Contributor

I did a review pass over bazelbuild/bazel-skylib#440, which brings build settings up to par with --define.

But yes, these approaches still require rule cooperation. And they don't work at all for certain native rule attributes.

A completely generic solution would take some Bazel refactoring. I'm not sure offhand was the original reason was to restrict make variable interpretation to a specifically opted in set of attributes, vs. making it universal. I know there was reluctance to support make variables at all. But I hear you on the use case.

@gregestren
Copy link
Contributor

One more thing: if we have global variables that could theoretically be consumed anywhere (no consumes declare that they need it), it's hard to figure out what needs to be invalidated when a variable changes between builds. So it makes it harder to tune fast incremental builds.

@pauldraper
Copy link
Contributor Author

pauldraper commented Aug 21, 2023

One more thing: if we have global variables that could theoretically be consumed anywhere (no consumes declare that they need it), it's hard to figure out what needs to be invalidated when a variable changes between builds.

The loading/analysis phases get recalculated. But in my experience analysis cache is already dumped fairly indiscriminately when flags change.

Doesn't seem to change the status quo much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants