Fix common depending on configuration in CMake targets - #537
Merged
Conversation
christian-schilling
previously approved these changes
Jul 31, 2026
rolandreichweinbmw
previously approved these changes
Aug 4, 2026
rolandreichweinbmw
left a comment
Contributor
There was a problem hiding this comment.
LGTM - just rebase to resolve the documentation merge conflict.
matthiaskessler
dismissed stale reviews from rolandreichweinbmw and christian-schilling
via
August 11, 2026 09:01
c749ac5
"common" is a foundational, header-only interface used by dozens of libraries, while "configuration" is executable-specific. "common" had ended up linking "configuration" directly, inverting the intended dependency direction and forcing every consumer of "common" to transitively depend on an application's configuration. - Remove the leftover duplicate "commonImpl" target from executables/referenceApp/configuration/CMakeLists.txt; its source file was already compiled into "configuration" and nothing linked against it. - Restore libs/bsw/common as a pure INTERFACE library (include dirs, etl, platform) with no reference to "configuration". - Have "configuration" (referenceApp and unitTest variants) link PUBLIC "common", the correct direction. - common::busid::BusIdTraits::getName() is declared in "common" but implemented in each executable's "configuration" library. libs/bsw/transport and platforms/s32k1xx/bsp/canflex2Transceiver call it directly, so link "configuration" PUBLIC from those targets themselves, matching the existing precedent in libs/bsw/uds. Reorder link lists so "configuration" appears after "transport", since static libraries are resolved left to right. Add a "module + moduleImpl" section to doc/dev/guidelines/module.rst describing this pattern: keep the generic interface target free of implementation-specific dependencies, and put those in a separate impl target instead.
matthiaskessler
force-pushed
the
cr-1214460
branch
from
August 11, 2026 09:30
c749ac5 to
2d1ace1
Compare
matthiaskessler
requested review from
christian-schilling and
rolandreichweinbmw
August 11, 2026 09:30
rolandreichweinbmw
approved these changes
Aug 11, 2026
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.
Fix common depending on configuration in CMake targets
"common" is a foundational, header-only interface used by dozens of
libraries, while "configuration" is executable-specific. "common"
had ended up linking "configuration" directly, inverting the
intended dependency direction and forcing every consumer of
"common" to transitively depend on an application's configuration.
executables/referenceApp/configuration/CMakeLists.txt; its source
file was already compiled into "configuration" and nothing linked
against it.
etl, platform) with no reference to "configuration".
PUBLIC "common", the correct direction.
implemented in each executable's "configuration" library.
libs/bsw/transport and platforms/s32k1xx/bsp/canflex2Transceiver
call it directly, so link "configuration" PUBLIC from those
targets themselves, matching the existing precedent in
libs/bsw/uds. Reorder link lists so "configuration" appears after
"transport", since static libraries are resolved left to right.
Add a "module + moduleImpl" section to
doc/dev/guidelines/module.rst describing this pattern: keep the
generic interface target free of implementation-specific
dependencies, and put those in a separate impl target instead.