bazel build support#496
Conversation
…dd an include path of "../src"
|
✅ Build benchmark 886 completed (commit 3eceac86fe by @monkeynova) |
| strip_include_prefix = "src", | ||
| deps = [ | ||
| ], | ||
| defines = ["HAVE_POSIX_REGEX"], |
There was a problem hiding this comment.
note, this won't work for everyone. this is one of the reasons i held off; no way to do this kind of dynamic configuration.
There was a problem hiding this comment.
It's not perfect agreed, but it's also not unprecedented. gflags has a similar hardcoding (https://github.com/gflags/gflags/blob/master/bazel/gflags.bzl). I started a patch for moving some of the defines there to run off of a config but haven't gotten to a PR for that yet (googletest does something similar for linkopts: https://github.com/google/googletest/blob/master/BUILD.bazel).
But I know I don't have complete access to all the build modes I might need. Is there an automated build tool I can use to ensure coverage?
There was a problem hiding this comment.
the travis and appveyor builds are probably the closest we have for covering the various platforms/compilers. there are other platforms that people run on, but we don't have a way to cover every combination.
| find_package(Threads REQUIRED) | ||
| include(CheckCXXCompilerFlag) | ||
|
|
||
| include_directories("../src") |
There was a problem hiding this comment.
this should not be necessary. We don't want the tests to have blanket access to the private parts of src.
for the same reason, you should be using visibility = ["//visibility:private"] on most of the cc_library targets above.
There was a problem hiding this comment.
A fair number of the tests #include "../src/" so the encapsulation is actually pretty bad. Since this was tests as part of the package I wasn't particularly concerned with the model here, but it's a reasonable concern.
As far as the visibility:private, isn't that the default in the build file? When I was trying to make use of this from an external dependency, I had to add the explicit visibility:public to the one rule I wanted to make visible.
There was a problem hiding this comment.
ah, you're right. i wasn't sure how they'd set the default.
they include some things from ../src, but don't have full access. i don't know enough about bazel, but would it be better to add a BUILD in src, a BUILD in test, and manage the visibility of certain src/ libraries to test/ that way?
|
(i just pushed 7f2d2cd which should help the travis xcode builds) |
|
Can you also add a travis entry to test the config? Example here: https://github.com/korfuri/bazel-travis/blob/master/.travis.yml |
|
Made obsolete by #533 |
basic bazel workspace and buildfile -- to accomplish this tests now add an include path of "../src"