bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
387 stars 180 forks source link

Expand make variables in write_file content and expand_template substitutions #442

Open SanjayVas opened 1 year ago

SanjayVas commented 1 year ago

If a user wants make variable substitution, they need to either write their own versions of these rules or else fall back to genrule.

I assume a boolean attribute would need to be added to toggle this behavior as it wouldn't be backwards-compatible.

tetromino commented 1 year ago

This is a reasonable request, but note that we'd need an ergonomic way to specify how to map from variable names to substitution keys (e.g. are you using $var or {{var}} or $(var) or ...)

aiuto commented 1 year ago

Strong vote for ${var}.

As an alternative, {var} is sort of OK.

SanjayVas commented 1 year ago

This is a reasonable request, but note that we'd need an ergonomic way to specify how to map from variable names to substitution keys (e.g. are you using $var or {{var}} or $(var) or ...)

To clarify, this is about substitution of Make Variables. There's no need to specify any kind of mapping here, as that's already defined. The rule implementation just needs to call ctx.expand_make_variables (ignore the deprecation warning on that function, as a suitable replacement does not actually exist).

tetromino commented 1 year ago

a suitable replacement does not actually exist).

I think the appropriate solution to this issue would be such a suitable replacement. With sufficient flexibility to support ${var}, $(var), or any other reasonable syntax. I don't see any particular reason why it's impossible; just some ugly string munging. (We can add some better APIs to a future version of Bazel to make the string munging less ugly, a real regex module exposed to Starlark for example would help massively, but skylib unfortunately needs to work with the current stable version of Bazel too.)