-
Notifications
You must be signed in to change notification settings - Fork 218
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
Fix #1289, cmake script modernization #1291
Merged
Merged
Conversation
This file contains 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
Use target properties to define interfaces and compiler definitions rather than referencing global variables or using "known" paths in other modules. This better aligns with current practices and creates a more robust build environment that is less dependent on specific path names existing in a given module. This adds an "osal_public_api" interface target that contains the paths to the public API headers as its INTERFACE_INCLUDE_DIRECTORIES and any required compiler definitions as its INTERFACE_COMPILE_DEFINITIONS property. Applications should use this rather than referring to the "${OSAL_SOURCE_DIR}/src/os/inc" include path directly.
jphickey
force-pushed
the
fix-1289-cmake-cleanup
branch
from
September 29, 2022 16:35
4e65e99
to
66f2d4a
Compare
jphickey
added
the
CCB:Ready
Pull request is ready for discussion at the Configuration Control Board (CCB)
label
Sep 29, 2022
Approved. Confirmation that testing on VxWorks is preferable before merging this pull request. |
Update - I did confirm the build using ppc-vxworks6.9 and all was fine, no issues observed. |
dzbaker
added
CCB:Approved
Indicates code review and approval by community CCB
and removed
CCB:Ready
Pull request is ready for discussion at the Configuration Control Board (CCB)
labels
Oct 11, 2022
2 tasks
dzbaker
added a commit
to nasa/cFS
that referenced
this pull request
Oct 11, 2022
*Combines:* cfe v7.0.0-rc4+dev193 cFS-GroundSystem v3.0.0-rc4+dev33 osal v7.0.0-rc4+dev131 to_lab v2.5.0-rc4+dev31 ci_lab v2.5.0-rc4+dev39 sample_app v1.3.0-rc4+dev35 sample_lib v1.3.0-rc4+dev28 tblCRCTool v1.3.0-rc4+dev24 elf2cfetbl v3.4.0-rc4+dev26 sch_lab v2.5.0-rc4+dev41 **Includes:** *cFS* - #567 - #514 *cFE* - nasa/cFE#2163 - nasa/cFE#2158 - nasa/cFE#2159 *osal* - nasa/osal#1283 - nasa/osal#1291 - nasa/osal#1298 *sample_app* - nasa/sample_app#185 - nasa/sample_app#183 *sch_lab* - nasa/sch_lab#123 *tblCRCTool* - nasa/tblCRCTool#73 *to_lab* - nasa/to_lab#127 - nasa/to_lab#126 - nasa/to_lab#129 *ci_lab* - nasa/ci_lab#123 - nasa/ci_lab#120 *sample_lib* - nasa/sample_lib#89 - nasa/sample_lib#86 *cFS-GroundSystem* - nasa/cFS-GroundSystem#224 - nasa/cFS-GroundSystem#225 *elf2cfetbl* - nasa/elf2cfetbl#117 Co-authored-by: Avi Weiss <[email protected]> Co-authored by: Joseph Hickey <[email protected]> Co-authored by: Ariel Adams <[email protected]> Co-authored by: Sam Price <[email protected]>
dzbaker
added a commit
to nasa/cFS
that referenced
this pull request
Oct 11, 2022
*Combines:* cfe v7.0.0-rc4+dev193 cFS-GroundSystem v3.0.0-rc4+dev33 osal v7.0.0-rc4+dev131 to_lab v2.5.0-rc4+dev31 ci_lab v2.5.0-rc4+dev39 sample_app v1.3.0-rc4+dev35 sample_lib v1.3.0-rc4+dev28 tblCRCTool v1.3.0-rc4+dev24 elf2cfetbl v3.4.0-rc4+dev26 sch_lab v2.5.0-rc4+dev41 **Includes:** *cFS* - #567 - #514 *cFE* - nasa/cFE#2163 - nasa/cFE#2158 - nasa/cFE#2159 *osal* - nasa/osal#1283 - nasa/osal#1291 - nasa/osal#1298 *sample_app* - nasa/sample_app#185 - nasa/sample_app#183 *sch_lab* - nasa/sch_lab#123 *tblCRCTool* - nasa/tblCRCTool#73 *to_lab* - nasa/to_lab#127 - nasa/to_lab#126 - nasa/to_lab#129 *ci_lab* - nasa/ci_lab#123 - nasa/ci_lab#120 *sample_lib* - nasa/sample_lib#89 - nasa/sample_lib#86 *cFS-GroundSystem* - nasa/cFS-GroundSystem#224 - nasa/cFS-GroundSystem#225 *elf2cfetbl* - nasa/elf2cfetbl#117 Co-authored-by: Avi Weiss <[email protected]> Co-authored by: Joseph Hickey <[email protected]> Co-authored by: Ariel Adams <[email protected]> Co-authored by: Sam Price <[email protected]>
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.
Checklist (Please check before submitting)
Describe the contribution
Use target properties to define interfaces and compiler definitions rather than referencing global variables or using "known" paths in other modules. This better aligns with current practices and creates a more robust build environment that is less dependent on specific path names existing in a given module.
This adds an "osal_public_api" interface target that contains the paths to the public API headers as its INTERFACE_INCLUDE_DIRECTORIES and any required compiler definitions as its INTERFACE_COMPILE_DEFINITIONS property. Applications should use this rather than referring to the "${OSAL_SOURCE_DIR}/src/os/inc" include path directly.
Fixes #1289
Testing performed
Build and confirm OSAL successfully builds on various platforms including Ubuntu 22.04 (new-ish CMake) and 18.04 (old-ish CMake).
Expected behavior changes
No impact to runtime behavior (no actual source code changes here)
Build script makes more use of CMake "INTERFACE" libraries to communicate compiler options, include paths, and definitions to dependencies, moving away from using global variables to perform this task. Notably, this permits the targets to be imported into another build more easily.
System(s) tested on
Ubuntu 18.04 and 22.04 (native, newer and older CMake)
RTEMS 4.11 and 5.0
Additional context
All changes should be backward compatible, in that the "magic variables" are still being honored as the current version of the CMake script had previously implemented:
The preferred method of configuring these items going forward would be to get/set the corresponding properties on the
osal_public_api
interface target for include directories and compile options, and use theut_coverage_compile
andut_coverage_link
interface targets for coverage testing flags. Defining both methods for now allows for transition.If the OSAL_OMIT_DEPRECATED flag is set, then the UT_COVERAGE flags will not be exported.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.