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

Documentation issue: Build Tutorial - C #12007

Open
bbulkow opened this issue Aug 25, 2020 · 2 comments
Open

Documentation issue: Build Tutorial - C #12007

bbulkow opened this issue Aug 25, 2020 · 2 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)

Comments

@bbulkow
Copy link

bbulkow commented Aug 25, 2020

Documentation URL: https://docs.bazel.build/versions/master/tutorial/cpp.html

There is one task everyone building a C or CPP project needs: to specify the language level. (eg, C99 for C files, C++11 for CPP files). This isn't really optional any more, given the proliferation of standards.

I've spent an hour reading, and what's confusing is Starlark is "coming", or something, but some documents are dated 2018 and some are dated with roadmap items in 2020 so it's very hard to tell what to do right now, or based on your version of Starlark and/or Bazel. I read there "is" one way, and there "will be" another way, but the "new way" was checked into the code base in 2018....

At this exact point ( Bazel 3.4.1 ), is one required to place options on the command line? Or environment variables? Or are they safe to put in the build file (which is clearly the result of the roadmap)?

From the documentation, it appears that "copts" is allowed in the cc_binary object, but, confusingly, it applies to "C" and "C++" equally. Of course, language specification strings in C are of the form "-std=c17" and C++ strings are "-std=c++17", so clearly stating in the cc_binary object how to set each independently, again, even for a simple project, is crucial.

Please put this in the tutorial, it is not an advanced topic, it is a necessity for first setting up a basic and initial C/C++ project.

While you are at it, how about updating the documentation here:

https://docs.bazel.build/versions/master/be/c-cpp.html

since it covers details of the cc_binary rules. It contains text like "Deprecated. Use toolchain_identifier attribute instead. It will be a noop after CROSSTOOL migration to Starlark , and will be removed by #7075.
When set, it will be used to perform crosstool_config.toolchain selection. It will take precedence over --cpu Bazel option."

7075 was checked in 2018, so either "compiler" has been removed, either crosstool migration to starlark has been completed, or it's a noop, or it's not. 2018 was a while ago.

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: documentation (cleanup) labels Aug 27, 2020
@oquenchil
Copy link
Contributor

Apart from the documentation fixes it seems like in your text there is a separate feature request to add support to cc_binary to allow different options for C and for C++. Would it be something like this #2954?

@bbulkow
Copy link
Author

bbulkow commented Sep 8, 2020

Actually, no. #2954 seems too heavy.

You have created confusion by having 'copts' apply to C++ even though bazel itself did not do that with command line opts, and 30 years of *nix build systems have two separate environment variables. Since this choice is so deeply "counterintuitive", I spent a lot of hours assuming otherwise.

But that ship sailed.... so what now?

First, DOCUMENT. The fact that you've broken with both tradition and bazel itself should have several clear lines that 'copts' applies to all C++ and all C invocations. My lost time was several hours of reading including the bazel code tree, as I couldn't believe Bazel had done that. I'm actually fine with the feature as it stands, because I've moved to 100% C++ and would have C as a library probably.

Second, have two different variables. It's tradition, it's bazel's command line solution, it works.

However, since you've used 'copts' for C++, I don't know how to do it without breaking backward compatibility. Thus, either break backward compatibility ( fine with me personally! ), or come up with two new variable names, or have some janky "if both are set do the right thing and if only copts is set use the old way".

I believe simply supporting two variables is far better than the complexity envisioned in 2954.

I also considered asking for a "language level" (std=) flag. However, the STD settings for compilers have become very stable and thus if 'copts' and 'cppopts' were properly supported, adding 'std' to each of them would be good enough.

Thanks

@ShreeM01 ShreeM01 added the team-Documentation Documentation improvements that cannot be directly linked to other team labels label Jan 10, 2023
@keertk keertk added P2 We'll consider working on this in future. (Assignee optional) and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

5 participants