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

Crash on using TEST_NAME as an environment variable name in rules #16094

Closed
varungandhi-src opened this issue Aug 12, 2022 · 4 comments · May be fixed by #16102
Closed

Crash on using TEST_NAME as an environment variable name in rules #16094

varungandhi-src opened this issue Aug 12, 2022 · 4 comments · May be fixed by #16102
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: bug

Comments

@varungandhi-src
Copy link

varungandhi-src commented Aug 12, 2022

Description of the bug:

Passing an environment variable with the name TEST_NAME causes Bazel to crash. Renaming it to (say) MY_TEST_NAME fixes the issue.

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

Repo with minimal example: https://github.com/varungandhi-src/bazel-crash/tree/042d1747b4e76c3e9e089cd72222bd3b5a36fdf5

Test with bazel test //....

The main problematic part seems to be this is this in x.bzl:

    native.sh_test(
      name = name,
      srcs = ["hello.sh"],
      deps = [],
      env = {
        "TEST_NAME": "ohno", # Using TEST_NAME creates a collision
      },
    )

Which operating system are you running Bazel on?

macOS 12.5

What is the output of bazel info release?

release 5.2.0-homebrew

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 ?

❯ git remote get-url origin; git rev-parse main; git rev-parse HEAD
https://github.com/varungandhi-src/bazel-crash.git
042d1747b4e76c3e9e089cd72222bd3b5a36fdf5
042d1747b4e76c3e9e089cd72222bd3b5a36fdf5

Have you found anything relevant by searching the web?

There is a similar error in #14681

Caused by: java.lang.IllegalArgumentException: Multiple entries with same key: @bazel_tools//tools/cpp:empty=[] and @bazel_tools//tools/cpp:empty=[]

But the cause seems to be different.

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

No response

@varungandhi-src varungandhi-src changed the title Crash on using TEST_NAME as an environment variable name for ctx.actions.run Crash on using TEST_NAME as an environment variable name in rules Aug 12, 2022
@illicitonion
Copy link
Contributor

Currently bazel hard-codes setting the TEST_NAME env var itself. It's unclear whether whether this should be overridable - on the one hand, TEST_NAME isn't specified at https://bazel.build/reference/test-encyclopedia so it feels like it should be overrideable. On the other hand, other tooling may be surprised by this override.

At the very least, though, we shouldn't crash. Either we should allow overriding (and have it work), or forbid it (with documentation and a clear error message).

@varungandhi-src
Copy link
Author

Not crashing and emitting a useful error message would be a reasonable solution IMO. 👍🏽

blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 15, 2022
This fixes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 15, 2022
This fixes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 15, 2022
This closes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 15, 2022
This closes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 15, 2022
This closes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Aug 16, 2022
This closes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
blindpirate added a commit to blindpirate/bazel that referenced this issue Oct 6, 2022
This closes bazelbuild#16094

Previously Bazel will crash if test uses reserved env variable
(like `TEST_NAME`). Let's report an error in this case.
@brandjon brandjon added team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts and removed team-Build-Language labels Nov 5, 2022
@comius comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Nov 17, 2022
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 22, 2024
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 22, 2024
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) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants