VPAAMP-856 Fix gmock linking errors by removing .libs/include from test CMakeLists - #1803
Closed
pstroffolino wants to merge 5 commits into
Closed
VPAAMP-856 Fix gmock linking errors by removing .libs/include from test CMakeLists#1803pstroffolino wants to merge 5 commits into
pstroffolino wants to merge 5 commits into
Conversation
Third-party nightly build was failing with undefined references to gmock symbols:
testing::Mock::AllowUninterestingCalls(void const*)
testing::Mock::UnregisterCallReaction(void const*)
Root cause: Multiple test CMakeLists.txt files were explicitly including
${AAMP_ROOT}/.libs/include, which contains old system-installed gmock headers.
These headers have incompatible ABI with the FetchContent gmock libraries,
causing link failures.
Fixed by removing .libs/include from:
- tests/AampLogManagerTests/CMakeLists.txt
- tests/FragmentCollectorMpdTests/CMakeLists.txt
- tests/AampDRMLicManagerTests/CMakeLists.txt
- tests/Scte35Tests/CMakeLists.txt
The FetchContent googletest already provides correct headers globally via
test/utests/CMakeLists.txt lines 101-102. Middleware headers are provided
via conditional logic in fakes/CMakeLists.txt and CommonTestIncludes.cmake.
Related: VPAAMP-856
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes L1 unit test build/link failures caused by test suites accidentally pulling in stale GoogleTest/GoogleMock headers from ${AAMP_ROOT}/.libs/include, which can be ABI-incompatible with the FetchContent-provided gmock libraries configured in test/utests/CMakeLists.txt.
Changes:
- Removed
${AAMP_ROOT}/.libs/includefrom four affected test suiteCMakeLists.txtfiles. - Added explanatory comments to prevent reintroducing
.libs/includeand to point to the supported middleware-header include mechanisms (fakes/CMakeLists.txtandtest/utests/tests/CommonTestIncludes.cmake).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/utests/tests/Scte35Tests/CMakeLists.txt | Removes .libs/include and documents why FetchContent googletest must be used. |
| test/utests/tests/FragmentCollectorMpdTests/CMakeLists.txt | Removes .libs/include and documents the correct middleware include sources. |
| test/utests/tests/AampLogManagerTests/CMakeLists.txt | Removes .libs/include to prevent gmock ABI/header mismatch and documents rationale. |
| test/utests/tests/AampDRMLicManagerTests/CMakeLists.txt | Removes .libs/include to avoid stale gmock headers and documents supported include paths. |
Third-party build has middleware pkg-config files that set includedir to .libs/include, which contains old system-installed gmock headers. When these are added via PLAYERFBINTERFACE_INCLUDE_DIRS etc., they cause ABI mismatch with FetchContent gmock libraries, resulting in undefined reference errors. Added filtering logic in fakes/CMakeLists.txt and CommonTestIncludes.cmake to skip any include directory ending with .libs/include when using pkg-config middleware. This ensures only FetchContent googletest headers are used, preventing the linking errors. The filter uses regex match to detect .libs/include paths and logs a message when skipping them for debugging purposes. Related: VPAAMP-856
Changed approach: Instead of filtering out .libs/include entirely (which removes needed middleware headers like DrmSystems.h and libdash), use include directory ordering to prioritize FetchContent googletest. Added BEFORE SYSTEM for FetchContent gtest/gmock include directories before adding middleware pkg-config paths. This ensures compiler finds FetchContent headers first, even if .libs/include contains old gmock/gtest. .libs/include contains: - Middleware headers (DrmSystems.h, libdash, etc.) - NEEDED - Old gmock/gtest subdirectories - NOT NEEDED (shadowed by FetchContent) The BEFORE SYSTEM directive ensures FetchContent headers take precedence in the search path, preventing ABI mismatch linking errors while still allowing access to middleware headers. Related: VPAAMP-856
Changed pkg_check_modules(GTEST REQUIRED gtest) to optional by removing REQUIRED keyword. This allows builds to proceed when gmock is not pre-installed, relying on FetchContent to provide googletest instead. Third-party builds don't have gmock pre-installed, so the REQUIRED check was causing CMake configuration to fail before FetchContent could run. By making it optional, FetchContent will always provide googletest, and the CMake targets (gtest, gmock) will be properly resolved during linking. The pkg_check_modules call is kept for backward compatibility with systems that do have gmock pre-installed, but FetchContent will override the variables to ensure consistent versions. Related: VPAAMP-856
Contributor
Author
|
This doesn't work on 3rd party test environment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third-party nightly build was failing with undefined references to gmock symbols:
testing::Mock::AllowUninterestingCalls(void const*)
testing::Mock::UnregisterCallReaction(void const*)
Root cause: Multiple test CMakeLists.txt files were explicitly including ${AAMP_ROOT}/.libs/include, which contains old system-installed gmock headers. These headers have incompatible ABI with the FetchContent gmock libraries, causing link failures.
Fixed by removing .libs/include from:
The FetchContent googletest already provides correct headers globally via test/utests/CMakeLists.txt lines 101-102. Middleware headers are provided via conditional logic in fakes/CMakeLists.txt and CommonTestIncludes.cmake.