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

select() is needlessly banned from workspace macros #9199

Closed
mboes opened this issue Aug 19, 2019 · 6 comments

Comments

@mboes
Copy link
Contributor

@mboes mboes commented Aug 19, 2019

Description of the feature request:

Say I have two macros, workspace_macro(x) and build_macro(x), typically called in WORKSPACE and BUILD files respectively. The following is legal:

build_macro(x = select({"//conditions:default": True}))

The following should also be legal:

workspace_macro(x = select({"//conditions:default": True}))

Feature requests: what underlying problem are you trying to solve with this feature?

Currently, the latter example above fails with an error message:

ERROR: [...] select() cannot be used in macros called from WORKSPACE files

This seems like an arbitrary limitation to me. While I understand that select() won't work when provided as an argument to repository rules, some workspace macros are ultimately generating BUILD files. If select() is legal in arguments to macros in BUILD files, it should be legal in WORKSPACE files.

Consider the following example. We have a macro rules_foo_toolchains() to define a toolchain repository, along with a toolchain rule with suitable target_constraints and exec_constraints. Say the user wants to conditionally pass default compiler arguments to the foo compiler, say -O2 if opt mode is on. The user only has access to the toolchain macro, so to control the default compiler arguments, they need to supply the arguments via rules_foo_toolchains().

Currently, the user cannot say e.g.

rules_foo_toolchains(
    copts = ["-bar"] + select({"//conditions:default": ["-baz"]}),
)

This is an unnecessary pessimization. It should be legal to define a macro that computes the BUILD-file content inside a toolchain repository, using the above provided copts. Since the string representation of the value of copts reads the same as its expression above, this is easy to do with some string concatenation.

What's the output of bazel info release?

v0.28

@katre

This comment has been minimized.

Copy link
Member

@katre katre commented Aug 23, 2019

There is no distinction between "WORKSPACE macros" and "BUILD macros". The distinction is that some functions, like select, cannot be used in the WORKSPACE file.

select depends on having a configuration to examine and make choices between. When the WORKSPACE is evaluated, there is no configuration present yet, and so select cannot be used.

You need to move the functionality into a BUILD file to use select there, and then have the WORKSPACE register the toolchain that declares the select.

@katre katre closed this Aug 23, 2019
@mboes

This comment has been minimized.

Copy link
Contributor Author

@mboes mboes commented Aug 24, 2019

select depends on having a configuration to examine and make choices between. When the WORKSPACE is evaluated, there is no configuration present yet, and so select cannot be used.

I get that. But my use case detailed above does not need a configuration. The user is providing a select in the WORKSPACE file that a macro is writing to a BUILD file.

As far as I can tell, select() does not eagerly resolve to one of its branches during BUILD file evaluation. select() evaluates to a special select structure, which the analysis phase resolves to one of the select() branches. Furthermore, the string representation of a select() is itself. So again, why not let the user pass a select, and let the macro print the representation of a select() to a BUILD file?

You need to move the functionality into a BUILD file to use select there, and then have the WORKSPACE register the toolchain that declares the select.

To summarize, I have a public WORKSPACE macro over a private repository rule that generates a BUILD file. The content of that BUILD file can therefore only be a function of the arguments of the macro. The user does not have access to the BUILD file nor is it easy for them to write the BUILD file themselves. See https://api.haskell.build/haskell/nixpkgs.html#haskell_register_ghc_nixpkgs. Are you suggesting that I ask my users to write the toolchain BUILD file themselves?

@katre

This comment has been minimized.

Copy link
Member

@katre katre commented Aug 26, 2019

Can you provide a larger example of what you want to do? I'm afraid I didn't actually understand your use-case.

In the snippet you provided:

rules_foo_toolchains(
    copts = ["-bar"] + select({"//conditions:default": ["-baz"]}),
)

There is no way for the macro to expand to change the actual build file. You could conceivably pass a string to the rules_foo_toolchains macro, and write that string into the BUILD file that the repository rule is writing. However, that would be confusing to users and prone to errors.

If users need this level of customization to the foo_toolchain target, they will need to write their own in an actual BUILD file in the project, and then register it from the WORKSPACE before any default toolchains registered by the rules.

@mboes

This comment has been minimized.

Copy link
Contributor Author

@mboes mboes commented Aug 30, 2019

There is no way for the macro to expand to change the actual build file. You could conceivably pass a string to the rules_foo_toolchains macro, and write that string into the BUILD file that the repository rule is writing.

That's exactly what I would like to be able to do, yes.

However, that would be confusing to users and prone to errors.

I'm not sure why. Could you clarify?

If users need this level of customization to the foo_toolchain target, they will need to write their own in an actual BUILD file in the project

They would need to define the tolochain in a separate repository so that bazel build //... doesn't pick it up on incompatible platforms. They would need to redo by hand all the things that the workspace rule does. In our case, the workspace rule is non-trivial. See the BUILD file that the workspace produces here: https://github.com/tweag/rules_haskell/blob/master/haskell/nixpkgs.bzl#L58-L86.

This is why I propose to liberalize the rules regarding select() in WORKSPACE. It's because it's the only way I see to provide a decent UX to my users. But I would be happy to hear about any other options for providing a good UX.

@katre

This comment has been minimized.

Copy link
Member

@katre katre commented Aug 30, 2019

Responding to the first half of your comment:

Passing BUILD-style Starlark as a string is error-prone because there are no compilation checks where it is written: any errors would be reported in the BUILD file you inject it into.

Consider a WORKSPACE macro like this:

set_up_custom_toolchain("copts = select{"//config:setting1": "-DFOO", "//conig:setting2": "-DBAR"}")

The errors in this code (missing parens for the select, the incorrect package name "//conig", etc) will not be reported from WORKSPACE, but from your code.

@katre

This comment has been minimized.

Copy link
Member

@katre katre commented Aug 30, 2019

Responding to the second half:

They would need to define the tolochain in a separate repository so that bazel build //... doesn't pick it up on incompatible platforms. They would need to redo by hand all the things that the workspace rule does. In our case, the workspace rule is non-trivial.

Toolchains are not executed by default. I am not sure why it would be a problem for them to be analyzed as part of "bazel build //...".

You can certainly provide helpful macros for users to create their own toolchains simply.

I think, in general, that you are confused about why "select" and related functions are forbidden in WORKSPACE. While we call functions in BUILD and WORKSPACE "macros", they are not macros in the same sense as in LISP or even the C pre-processor: the arguments to Starlark functions are not passed in to the body and then become part of the AST of the generated code. If the "select" function were allowed in the WORKSPACE, it would be executed in the context of the workspace, and there is no configured target or configuration there to allow it to be executed.

Starlark does not have first-class functions as an intentional choice, to prevent BUILD files from becoming too complex and to hep ensure that build rules will eventually halt (and for this reason, Starlark is technically not Turing complete, I believe).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.