cxx_builtin_include_directories package resolution uses deprecated function

[t-rad679]

Description of the bug:

The logic which expands package directories for cxx_builtin_include_directories uses the deprecated Label.relative function here

Which category does this issue belong to?

C++ Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

According to @Wyverald, this should cause issues when you have a package in a repo with @ (for example, `"%package(@repo//foo/bar)%/baz/quux") when Bzlmod is enabled.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[Wyverald]

This is blocking Pigweed's Bzlmod migration: https://issues.pigweed.dev/issues/258836641#comment4

[Wyverald]

@comius do you think it's feasible to introduce a label attr for the CcToolchainConfigInfo provider, similar to what's being discussed in #20334? Fixing the existing usage pattern requires introducing new APIs (ctx.package_relative_label maybe) which I'd rather avoid doing.

[fmeum]

Could this be worked around by resolving the package paths in a rule initializer, where native.package_relative_label is available?

[Wyverald]

The problem is that this is in a provider. So unless the author of the rule pays attention to have a macro and use native.package_relative_label to translate the string, we'd always get a bad input. Having the attr type be a label to begin with makes it less error-prone.

[fmeum]

That would need to be a new attribute though: There also needs to be a way to pass in absolute paths (see the auto-configured Unix toolchain).

[Wyverald]

yeah so it would be a new label attribute alongside the existing string attribute. Though I just realized that it's a string list right now, so ordering might be tricky... gah.

[Wyverald]

Discussed more with @t-rad679 and @brandjon; we're probably going to go with a new ctx.package_relative_label API.

The awkwardness with using native.package_relative_label is that the "producer rule" (the rule returning the provider) will necessarily need to have a label-typed attribute, which in turn means that the label must point to an existing target (since we don't have nodep labels in Starlark), which is annoying because there's no guaranteed existing label in a given package. The alternative would be to let the macro then str() the label back to a canonical label string and keep using a string list attr, but that's becoming incredibly convoluted at that point.

Of course, there's downsides with ctx.package_relative_label too -- mostly that it might be non-obvious to BUILD rule authors that they'd get back a Label, not a Target.