bazelbuild
Require `data` on `cc_sysroot`?
With cc_sysroot it's valid to provide just something like this:
cc_sysroot(
name = "sysroot",
sysroot = "@sdk//:dir",
)
Without actually propagating the files to the actions that rely on this. As far as I can tell the expected use of this is actually do provide the data alongside this:
cc_sysroot(
name = "sysroot",
data = ["@sdk//:all_files"],
sysroot = "@sdk//:dir",
)
Which makes me wonder if this should be required on the cc_sysroot macro to make it more clear what attribute is expected for this?
Hmm, I wouldn't normally be opposed to it, but it'd be a breaking change, and there might be some use cases for not providing the data attribute.
Maybe we could add a warning instead of an error?
Just wondering: Would a warning actually yield a benefit?
If it's a breaking change but the right thing to do, it'd be something for Bazel 9. In that case a warning likely would be a proper first step. Question, since I'm new to the space: If we added a warning, would we have a way of noticing if it triggers for anyone out there, so we could learn whether there is a use case for not providing the data attribute?
Hmm, I wouldn't normally be opposed to it, but it'd be a breaking change, and there might be some use cases for not providing the data attribute.
Oh is there a purpose of having a sysroot without this?
Just wondering: Would a warning actually yield a benefit?
IMO the benefit, and why I filed this at the time, was because when you're ramping up on these rules writing a new toolchain, that not being required but not doing anything seems to be a distraction, so requiring it would clarify the behavior, since it seems like it is always required?
Oops this slipped through my inbox somehow. IMO data should implicitly be set here so you can simply do the obvious thing of
cc_sysroot(
name = "sysroot",
sysroot = "@sdk//:dir",
)
After all, this is a helper macro designed to simplify things. From there, either a user-specified data should clobber that default, or it should extend the set of included runfiles. I'm leaning towards clobbering, but no particularly strong preference since it's such a tiny helper.
ah yea a default might be nice, but I guess there probably isn't a standard expectation we could have for what that label would be?
Yeah, I think a default would probably be the right solution, since it's non-breaking. You can default it to the sysroot attribute (it has type directory, so it's a valid label).
Oh is there a purpose of having a sysroot without this?
Absolutely, though it isn't common. You can have a sysroot flag declared here, but conditionally include various aspects of it at various times. For example, you could write:
subdirectory(
name = "includes",
parent = "@sdk//:dir",
path = "usr/include",
)
cc_args(
name = "include_args",
actions = ["compile_actions"],
data = [":includes"],
args = ["-I..."]
...
)
cc_args(
name = "shared_libs_args",
data = [":shared_libs"],
actions = [":link_actions"],
...