facebook
cxx_library not respecting compiling .c/.h files in C mode
buck2 build :bin
cxx_binary(
name = "bin",
srcs = [
"lua.c",
],
deps = [
":lua",
],
)
cxx_library(
name = "lua",
srcs = [
"lapi.c",
"lapi.h",
"lcode.c",
"lcode.h",
"lctype.c",
"lctype.h",
"ldebug.c",
"ldebug.h",
"ldo.c",
"ldo.h",
"ldump.c",
"lfunc.c",
"lfunc.h",
"lgc.c",
"lgc.h",
...
Errors:
error: invalid argument '-std=c++17' not allowed with 'C'
Which is correct, but I don't see way to force cxx_library to use C and I'm also not quite sure why it's not being autodetected as a c target.
-std=c++17 is passed as a cxx_flag to CxxCompilerInfo . CCompilerInfo has no flags passed to it.
I figured out what it was. The autodetection fails without warning if you add a .h file to the srcs list. I think that should be an allowable extension to be passed for C targets.
Ah, you should be putting the headers in a separate headers argument. You don't compile headers, so it makes sense to differentiate them. I'll see if we can make srcs of .h an error, since no one should want that.
I'm tweaking converting a priorly bazel project right now, which handles this situation pretty differently than buck2 does. It makes sense but it was not what I was expecting :)
Yeah, I'm testing a change internally to make it say Not allowed to have header files in the `srcs` attribute - put them in `headers` which hopefully fixes it for good. Might take a while to ship, as I imagine there are a few cases where it relies on that, but with time.
The reason we have to support headers is because of https://github.com/facebook/buck2/blob/da5373151c7f4f4d6d08666c37a2d23a3aef83dc/prelude/cxx/compile.bzl#L187-L190. While it doesn't say, that is there to support compilation databases, so if a target has no C++ files, we can generate a compilation database by pretending the headers are really C++ files. Not convinced that is a good idea. Following up internally.
Plan is to slightly more tightly scope the compilation database fix, and then it will error if you put a header in srcs, with a good error message.
Why do headers need a compilation database if they're not compiled? Can't the headers have their database appear in the target that depends on the headers?
For info, I have a diff ready internally that makes the headers in srcs an error.
As to @calebkiage's question, imagine you are writing an IDE, and someone opens a .c file. The best way to generate intelligent suggestions etc is to find the BUCK file that contains the .c file (easy, must be the closest one above it) ask that BUCK file what targets it has and their sources (easy, call buck2 targets) and then generate the compilation database for the .c file (easy, call buck2 build thetarget[compilation_database]). For headers, you need to find something that compiles it. If there is a .c in the same target, that probably includes the header, and will be sufficient. If it is header only, you need to search for a target that depends on the header-only target - which might be anywhere in a multi-million file repo. Or you can do a little bit of a cheat, pretend the header files are .c files, and compile them locally. That means the process looks much the same as for .c files.
Thanks for the response @ndmitchell. I have a question about this technique though. How does it work with different compilation options? For example, if the same header is included in a .cpp file that's compiled with C++11 and in another file that is compiled with C++20, the suggestions have to differ when showing suggestions, no? It isn't possible to know which suggestions to show if the header file is a source file on its own since the build system has to pick one standard. Unless I'm missing some context?
@calebkiage - Normally things won't differ too much between C++11 and C++20 (or based on define or similar), so its close enough to have value. Given a single large repo that compiles some things one way, some things another, without knowing the project you are working on its impossible to give a perfect answer.
Since 67bdf15d699955498b953d82d3606c862c1ac49a it is an error to put a .h in the srcs field, so hopefully whoever runs into this next will have a good error message and know how to fix it.