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

FR: allow ctx.which to watch for PATH/ program changes #24864

Open
rickeylev opened this issue Jan 8, 2025 · 2 comments
Open

FR: allow ctx.which to watch for PATH/ program changes #24864

rickeylev opened this issue Jan 8, 2025 · 2 comments
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request untriaged

Comments

@rickeylev
Copy link
Contributor

Description of the feature request:

In short: Add a watch=True arg to ctx.which(). This makes it parse PATH and watch for $program to exist/change in a directory before where it first found $program. Unless that condition is met, the which() result is not invalidated. The pesudo-code for this would be something like:

Background:

The ctx.which() function is used to find a program by looking for it in the locations specified by the PATH environment variable. Thus, changing PATH could change the result of which(), depending on how path changed and where a program was found.

Today, changes to the PATH environment variable won't invalidate any lookups using which(). This makes some sense because it's pretty context specific as to whether it matters or not. However, this is problematic for cases which do care if the result would have changed. My go-to example is when configuring a language toolchain using the local system (non-ideal, but IMHO, quite reasonable when users intentionally want to use a custom toolchain for whatever reason).

While a repository rule can call getenv("PATH") to watch the path environment variable, this ends up being too noisy -- tools like git may modify PATH when running hooks (bazelbuild/rules_python#2551 (comment)) in such a way that wouldn't invalidate most which() calls.

Which category does this issue belong to?

External Dependency

What underlying problem are you trying to solve with this feature?

Making a repository rule correctly watch for changes to its inputs while not invalidating when unnecessary.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

8.0.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

Have you found anything relevant by searching the web?

Searched all, didn't find a mention of this.

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

This feature request originates from discussion in bazelbuild/rules_python#2551

@github-actions github-actions bot added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Jan 8, 2025
@fmeum
Copy link
Collaborator

fmeum commented Jan 8, 2025

What do you think of the alternative of watching the binary that is returned? That's something you can already do today and it sounds more reliable to me (lower false negative and positive rate).

@rickeylev
Copy link
Contributor Author

rickeylev commented Jan 8, 2025

i.e, given which(watch=True, ...) it would automatically watch the found binary?

A repo rule can do that today, so it's a small, but welcome, quality-of-life improvement IMHO.

However, it doesn't address the case that user code can't implement today: if PATH changes in such a way that which()'s value would be different. i.e. these cases:

  • A new PATH entry is added before the matched-entry and $newEntry/$program exists
  • The original PATH entry that matched is removed from PATH.
  • $program starts existing at a PATH entry before where it was originally found. It'd be easy to make which() also handle this (just watch all earlier entries) if started watching the found binary.

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: feature request untriaged
Projects
None yet
Development

No branches or pull requests

5 participants