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

Expand make variables in write_file content and expand_template substitutions #442

Open
SanjayVas opened this issue Mar 28, 2023 · 4 comments
Labels
good first issue P3 We're not considering to work on this, but happy to review a PR. (No assignee) type: feature request

Comments

@SanjayVas
Copy link

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
Copy link
Collaborator

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 ...)

@tetromino tetromino added type: feature request good first issue P3 We're not considering to work on this, but happy to review a PR. (No assignee) labels Jul 5, 2023
@aiuto
Copy link
Contributor

aiuto commented Jul 6, 2023

Strong vote for ${var}.

  • The leading $ is widely used in scripting languages as a variable reference. It is universally familiar.
  • $var is insufficient because you you can't parse where var ends.
  • $(var) is very Makefile centric. While the Bazel engineers know that from history, it is not actually a widely used style anywhere else.
  • {{var}} is also not commonly used.
  • %var% is widely known in the windows world, but no where else.

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

@SanjayVas
Copy link
Author

SanjayVas commented Jul 6, 2023

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
Copy link
Collaborator

tetromino commented Jul 6, 2023

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.)

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

No branches or pull requests

3 participants