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

module extension not re-evaluated when env variable is modified #22404

Closed
laurentlb opened this issue May 16, 2024 · 1 comment
Closed

module extension not re-evaluated when env variable is modified #22404

laurentlb opened this issue May 16, 2024 · 1 comment
Assignees
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug untriaged

Comments

@laurentlb
Copy link
Contributor

Description of the bug:

TL;DR: Extension modules that use ctx.getenv are not re-evaluated when the env variable is modified.

Repro.

I have a simple MODULE file:

$ cat ../repro/MODULE.bazel
my_repo = use_extension("//:ext.bzl", "my_repo")
use_repo(my_repo, "repo")

and the extension ext.bzl:

def _impl(ctx):
  print("MYVAR", ctx.getenv("MYVAR"))
  # ...

my_repo = module_extension(implementation = _impl)

Steps:

  1. During the build, notice that the print statement is correctly evaluated.
  2. Modify the environment variable (e.g. MYVAR=abc bazel build ...)
  3. Notice that the print statement is not re-executed.

I've noticed that repository rules are re-evaluated if I use ctx.getenv, but module extensions are not. Is it working as intended?

To me, this seems error-prone and can lead to correctness issues. I fail to see how ctx.getenv can be used in a safe way.

Which category does this issue belong to?

External Dependency

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

No response

Which operating system are you running Bazel on?

MacOS

What is the output of bazel info release?

release 7.1.0

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 HEAD ?

No response

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

No response

Have you found anything relevant by searching the web?

No response

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

No response

@github-actions github-actions bot added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label May 16, 2024
@fmeum
Copy link
Collaborator

fmeum commented May 16, 2024

Can confirm that this was missed when adding getenv. This is still an issue with Bazel 7.2.0rc1 and at HEAD.

The Skyframe dependency is added correctly, but the information is not added to the lockfile.

CC @Wyverald

@fmeum fmeum self-assigned this May 22, 2024
Wyverald pushed a commit that referenced this issue May 25, 2024
Fixes #22404

RELNOTES: Changes to environment variables read via `getenv` now correctly invalidate module extensions.

Closes #22498.

PiperOrigin-RevId: 637058337
Change-Id: Id4aaa4155a728452472eedae4a59c8d29456e512
github-merge-queue bot pushed a commit that referenced this issue May 28, 2024
Fixes #22404

RELNOTES: Changes to environment variables read via `getenv` now
correctly invalidate module extensions.

Closes #22498.

PiperOrigin-RevId: 637058337
Change-Id: Id4aaa4155a728452472eedae4a59c8d29456e512

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug untriaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants