[bazel] glob the c++ files/headers in wpilib_cc_library #8393
Conversation
Signed-off-by: Zach Harel <[email protected]>
github-actions
bot
added
component: ntcore
Signed-off-by: Zach Harel <[email protected]>
|
@AustinSchuh requesting your input :) |
| hdrs = glob([ | ||
| "include/**/*.h", | ||
| "include/**/*.hpp", | ||
| ], allow_empty = True), |
There was a problem hiding this comment.
Why are we allowing empty glob expansions here?
There was a problem hiding this comment.
This also shows up in cscore/BUILD.bazel and shared/bazel/rules/cc_rules.bzl.
| ["src/main/native/cpp/**/*.cpp", | ||
| "src/main/native/cpp/**/*.hpp"], |
| "src/**/*.cc", | ||
| "src/**/*.cpp", | ||
| "src/**/*.h", | ||
| "src/**/*.hpp", |
There was a problem hiding this comment.
There don't seem to be any files in thirdparty/googletest/src that end with .hpp.
| ], allow_empty = True), | ||
| hdrs = glob([ | ||
| "include/**/*.h", | ||
| "include/**/*.hpp", |
There was a problem hiding this comment.
There don't seem to be any files in thirdparty/googletest/include that end with .hpp.
| name = "wpinet", | ||
| srcs = glob( | ||
| ["src/main/native/cpp/**"], | ||
| ["src/main/native/cpp/**/*.cpp"], |
There was a problem hiding this comment.
This is missing the following files:
wpinet/src/main/native/cpp/WebSocketSerializer.hpp
wpinet/src/main/native/cpp/WebSocketDebug.hpp
wpinet/src/main/native/cpp/MulticastHandleManager.hpp
| "src/main/native/cpp/**", | ||
| "src/generated/main/native/cpp/**", | ||
| "src/main/native/cpp/**/*.cpp", | ||
| "src/generated/main/native/cpp/**/*.cpp", |
There was a problem hiding this comment.
Is it a problem that this doesn't include hal/src/generated/main/native/cpp/mrc/protobuf/MrcComm.npb.h?
|
Apart from the other feedback in here, I think we should create 2 macros inside //shared/bazel/rules:cc_rules.bzl which are essentially "glob_headers" and "glob_sources" and have those handle the extensions. That'll make it more uniform between different folders. |
|
I'm not really sure hiding away the globs in a macro makes sense. Many of the packages have different globs by necessity, and hiding the glob behind an abstraction makes it more difficult to find the rule a source file corresponds to (especially for static analysis tools). |
4 participants
i think i caught everything but i may not have