-
-
Notifications
You must be signed in to change notification settings - Fork 173
feat: add $(rlocation ...) syntax as a cleaner alternative to $$RUNFILES_DIR/$(rlocationpath ...) #2873
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
feat: add $(rlocation ...) syntax as a cleaner alternative to $$RUNFILES_DIR/$(rlocationpath ...) #2873
Changes from all commits
73f086b
baeb9ab
a440d5b
6b9afcc
54da0de
45d2cbb
2aed59f
c8bcba6
bc0e903
b84b6c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,6 +190,36 @@ def gather_runfiles( | |
| for target in deps | ||
| ]) | ||
|
|
||
| def expand_rlocation_refs(value): | ||
| """Pre-processes $(rlocation <label>) into $$RUNFILES_DIR/$(rlocationpath <label>). | ||
|
|
||
| After this transformation, the standard expand_locations/expand_variables chain | ||
| resolves the rlocationpath and converts $$ to a literal $. | ||
| """ | ||
| result = [] | ||
| remaining = value | ||
| prefix = "$(rlocation " | ||
| for _ in range(len(value)): | ||
| idx = remaining.find(prefix) | ||
| if idx == -1: | ||
| break | ||
| result.append(remaining[:idx]) | ||
| remaining = remaining[idx + len(prefix):] | ||
|
|
||
| # Parentheses are allowed in Bazel target names and could therefore | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can they!? I thought we exclusively escaped them in rules_js because they can't....
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, surprisingly they are explicitly allowed according to the documentation here: https://bazel.build/concepts/labels#target-names |
||
| # theoretically appear in a label. We just look for the first closing | ||
| # parenthesis without handling that possibility, as this is what Bazel | ||
| # itself does: | ||
| # https://github.com/bazelbuild/bazel/blob/b4f16d988ddebc1891d7af57622ec5aff149a838/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java#L179 | ||
| end = remaining.find(")") | ||
|
acozzette marked this conversation as resolved.
|
||
| if end == -1: | ||
| fail("Unclosed $(rlocation ...) in: " + value) | ||
| label_str = remaining[:end] | ||
| result.append("$$RUNFILES_DIR/$(rlocationpath " + label_str + ")") | ||
|
jbedard marked this conversation as resolved.
|
||
| remaining = remaining[end + 1:] | ||
| result.append(remaining) | ||
| return "".join(result) | ||
|
|
||
| LOG_LEVELS = { | ||
| "fatal": 1, | ||
| "error": 2, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| """Tests for js_helpers utility functions""" | ||
|
|
||
| load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") | ||
| load("//js/private:js_helpers.bzl", "expand_rlocation_refs") | ||
|
|
||
| def _no_rlocation_refs_test(ctx): | ||
| env = unittest.begin(ctx) | ||
| asserts.equals(env, "build", expand_rlocation_refs("build")) | ||
| return unittest.end(env) | ||
|
|
||
| def _single_rlocation_ref_test(ctx): | ||
| env = unittest.begin(ctx) | ||
| asserts.equals( | ||
| env, | ||
| "$$RUNFILES_DIR/$(rlocationpath :rspack_config)", | ||
| expand_rlocation_refs("$(rlocation :rspack_config)"), | ||
| ) | ||
| return unittest.end(env) | ||
|
|
||
| def _rlocation_ref_with_surrounding_text_test(ctx): | ||
| env = unittest.begin(ctx) | ||
| asserts.equals( | ||
| env, | ||
| "--config=$$RUNFILES_DIR/$(rlocationpath :my_config)", | ||
| expand_rlocation_refs("--config=$(rlocation :my_config)"), | ||
| ) | ||
| return unittest.end(env) | ||
|
|
||
| def _multiple_rlocation_refs_test(ctx): | ||
| env = unittest.begin(ctx) | ||
| asserts.equals( | ||
| env, | ||
| "$$RUNFILES_DIR/$(rlocationpath :foo):$$RUNFILES_DIR/$(rlocationpath :bar)", | ||
| expand_rlocation_refs("$(rlocation :foo):$(rlocation :bar)"), | ||
| ) | ||
| return unittest.end(env) | ||
|
|
||
| def _absolute_label_rlocation_ref_test(ctx): | ||
| env = unittest.begin(ctx) | ||
| asserts.equals( | ||
| env, | ||
| "$$RUNFILES_DIR/$(rlocationpath //some/pkg:target)", | ||
| expand_rlocation_refs("$(rlocation //some/pkg:target)"), | ||
| ) | ||
| return unittest.end(env) | ||
|
|
||
| no_rlocation_refs_test = unittest.make(_no_rlocation_refs_test) | ||
| single_rlocation_ref_test = unittest.make(_single_rlocation_ref_test) | ||
| rlocation_ref_with_surrounding_text_test = unittest.make(_rlocation_ref_with_surrounding_text_test) | ||
| multiple_rlocation_refs_test = unittest.make(_multiple_rlocation_refs_test) | ||
| absolute_label_rlocation_ref_test = unittest.make(_absolute_label_rlocation_ref_test) | ||
|
|
||
| def js_helpers_test_suite(name): | ||
| unittest.suite( | ||
| name, | ||
| no_rlocation_refs_test, | ||
| single_rlocation_ref_test, | ||
| rlocation_ref_with_surrounding_text_test, | ||
| multiple_rlocation_refs_test, | ||
| absolute_label_rlocation_ref_test, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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