Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++][CMake] Use transitive dependency for system GoogleTest #38339

Closed
kou opened this issue Oct 19, 2023 · 0 comments · Fixed by #38340
Closed

[C++][CMake] Use transitive dependency for system GoogleTest #38339

kou opened this issue Oct 19, 2023 · 0 comments · Fixed by #38340

Comments

@kou
Copy link
Member

kou commented Oct 19, 2023

Describe the enhancement requested

Our bundled GoogleTest is built with FetchContent. It uses targets configured by GoogleTest's CMake directly. They have transitive dependencies (target_link_libraries(gtest_main PUBLIC gtest)). So we don't need to link gtest_main and gtest. We just need to use gest_main. gtest is also linked automatically.

But GoogleTest related targets found by CMake's FindGTest.cmake don't have transitive dependencies. So we need to link gtest_main and gtest explicitly. If we set transitive dependencies to GoogleTest related targets found by CMake's FindGTest.cmake, we can simplify our link targets.

Component(s)

C++

kou added a commit to kou/arrow that referenced this issue Oct 19, 2023
kou added a commit to kou/arrow that referenced this issue Oct 27, 2023
@kou kou closed this as completed in #38340 Nov 2, 2023
kou added a commit that referenced this issue Nov 2, 2023
#38340)

### Rationale for this change

Our bundled GoogleTest has transitive dependency. So we can omit explicit `GTest::gtest` linking because `GTest::gtest_main`/`GTest::gmock` has transitive dependency.

But GoogleTest related targets found by CMake's `FindGTest.cmake` don't have transitive dependency. So our code base needs to link `GTest::gtest` explicitly.

We can remove needless links by setting transitive dependency to GoogleTest related targets found by CMake's `FindGTest.cmake`.

### What changes are included in this PR?

* Set transitive dependencies to `GTest::gtest_main` and `GTest::gmock`
* Remove needless links

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: #38339

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 15.0.0 milestone Nov 2, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…gleTest (apache#38340)

### Rationale for this change

Our bundled GoogleTest has transitive dependency. So we can omit explicit `GTest::gtest` linking because `GTest::gtest_main`/`GTest::gmock` has transitive dependency.

But GoogleTest related targets found by CMake's `FindGTest.cmake` don't have transitive dependency. So our code base needs to link `GTest::gtest` explicitly.

We can remove needless links by setting transitive dependency to GoogleTest related targets found by CMake's `FindGTest.cmake`.

### What changes are included in this PR?

* Set transitive dependencies to `GTest::gtest_main` and `GTest::gmock`
* Remove needless links

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: apache#38339

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…gleTest (apache#38340)

### Rationale for this change

Our bundled GoogleTest has transitive dependency. So we can omit explicit `GTest::gtest` linking because `GTest::gtest_main`/`GTest::gmock` has transitive dependency.

But GoogleTest related targets found by CMake's `FindGTest.cmake` don't have transitive dependency. So our code base needs to link `GTest::gtest` explicitly.

We can remove needless links by setting transitive dependency to GoogleTest related targets found by CMake's `FindGTest.cmake`.

### What changes are included in this PR?

* Set transitive dependencies to `GTest::gtest_main` and `GTest::gmock`
* Remove needless links

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: apache#38339

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant