Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buildifier warns about lack of rules_cc; rules_cc says you don't have to use it #923

Closed
GMNGeoffrey opened this issue Nov 13, 2020 · 4 comments · Fixed by #952
Closed

Buildifier warns about lack of rules_cc; rules_cc says you don't have to use it #923

GMNGeoffrey opened this issue Nov 13, 2020 · 4 comments · Fixed by #952

Comments

@GMNGeoffrey
Copy link

GMNGeoffrey commented Nov 13, 2020

native-cc: Function "cc_library" is not global anymore and needs to be loaded from "@rules_cc//cc:defs.bzl". (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#native-cc)

There is no need to use rules from this repository just yet. If you want to use rules_cc anyway, add the following to your WORKSPACE file
https://github.com/bazelbuild/rules_cc#getting-started

These are clearly not compatible. In addition, the instruction to add rules_cc to your WORKSPACE file seem to be not accurate based on bazelbuild/bazel#8743 (comment).

Please make the tools in the ecosystem and their documentation consistent.

Cross-posted as bazelbuild/rules_cc#92

@vladmos
Copy link
Member

vladmos commented Nov 13, 2020

@lberki to comment on whether the warning should be shown by default.

In general, these warning help users to migrate to a new API before the old API has been deprecated, so the warning is valid despite it's not necessary to load cc rules from the external repository at the moment.

@lberki
Copy link

lberki commented Nov 27, 2020

I think it's better to remove the warning especially if it's worded that way -- I'd love the rules_cc migration to happen soon, but it will take a good while and until then, this is unnecessarily scary. That said, the load() statement still has value:

https://groups.google.com/g/bazel-discuss/c/XNvpWcge4AE

@GMNGeoffrey
Copy link
Author

I think it's better to remove the warning especially if it's worded that way -- I'd love the rules_cc migration to happen soon, but it will take a good while and until then, this is unnecessarily scary. That said, the load() statement still has value:

groups.google.com/g/bazel-discuss/c/XNvpWcge4AE

I don't totally follow this thread. Should I add the loads? I'm currently rather confused by the behavior where I can add a load for load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") even though I haven't configured rules_cc in our WORKSPACE

@chandlerc
Copy link

Any chance we could get some clarity here? It's been a month of having buildifier insist on one thing and the docs on another.

I don't have any preference for what the plan is or how we get there. I just want either clear guidance that adding the loads is a valid and good thing to do, or I want buildifier to not lint on them. Otherwise we continue to miss potentially important buildifier warnings because of the noise of these.

limdor added a commit to limdor/rules_go that referenced this issue Sep 9, 2021
Long time ago Bazel was saying that cc_binary, cc_library, cc_test and the other cc_* rules should be imported from rules_cc.
However rules_cc was always saying that there is no need to use them yet.
After several discussions it was clarified that migration to bazelbuild/rules_cc was put on hold and there is not need to the users that they start using it.
One of the reasons why a person would not want to use rules_cc right now is because there is no release and there is no notion of what is a good commit:
bazelbuild/rules_cc#91
bazelbuild/rules_cc#68

The fact that rules_go depends on rules_cc forces any rules_go user to use rules_cc. Considering that rules_docker depends on rules_go, any rules_docker user is also forced to depend on rules_cc.

More information about the discussions:
bazelbuild/rules_cc#86
bazelbuild/rules_cc#92
bazelbuild/buildtools#923
bazelbuild/buildtools#952

Fixes bazel-contrib#2949
limdor added a commit to limdor/rules_go that referenced this issue Sep 9, 2021
Long time ago Bazel was saying that cc_binary, cc_library, cc_test and the other cc_* rules should be imported from rules_cc.
However rules_cc was always saying that there is no need to use them yet.
After several discussions it was clarified that migration to bazelbuild/rules_cc was put on hold and there is not need to the users that they start using it.
One of the reasons why a person would not want to use rules_cc right now is because there is no release and there is no notion of what is a good commit:
bazelbuild/rules_cc#91
bazelbuild/rules_cc#68

The fact that rules_go depends on rules_cc forces any rules_go user to use rules_cc. Considering that rules_docker depends on rules_go, any rules_docker user is also forced to depend on rules_cc.

More information about the discussions:
bazelbuild/rules_cc#86
bazelbuild/rules_cc#92
bazelbuild/buildtools#923
bazelbuild/buildtools#952

Fixes bazel-contrib#2949
achew22 pushed a commit to bazel-contrib/rules_go that referenced this issue Sep 10, 2021
Long time ago Bazel was saying that cc_binary, cc_library, cc_test and the other cc_* rules should be imported from rules_cc.
However rules_cc was always saying that there is no need to use them yet.
After several discussions it was clarified that migration to bazelbuild/rules_cc was put on hold and there is not need to the users that they start using it.
One of the reasons why a person would not want to use rules_cc right now is because there is no release and there is no notion of what is a good commit:
bazelbuild/rules_cc#91
bazelbuild/rules_cc#68

The fact that rules_go depends on rules_cc forces any rules_go user to use rules_cc. Considering that rules_docker depends on rules_go, any rules_docker user is also forced to depend on rules_cc.

More information about the discussions:
bazelbuild/rules_cc#86
bazelbuild/rules_cc#92
bazelbuild/buildtools#923
bazelbuild/buildtools#952

Fixes: #2949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants