feat: add $(rlocation ...) syntax as a cleaner alternative to $$RUNFILES_DIR/$(rlocationpath ...)#2873
feat: add $(rlocation ...) syntax as a cleaner alternative to $$RUNFILES_DIR/$(rlocationpath ...)#2873acozzette wants to merge 10 commits into
Conversation
…LES_DIR/$(rlocationpath ...) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
env values are quoted and stored as literal strings, so $RUNFILES_DIR would not be evaluated at runtime. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
acozzette
force-pushed
the
adamc/rlocation
branch
from
d92c54c to
a440d5b
Compare
…lib_helpers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8bcba6317
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "build", | ||
| "--config", | ||
| "$$RUNFILES_DIR/$(rlocationpath :rspack_config)", | ||
| "$(rlocation :rspack_config)", |
There was a problem hiding this comment.
So we are adding a magic $(rlocation) util in addition to the "standard" $() utils. Where/who defines those "standard" ones? Are they native bazel?
There was a problem hiding this comment.
Right, this is our own magic one. The standard ones are native Bazel in ctx.expand_location(): https://bazel.build/rules/lib/builtins/ctx#expand_location
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| result.append(remaining[:idx]) | ||
| remaining = remaining[idx + len(prefix):] | ||
|
|
||
| # Parentheses are allowed in Bazel target names and could therefore |
There was a problem hiding this comment.
Can they!? I thought we exclusively escaped them in rules_js because they can't....
There was a problem hiding this comment.
Yep, surprisingly they are explicitly allowed according to the documentation here: https://bazel.build/concepts/labels#target-names
2 participants
This change also exposes the
expand_rlocation_refshelper function injs_lib_helpersso that we can reuse it inrules_webpackand possibly other rulesets where it might be useful.Changes are visible to end-users: yes
js_binarynow supports the$(rlocation ...)syntax infixed_args.Test plan