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

OSAL CMake script should not force/overwrite CMAKE_C_FLAGS #312

Closed
jphickey opened this issue Dec 12, 2019 · 0 comments · Fixed by #404
Closed

OSAL CMake script should not force/overwrite CMAKE_C_FLAGS #312

jphickey opened this issue Dec 12, 2019 · 0 comments · Fixed by #404
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
The OSAL build script currently configures several compiler flags by directly setting the CMAKE_C_FLAGS variable, and also setting it differently for unit test vs. FSW code.

This was originally done for compatibility with very old CMake versions but this is no longer necessary, as any reasonable CMake version (including v2.8.12 distributed in RHEL/Centos6+) have better commands to deal with target-specific flags (e.g. target_compile_definitions, etc).

Overriding this variable is not ideal as it is expected to retain its value from the parent.

To Reproduce
Build OSAL using the current script and ENABLE_UNIT_TESTS=TRUE. CMAKE_C_FLAGS is forcibly reset twice during the CMakeLists.txt evaluation.

Expected behavior
CMAKE_C_FLAGS should not be modified by the OSAL build script. It should preserve whatever value the parent had set (if any) and use the preferred commands (e.g. target_compile_options, etc) to manage the different flags required for UT and normal FSW code.

System observed on:
Ubuntu 18.04 LTS, 64 bit

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Dec 12, 2019
jphickey added a commit to jphickey/osal that referenced this issue Feb 26, 2020
Do not clobber the CMAKE_C_FLAGS value as part of the OSAL build.
Instead, use target_compile_options and target_include_directories
as needed to set the compile options for specific targets.

This also creates a separate CMakeLists.txt file for each OS/BSP
implementation library rather than using aux_source_directory.
Each implementation-specific build can then set any additional
options as required for that platform.
jphickey added a commit to jphickey/osal that referenced this issue Apr 2, 2020
Do not clobber the CMAKE_C_FLAGS value as part of the OSAL build.
Instead, use target_compile_options and target_include_directories
as needed to set the compile options for specific targets.

This also creates a separate CMakeLists.txt file for each OS/BSP
implementation library rather than using aux_source_directory.
Each implementation-specific build can then set any additional
options as required for that platform.

Note that any entity needing to compile/link with OSAL should
now obtain the requisite compile flags and directories by querying
the INTERFACE_COMPILE_DEFINITIONS and INTERFACE_INCLUDE_DIRECTORIES
properties on the osal library target.
astrogeco added a commit that referenced this issue Apr 16, 2020
Fix #261, #312, and #362, OSAL build cleanup (multiple issues)
@astrogeco astrogeco added this to the v5.1.0 milestone Sep 28, 2020
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.

3 participants