Allow native.package_relative_label in initializers

[fmeum]

Work towards #21622

RELNOTES: native.package_relative_label can now be used in rule initializers.

[fmeum]

FYI @Wyverald

[comius]

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.

[fmeum]

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?

[fmeum]

@comius Friendly ping

[comius]

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?

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.

[comius]

Introducing a new phase was avoided before and introducing it now for a single function looks like an overkill.

This also potentially interacts with other locations that use phases. And potentially also with symbolic macros.

@brandjon

[fmeum]

The problem is that once you have native.repo_name you can implement native.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.

[fmeum]

@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.

[fmeum]

@comius Friendly ping

[fmeum]

@bazel-io fork 7.2.0

[fmeum]

@comius @iancha1992 Friendly ping, is this still on track for merging?

[iancha1992]

@comius @iancha1992 Friendly ping, is this still on track for merging?

@fmeum Yes. We are currently working on this :)

[iancha1992]

@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 :)

[fmeum]

@iancha1992 Done