bazel
bazel copied to clipboard
Allow `native.package_relative_label` in initializers
Work towards #21622
RELNOTES: native.package_relative_label
can now be used in rule initializers.
FYI @Wyverald
I’d feel more comfortable accepting just package_relative_label, because it seems this function doesn’t lead into parsing and concatenating the labels and is covered by a clear usecase. Other functions seem more risky.
I’d feel more comfortable accepting just package_relative_label, because it seems this function doesn’t lead into parsing and concatenating the labels and is covered by a clear usecase. Other functions seem more risky.
I have made the change to only allow this function, but I don't quite follow your reasoning: With native.package_relative_label
, I can implement native.repo_name
as native.package_relative_label("foo").repo_name
. The other functions, with the exception of native.module_version
, can also mostly be derived from that label. We wouldn't really be exposing more information that way, but improve consistency between the APIs available to macros and initializers. Which additional risk do you see?
@comius Friendly ping
I’d feel more comfortable accepting just package_relative_label, because it seems this function doesn’t lead into parsing and concatenating the labels and is covered by a clear usecase. Other functions seem more risky.
I have made the change to only allow this function, but I don't quite follow your reasoning: With
native.package_relative_label
, I can implementnative.repo_name
asnative.package_relative_label("foo").repo_name
. The other functions, with the exception ofnative.module_version
, can also mostly be derived from that label. We wouldn't really be exposing more information that way, but improve consistency between the APIs available to macros and initializers. Which additional risk do you see?
The problem is that once you have native.repo_name
you can implement native.package_relative_label
. But in any such implementation you probably need to make assumptions about how a label is composed of different parts.
Also I see package_relative_label
backed up by a use-case. Other functions aren't.
The problem is that once you have
native.repo_name
you can implementnative.package_relative_label
.
I see your point, just want to remark that this is not possible: native.package_relative_label
crucially applies the calling package's repository mapping to resolve the apparent to a canonical repo name, which is not something you can't emulate with any other API.
@comius I special cased the setup for native.package_relative_label
by just providing the LabelConverter
as a thread local, thus getting rid of the new phase. PTAL, this is much simpler now.
@comius Friendly ping
@bazel-io fork 7.2.0
@comius @iancha1992 Friendly ping, is this still on track for merging?
@comius @iancha1992 Friendly ping, is this still on track for merging?
@fmeum Yes. We are currently working on this :)
@fmeum can you please resolve the conflicts from src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
? There is a conflict that I could not resolve on my own from the file. Thanks :)
@iancha1992 Done