Skip to content

Fix misleading error message - #927

Merged
rickeylev merged 1 commit into
bazel-contrib:mainfrom
suzaku:fix-error-message
Jan 20, 2023
Merged

Fix misleading error message#927
rickeylev merged 1 commit into
bazel-contrib:mainfrom
suzaku:fix-error-message

Conversation

@suzaku

@suzaku suzaku commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

When rctx.which can not find a program for the specified name, it returns None.
We should not override the original python_interpreter in this case, otherwise we'll see a misleading error message like Error in fail: python interpreter `None` not found in PATH

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@suzaku
suzaku requested a review from hrfuller as a code owner December 9, 2022 06:15
When `rctx.which` can not find a program for the specified name, it
returns `None`.
We should not override the original `python_interpreter` in this case,
otherwise we'll see a misleading error message like `Error in fail:
python interpreter `None` not found in PATH`
@suzaku
suzaku force-pushed the fix-error-message branch from 009e121 to a4b15f7 Compare December 9, 2022 06:54

@rickeylev rickeylev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving this error message! I ran into this when I was first getting started and it did leave me a bit perplexed

@rickeylev
rickeylev merged commit 63379a4 into bazel-contrib:main Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants