Support using external repositories for builtin_sysroot in create_cc_toolchain_config_info

[armandomontanez]

Description of the feature request:

Pigweed doesn't have a checked in sysroot to reference in create_cc_toolchain_config_info(), and instead pulls down a sysroot hosted through CIPD. Right now, the only way I've found to reference the contents of this package is through "external/linux_sysroot". For cxx_builtin_include_directories, we use strings like "%package(@llvm_toolchain//)%/include/c++/v1" to reference include directories provided by external repositories, it would be nice for this syntax to be supported for builtin_sysroot as well.

Which category does this issue belong to?

No response

What underlying problem are you trying to solve with this feature?

Not relying on the exec-root-relative external/* path to reference contents of an external repository when specifying builtin_sysroot for a cc toolchain.

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 master; git rev-parse HEAD ?

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

[t-rad679]

I'm now looking into this.

Is there any reason we couldn't extract the logic here and then call it here?

Seems pretty straightforward.

[armandomontanez]

Yeah, I think it's that simple. Thanks for picking this up! :)

[t-rad679]

I was talking with @comius about this and he asked me to find out if it's possible for regular labels for your use case. He is hesitant to introduce more parsing and extra syntax in more cases. If it is not possible or preferable for some reason, he approved the solution I mentioned above.

[Wyverald]

Using a label would indeed be nicer as it would avoid the headache associated with parsing a label in Starlark (what is the current repo/package, where does the repo mapping come from, etc.) Essentially, avoid #21801.

[t-rad679]

I think it should work if we replace this with this:

if toolchain_config_info.builtin_sysroot() != "":
        default_sysroot = toolchain_config_info.builtin_sysroot()
    elif toolchain_config_info.builtin_sysroot_label() is not None:
        default_sysroot = toolchain_config_info.builtin_sysroot_label().package

    if attributes.libc_top_label == None:
        sysroot = default_sysroot
    else:
        sysroot = attributes.libc_top_label.package

Thoughts?

[t-rad679]

builtin_sysroot_label would be new

[armandomontanez]

I'd much prefer a label pointing to a package. I only mentioned the other syntax in the bug since there was precedence and the compatibility story was obvious.

Naming is always "fun," but perhaps sysroot_package would be more self-documenting? I'm not sure what the builtin in builtin_sysroot even refers to.

[tpudlik]

Does bazelbuild/rules_cc@0069837 address this issue, or is there more to be done?

@armandomontanez @matts1

[matts1]

Should be addressed in the commit just before that. For an example, take a look at https://chromium-review.googlesource.com/c/chromiumos/bazel/+/5595076 (it got reverted due to a bug in Cq-Depend, but the code is correct).