Use in-repo gtest in buck build #6858
Conversation
Summary: ... so that we have freedom to upgrade it (see facebook#6808). As a side benefit, gtest will no longer be linked into main library in buck build. Test Plan: fb internal build & link
There was a problem hiding this comment.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Thanks @pdillinger for the PR. Mostly LGTM. Could you run python buckifier/buckify_rocksdb.py?
TARGETS
Outdated
| @@ -114,6 +109,15 @@ ROCKSDB_LIB_DEPS = [ | |||
| ":rocksdb_test_lib", | |||
| ] if not is_opt_mode else [":rocksdb_lib"] | |||
|
|
|||
| # Add a directory for #include | |||
There was a problem hiding this comment.
How about moving these closer to line 60?
TARGETS
Outdated
| cpp_library( | ||
| name = "rocksdb_third_party_gtest", | ||
| srcs = ["third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc"], | ||
| headers = [u'third-party/gtest-1.8.1/fused-src/gtest/gtest.h'], |
There was a problem hiding this comment.
I think u'<string>' is not allowed.
There was a problem hiding this comment.
Thanks @pdillinger for the PR. Mostly LGTM. Could you run
python buckifier/buckify_rocksdb.py?
Considering I've modified the generation code and there are corresponding changes to TARGETS, I obviously did, but who knows what's different in the GitHub build. Maybe it's just the problem with the existing implementation of add_library generating a u string for headers.
Move gtest_dir include into header by making a template Print python version used for TARGETS file check
|
@pdillinger has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pdillinger has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pdillinger merged this pull request in aaafcb8. |
Summary: ... so that we have freedom to upgrade it (see #6808).
As a side benefit, gtest will no longer be linked into main library in
buck build.
Test Plan: fb internal build & link