Remove usage of grep_includes

[kotlaja]

No description provided.

[scentini]

cc @meteorcloudy

This change relies on changes that landed in Bazel 6.3, and as such our min version (6.0) builds are failing.

We have the option of:
a) update min bazel version to 6.3 and
b) add if statements around cc_common.link and cc_common.register_linkstamp_compile_action that call the functions with the right parameters.

@UebelAndre @illicitonion , do you have a stance on updating the min supported version to 6.3?

@kotlaja in case the maintainers would like to keep compatibility with 6.0, is there some API exposed that we could use to conditionally select on API versions (e.g a --experimental_drop_grep_includes or cc_common.drop_grep_includes)?

[meteorcloudy]

b) add if statements around cc_common.link and cc_common.register_linkstamp_compile_action that call the functions with the right parameters.

Consider using https://github.com/bazel-contrib/bazel_features to do the conditional selection ;)

You can submit a feature condition here: https://github.com/bazel-contrib/bazel_features/blob/8fbef9fce356d87f8221a217e1e1b6c9dae69604/features.bzl#L10

[illicitonion]

This change relies on changes that landed in Bazel 6.3, and as such our min version (6.0) builds are failing.

We have the option of: a) update min bazel version to 6.3 and b) add if statements around cc_common.link and cc_common.register_linkstamp_compile_action that call the functions with the right parameters.

@UebelAndre @illicitonion , do you have a stance on updating the min supported version to 6.3?

I'm ok with bumping to 6.3.0.

In #415 we discussed basically wanting all of:

• To support the current LTS stream
• To support the previous LTS stream for at least 3 months unless really infeasible
• To support Bazel @ HEAD

I guess the spirit of following that policy is that we'd wait 3 months until bumping from 6.2 to 6.3, but in practice I haven't seen any regressions in minor releases, so don't think there's likely to be a strong need for someone to stay on 6.2 when 6.3 is out.

[meteorcloudy]

It's also worth pointing out that for end users, migrating from 6.2 to 6.3 should be effortless since the newer version should be fully backwards compatible, if not then it's a bug we should fix.

[kotlaja]

@kotlaja in case the maintainers would like to keep compatibility with 6.0, is there some API exposed that we could use to conditionally select on API versions (e.g a --experimental_drop_grep_includes or cc_common.drop_grep_includes)?

@scentini No, there isn't. And there shouldn't be an issue with compatibility since grep_includes are fully internal (externally there is a binary which should never be executed since grep_includes are not supported outside Google). And for internal case I'll come back to you on the internal chat.

I'm ok with bumping to 6.3.0.

@illicitonion Perfect, thank you! When can we expect this to happen? Just to know since this change is blocking me to continue with other changes.

[scentini]

I'd like to hear @UebelAndre's opinion on updating min Bazel to 6.3 too before proceeding with this change. If they prefer to keep compatibility with 6.0 we will ask for this PR to be modified so that rules_rust still supports both 6.0 and 6.3+, by using bazel_features. I've sent a PR over there that will allow us to conditionally remove the grep_includes parameter.

[UebelAndre]

I'd like to hear @UebelAndre's opinion on updating min Bazel to 6.3 too before proceeding with this change. If they prefer to keep compatibility with 6.0 we will ask for this PR to be modified so that rules_rust still supports both 6.0 and 6.3+, by using bazel_features. I've sent a PR over there that will allow us to conditionally remove the grep_includes parameter.

I think we would need to show our min target version continues to work in some way. If there are flags we can make and/or pass to achieve this then I think most changes people wanna make are fine. 😄

[kotlaja]

Thanks @scentini for bumping the min Bazel version supported by rules_rust! I really appreciate it! :)

Related to this PR, the Crate Universe Unnamed Examples test is failing with Permission denied error. Do you know how we can fix this?

[scentini]

I'd like to hear @UebelAndre's opinion on updating min Bazel to 6.3 too before proceeding with this change. If they prefer to keep compatibility with 6.0 we will ask for this PR to be modified so that rules_rust still supports both 6.0 and 6.3+, by using bazel_features. I've sent a PR over there that will allow us to conditionally remove the grep_includes parameter.

I think we would need to show our min target version continues to work in some way. If there are flags we can make and/or pass to achieve this then I think most changes people wanna make are fine. 😄

@UebelAndre we were talking about updating the min version altogether so that we don't have to maintain branches of the likes of
if version < 6.3 ... else for a few months.
I updated the min version to 6.3 just today, in #2150