fix: search for the `sysroot` in compile args

[lukasoyen]

With the new cc_rules based toolchains, the sysroot on the toolchain is not set. But we can infer it from the command line args a normal compilation with that toolchain would get.

[cloudhan]

@lukasoyen Do you know anyway to query the info from cc_toolchain directly. This creates an unused args on each action run. And is quite wasteful... Better way should fuse it into the toolchain configuration.

[cloudhan]

Seems to be designed to be accessible from cc_toolchain directly

https://github.com/bazelbuild/rules_cc/blob/6be85c266b1df3a5bd38290c814d83ee643185dd/cc/private/rules_impl/cc_flags_supplier_lib.bzl#L57-L62

Then we can attach it to our cuda_info then add a corresponding feature for it.

[lukasoyen]

Seems to be designed to be accessible from cc_toolchain directly

I read through that code while figuring things out. I think that is different and not meant for the rule based toolchains.

• This is the corresponding code for the rule based toolchains. It (and the old variant) use CC_FLAGS_MAKE_VARIABLE_ACTION_NAME
• CC_FLAGS_MAKE_VARIABLE_ACTION_NAME is not used in cc_sysroot
• cc_toolchain_config does not set builtin_sysroot

I think the rule based toolchains are designed in a way to make the builtin_sysroot obsolete.

[lukasoyen]

Do you know anyway to query the info from cc_toolchain directly.

As said in https://github.com/bazel-contrib/rules_cuda/pull/313#discussion_r1921941074 I agree this is wasteful and slow. I don't see a reason to not put the detection into the toolchain config. The CC toolchain is an attribute already at the detection already queries the CC toolchain for the compiler binary.

I am just not sure how all that plumbing would turn out. But I am up to try if you prefer it that way.

Edit: to not put

[cloudhan]

Sorry for the delay. Was busy working on internal projects.

After examining the cc rules implementation https://github.com/bazelbuild/bazel/blob/a3abc625c78439a5ebf1bb09491627c802f2453d/src/main/starlark/builtins_bzl/common/cc/cc_toolchain_provider_helper.bzl#L198-L204 I think cc_toolchain.sysroot is the de facto sysroot what rules_cuda actions can rely on.

We can just ignore libc_top_label things here. See https://github.com/bazelbuild/bazel/blob/a3abc625c78439a5ebf1bb09491627c802f2453d/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java#L796-L800 and https://bazel.build/reference/command-line-reference#flag--grte_top

Then we have a better way to implement this. I will provide you an improved PR for this issue.

[keith]

note that the linked PR does not solve the original issue here. The core issue is that the rules based toolchain just doesn't set sysroot on the toolchain config ever https://github.com/bazelbuild/rules_cc/blob/d74915024017250e46d95e91a3defc34174effe0/cc/toolchains/impl/toolchain_config.bzl#L69-L89 and instead it passes a flag https://github.com/bazelbuild/rules_cc/blob/91bef569ae45917be8175d4629a6157ff120d494/cc/toolchains/args/sysroot.bzl#L29-L43 but otherwise doesn't treat it as special at all. The alternative to this change would be allowing the user to provide the sysroot another way, theoretically another --@rules_cuda flag would work

[keith]

Here's an upstream issue summarizing the state of things https://github.com/bazelbuild/rules_cc/issues/373

but without this change, if you're using a hermetic sysroot, it is not correctly picked up

[hofbi]

@cloudhan should we move on with this feature until rules_cc decide how to move forward. As mentioned by @keith this change is required as of now for rule based toolchains (I was also running into this issue)

[cloudhan]

I think a --@rules_cuda flag might be the prefered way to move forward.

The current implementation is a brutal search, it affects every action invocation. It matches a "--sysroot=<arg>" with "--sysroot" prefix but it might be implemented as "--sysroot", "<arg>" in cc rule :(

[lukasoyen]

I think a --@rules_cuda is not the best idea, as it forces the user to include a weird path into external/ into their .bazelrc.

It matches a "--sysroot=" with "--sysroot" prefix but it might be implemented as "--sysroot", "" in cc rule :(

Yes and no. For the legacy toolchains rules_cc can still take the sysroot attribute from the toolchain. For rules_cc based toolchains the --sysroot will be generated like this. If there is a concern to also match --sysroot <arg>, this PR is easy to adapt.

[cloudhan]

@lukasoyen another (much) better option would be adding a special rule cuda_cc_sysroot to figure out the runtime sysroot and then add the target to the generated @cuda repo and then add it as the implicit deps to all cuda rules, this rules provides CudaInfo https://github.com/bazel-contrib/rules_cuda/blob/2e4e33441d24f87f2236158d104574d76381b153/cuda/private/providers.bzl#L64

We somehow need to extend CudaInfo with host flags tho.

This will be a little bit complex but it is the ultimate solution.

[cloudhan]

supersede by #390