Add ctx.package_relative_label
#28102
Conversation
fmeum
requested review from
a team,
Wyverald and
meteorcloudy
as code owners
github-actions
bot
added
team-Configurability
|
FYI @dzbarsky, this is the missing piece for path mapping support |
|
@bazel-io fork 9.0.0 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ctx.package_relative_label, a new function for creating Label objects relative to the current rule's package, which is a valuable addition for Starlark rule authors. The implementation is generally sound and the accompanying tests are thorough. However, I've identified a potential issue where a missing mutability check could lead to a NullPointerException if the context is accessed after its intended lifecycle. I've provided a comment with a suggested fix for this.
src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java
Show resolved
Hide resolved
|
The answer is probably obvious, but I'm guessing we decided " |
That combination could be used to parse all relevant attributes once to convert all user-specified labels to canonical form, then forward this mapping via a special public I wouldn't want to encourage this pattern though as it seems overly tricky and unnecessarily inefficient (it parses the attrs twice). The new API has such small scope and mirrors a method that exists in a different context that I believe it's warranted. |
| <p><i>Usage note:</i> The difference between this function and \ | ||
| <a href='../builtins/Label.html#Label'>Label()</a></code> is \ | ||
| that <code>Label()</code> uses the context of the package of the <code>.bzl</code> file \ | ||
| that called it, not the package of the target currently being analyzed.""", |
There was a problem hiding this comment.
could you add a note here mentioning that this is effectively the same as native.package_relative_label? and vice versa for the docs of native.package_relative_label, and maybe look at Label() docs where it mentions native.package_relative_label to make it mention both instead.
all in all, it would be nice if our documentation treated the two as equals, instead of one being mentioned everywhere and the other being an afterthought.
There was a problem hiding this comment.
I added references to Label, Label#relative and native.package_relative_label.
fmeum
requested review from
a team,
fweikert,
gregestren and
lberki
as code owners
fmeum
removed request for
a team,
dabanki,
fweikert,
gregestren,
lberki and
meteorcloudy
This is necessary to permit Starlark implementations of `ctx.expand_location`, which can be more memory efficient, support path mapping and provide better defaults. RELNOTES: The new `package_relative_label` function on the rule context (`ctx`) can be used to turn a user-provided label string into a `Label` relative to the target that is currently being analyzed (where `Label(...)` would return a `Label` relative to the `.bzl` file containing the call).
…StarlarkRuleContext.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
fmeum
force-pushed
the
ctx.package_relative_label
branch
from
c535b8f to
00d7f4c
Compare
fmeum
force-pushed
the
ctx.package_relative_label
branch
from
5fdcdc3 to
18f6d10
Compare
Wyverald
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
This is necessary to permit Starlark implementations of `ctx.expand_location`, which can be more memory efficient, support path mapping and provide better defaults. RELNOTES: The new `package_relative_label` function on the rule context (`ctx`) can be used to turn a user-provided label string into a `Label` relative to the target that is currently being analyzed (where `Label(...)` would return a `Label` relative to the `.bzl` file containing the call). Closes #28102. PiperOrigin-RevId: 853170854 Change-Id: Ibecff3b0b599da0be2fbbba707c15ed540c6c38c
This is necessary to permit Starlark implementations of `ctx.expand_location`, which can be more memory efficient, support path mapping and provide better defaults. RELNOTES: The new `package_relative_label` function on the rule context (`ctx`) can be used to turn a user-provided label string into a `Label` relative to the target that is currently being analyzed (where `Label(...)` would return a `Label` relative to the `.bzl` file containing the call). Closes #28102. PiperOrigin-RevId: 853170854 Change-Id: Ibecff3b0b599da0be2fbbba707c15ed540c6c38c
This is necessary to permit Starlark implementations of `ctx.expand_location`, which can be more memory efficient, support path mapping and provide better defaults. RELNOTES: The new `package_relative_label` function on the rule context (`ctx`) can be used to turn a user-provided label string into a `Label` relative to the target that is currently being analyzed (where `Label(...)` would return a `Label` relative to the `.bzl` file containing the call). Closes #28102. PiperOrigin-RevId: 853170854 Change-Id: Ibecff3b0b599da0be2fbbba707c15ed540c6c38c Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
2 participants
This is necessary to permit Starlark implementations of
ctx.expand_location, which can be more memory efficient, support path mapping and provide better defaults.RELNOTES: The new
package_relative_labelfunction on the rule context (ctx) can be used to turn a user-provided label string into aLabelrelative to the target that is currently being analyzed (whereLabel(...)would return aLabelrelative to the.bzlfile containing the call).