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 using bazel from PATH #265

Open
rickeylev opened this issue Jan 7, 2024 · 2 comments
Open

FR: Allow using bazel from PATH #265

rickeylev opened this issue Jan 7, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@rickeylev
Copy link

rickeylev commented Jan 7, 2024

The basic use case is to allow easily using the same bazel that the user uses to run their builds. The main use case I have in mind for this is when you're using a custom build of bazel specified using USE_BAZEL_VERSION. e.g. if one does:

USE_BAZEL_VERSION=/tmp/bazel-dev bazel //my:test

Then the version of bazel at /tmp/bazel-dev will be used, without having to go into the MODULE.bazel config (or elsewhere) to reconfigure this. Having to do that is a hassle and changes the name of generated targets.

I think the bazel_binaries.local() tag class can be easily adopted for this purpose:

  • Add an attribute, shell_command, which takes a single word, e.g. "bazel", "mybazel", "bazel-dev", etc.
  • In _local_bazel_binary_repo, call repository_ctx.which(shell_command) to get the actual path
  • Add USE_BAZEL_VERSION and PATH to the repo rule's environment variables that affect it.

The end result looks something like:

bazel_binaries.local(shell_command="bazel", name="self")

I was able to emulate this by using a local path to a script that just re-execs using the command "bazel", but it'd be nicer if I didn't have to plumb through a wrapper to achieve that.

@cgrindel cgrindel added the enhancement New feature or request label Jan 7, 2024
@cgrindel
Copy link
Member

cgrindel commented Jan 7, 2024

This sounds reasonable to me. Do you have an implementation that you want to share? The updates as you describe them sound about right. Below are some additional thoughts on the implementation:

  1. Add an integration test to exercise the local(shell_command="...").
  2. Inside the handler for the local tag class, we should error if a client specified shell_command and path.

@katre Do you have any thoughts?

@katre
Copy link
Collaborator

katre commented Jan 8, 2024

Yes, I think this makes sense (and @rickeylev's approach is what I would do).

I'm happy to review a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants