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

repository_rule no longer supports making attrs inside a function #19459

Closed
illicitonion opened this issue Sep 8, 2023 · 7 comments
Closed
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: bug

Comments

@illicitonion
Copy link
Contributor

Description of the bug:

In Bazel 6, we could write a repository rule which read an env var, and was re-run when that env var changed, but where the particular env var could be specified by the instantiator of the repository rule, like:

def _create_repository_rule_sensitive_to_env_var(name, env_var_name):
    # Generate an intermediate rule which re-triggers if the value of the env var pointed at by env_var_name changes.
    generated_rule = repository_rule(
    _impl,
        attrs = {
            "env_var_name": attr.string(),
        },
        environ = [env_var_name],
    )

    generated_rule(
        name = name,
        env_var_name = env_var_name,
    )

_create_repository_rule_sensitive_to_env_var("foo", "SOME_ENV_VAR")

In Bazel 7, this pattern no longer works because of this error:

Error in string: attr.string() can only be used during .bzl initialization (top-level evaluation) or package evaluation (a BUILD file or macro)

It looks like the commit which introduced this restriction was attempting to avoid pointless calls to attr.string(), but accidentally catches "creating a repository_rule inside a function".

It looks like we can extract the attrs value to a top-level variable, assuming it itself isn't parameterised in the containing function, and things start working again, but this seems unnecessarily fiddly/restrictive?

Which category does this issue belong to?

External Dependency, Rules API

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

/WORKSPACE file:

load("//:rr.bzl", "create_repository_rule_sensitive_to_env_var")

create_repository_rule_sensitive_to_env_var("foo", "SOME_ENV_VAR")

/rr.bzl file:

def _impl(rctx):
    rctx.file("WORKSPACE")
    rctx.file("BUILD.bazel")
    value = rctx.os.environ.get(rctx.attr.env_var_name, None)
    rctx.file("defs.bzl", "env_var = " + repr(value) + "\n")

def create_repository_rule_sensitive_to_env_var(name, env_var_name):
    # Generate an intermediate rule which re-triggers if the value of the env var pointed at by env_var_name changes.
    generated_rule = repository_rule(
        _impl,
        attrs = {
            "env_var_name": attr.string(),
        },
        environ = [env_var_name],
    )

    generated_rule(
        name = name,
        env_var_name = env_var_name,
    )

Run: bazel query @foo//...

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.0-pre.20230823.4

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

835512a /cc @brandj

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@illicitonion illicitonion changed the title repository_rule no longer supports setting environ dynamically repository_rule no longer supports making attrs inside a function Sep 8, 2023
@Wyverald
Copy link
Member

Wyverald commented Sep 8, 2023

cc @brandjon

Before we try for a fix, what's the use case for such a setup? Dynamically generating a repo rule is arguably a bug (see #10441)

@illicitonion
Copy link
Contributor Author

The use-case here is that we use a repository rule to get a build number for stamping from an environment variable. We want changes to the environment variable to invalidate the repository rule (so that the new value will be used). And we want different instantiations of the repository rule (which may in fact be in different repositories) to be able to specify which environment variable they want to use as the build number.

So one repo may say:

make_my_stamping_context(
    name = "stamp_context",
    env_var_name = "BUILD_NUMBER",
)

And another repo may say:

make_my_stamping_context(
    name = "stamp_context",
    env_var_name = "OTHER_BUILD_NUMBER",
)

Because the environ arg of repository_rule is just a static list of strings, the only way to allow env_var_name to be user-specified is to call repository_rule with the env var name.

If there was a way for a repository rule impl to return back "Here are env vars I want to rerun on", like you can specifty files that should trigger reruns, I wouldn't need this functionality any more.

@Wyverald
Copy link
Member

Wyverald commented Sep 8, 2023

If there was a way for a repository rule impl to return back "Here are env vars I want to rerun on", like you can specifty files that should trigger reruns, I wouldn't need this functionality any more.

Hmm. This could be worth pursuing. There's nothing technically preventing us from such an API, as far as I'm aware.

@iancha1992 iancha1992 added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts labels Sep 8, 2023
@meteorcloudy meteorcloudy added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Sep 12, 2023
@meteorcloudy
Copy link
Member

Hmm. This could be worth pursuing. There's nothing technically preventing us from such an API, as far as I'm aware.

Can we file another issue to track this feature request?

@illicitonion
Copy link
Contributor Author

@meteorcloudy I opened #19511 :) Thanks!

@meteorcloudy
Copy link
Member

Thanks! Do we close this one or keep it as a P3?

@Wyverald
Copy link
Member

IMO close. In Bzlmod it's expressly forbidden to instantiate a repo rule that hasn't been exported. Per #10441 this is a bug in the WORKSPACE repo rule code.

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-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: bug
Projects
None yet
Development

No branches or pull requests

6 participants