[bazel] Split gtest and gmock and become compatible with Bazel incompatible changes #2364
Conversation
Yannic
force-pushed
the
incompatible_bazel
branch
from
eb62f31 to
e2aeb39
Compare
Yannic
force-pushed
the
incompatible_bazel
branch
from
e2aeb39 to
e499987
Compare
googletest/src/gtest-port.cc
Outdated
| @@ -83,7 +83,7 @@ | |||
| #include "gtest/gtest-message.h" | |||
| #include "gtest/internal/gtest-internal.h" | |||
| #include "gtest/internal/gtest-string.h" | |||
| #include "src/gtest-internal-inl.h" | |||
| #include "gtest-internal-inl.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
googletest/src/gtest-printers.cc
Outdated
| @@ -48,7 +48,7 @@ | |||
| #include <ostream> // NOLINT | |||
| #include <string> | |||
| #include "gtest/internal/gtest-port.h" | |||
| #include "src/gtest-internal-inl.h" | |||
| #include "gtest-internal-inl.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
googletest/src/gtest-test-part.cc
Outdated
| @@ -31,7 +31,7 @@ | |||
| // The Google C++ Testing and Mocking Framework (Google Test) | |||
|
|
|||
| #include "gtest/gtest-test-part.h" | |||
| #include "src/gtest-internal-inl.h" | |||
| #include "gtest-internal-inl.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
googletest/src/gtest.cc
Outdated
| @@ -119,7 +119,7 @@ | |||
| # include <sys/types.h> // NOLINT | |||
| #endif | |||
|
|
|||
| #include "src/gtest-internal-inl.h" | |||
| #include "gtest-internal-inl.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
| @@ -32,7 +32,7 @@ | |||
| // Google Test work. | |||
|
|
|||
| #include "gtest/gtest.h" | |||
| #include "googletest-param-test-test.h" | |||
| #include "test/googletest-param-test-test.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
googletest/test/production.cc
Outdated
| @@ -30,6 +30,6 @@ | |||
| // | |||
| // This is part of the unit test for gtest_prod.h. | |||
|
|
|||
| #include "production.h" | |||
| #include "test/production.h" | |||
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
googletest/test/gtest_prod_test.cc
Outdated
| #include "gtest/gtest.h" | ||
| #include "test/production.h" |
There was a problem hiding this comment.
How is this related to the bazel change? Please revert and just leave the bazel related changes.
|
@Yannic Thank you for your PR, please take a look at the comments. In general it is vastly preferred to only have 1 change in the PR |
|
@gennadiycivil Thanks for your review! The changes to C++ files are necessary because the new Bazel build is more hermetic than the old one was. I.e. the old one leaked headers in src as public headers. The C++ changes should also work with the old Bazel build. If you prefer, we can split them into a new PR and merge them first. |
|
Yes please split them into a separate PR. The code structure is not that easy to change , because there are hidden transformations beween the internal code that has somewhat different structure and the OSS code here. We would have to see proof that the code the way it is has issues with new Bazel. |
Yannic
force-pushed
the
incompatible_bazel
branch
from
1b0d3fd to
5c22842
Compare
|
We should make this work without code structure alterations. Bazel changes do not require that the project code change. They only ask to load rules from a different place. |
|
Okay, let's split this PR again to get a "load rules_{cc,python}" and a "split gtest and gmock" change then, ack? |
I think the "load rules_{cc,python}" is definitely valuable. The second one you would have to prove that it is indeed an improvement - a strong supporting case would have to be built. Thank you for this! |
|
I'll do some more investigations regarding the split into gtest and gmock. Closing for now. |
Fixes googletest for upcoming
--incompatible_load_cc_rules_from_bzl(bazelbuild/bazel#8743) and--incompatible_load_python_rules_from_bzl(bazelbuild/bazel#9006).This change also makes the build more hermetic.