Fix NPE caused by --repo_env with no value set #28606
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
github-actions
bot
added
the
awaiting-review
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a NullPointerException that occurs when --repo_env is used with an environment variable that is not set. The fix involves adding a null check for the environment variable's value. If the value is null, a warning is issued, and the variable is ignored, preventing the crash. The change is correct and effectively resolves the issue.
|
@bazel-io fork 9.0.1 |
|
@bazel-io fork 9.1.0 |
fmeum
left a comment
fmeum
left a comment
There was a problem hiding this comment.
Could you add test coverage to the tests touched by the culprit?
| repoEnvBuilder.put(name, value); | ||
| nonstrictRepoEnvBuilder.put(name, value); | ||
| } else { | ||
| warnings.add( |
There was a problem hiding this comment.
I don't think we need to (or should) warn here, not being set is an entirely fine state to inherit.
There was a problem hiding this comment.
I thought about it for a moment but then I remembered several occasions when envvars were leaking into build context and the only way to detect that was to compare 2 heavy execution logs. Also, this may be informative to quickly identify differences in local and CI environments. But I am fine to remove this
Done |
JSGette
force-pushed
the
jsgette/28605/fix_NPE_on_repo_env
branch
from
19deb3f to
e94312a
Compare
Wyverald
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
This is to address bazelbuild#28605. bazel crashes with `NullPointerException` in case `--repo_env=NON_EXISTENT` is used with no value set in user's local environment. It worked in `8.5.1` but doesn't in `9.0.0` so it seems like a regression. Closes bazelbuild#28606. PiperOrigin-RevId: 868871905 Change-Id: I0ecd9a4b76b7795e1f00583050aaf343fe930832
This is to address bazelbuild#28605. bazel crashes with `NullPointerException` in case `--repo_env=NON_EXISTENT` is used with no value set in user's local environment. It worked in `8.5.1` but doesn't in `9.0.0` so it seems like a regression. Closes bazelbuild#28606. PiperOrigin-RevId: 868871905 Change-Id: I0ecd9a4b76b7795e1f00583050aaf343fe930832
4 participants
This is to address #28605. bazel crashes with
NullPointerExceptionin case--repo_env=NON_EXISTENTis used with no value set in user's local environment. It worked in8.5.1but doesn't in9.0.0so it seems like a regression.