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

Fix #906, Update variable checks in read_targetconfig #907

Merged

Conversation

jphickey
Copy link
Contributor

Describe the contribution

CMake script was using a "DEFINED" test to check if these variables were set. Problem discovered is that this is always true because "SIMULATION" is a cache var set from an environment variable, so it ALWAYS defined, it is just empty if not being used.

Fix is to use if (SIMULATION) rather than if (DEFINED SIMULATION) which should only be true if the string is not empty, as intended. Also applying this to ${CPUNAME}_SYSTEM so if someone does e.g. set(cpu1_SYSTEM) then it won't try to use that empty string either.

Fixes #906

Testing performed
Build for Vxworks 6.9 using GSFC build machine
Build for native using both SIMULATION=native and without SIMULATION (using toolchain-cpu1.cmake)

Expected behavior changes
Builds without SIMULATION directive work as expected

System(s) tested on

  • Vxworks 6.9 using GSFC build machine
  • Native on Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

This was using "DEFINED" to check if these variables were set.
Problem discovered is that this is always true because "SIMULATION"
is a cache var set from an environment variable, so it ALWAYS defined,
it is just empty if not being used.

Fix is to use if (SIMULATION) rather than if (DEFINED SIMULATION) which
should only be true if the string is not empty, as intended.
@jphickey jphickey added CCB:FastTrack CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 23, 2020
@jphickey
Copy link
Contributor Author

Suggesting fast track on this one since its broken in main

@jphickey jphickey requested a review from skliper September 23, 2020 15:26
@yammajamma yammajamma added CCB-20200923 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 23, 2020
@yammajamma yammajamma changed the base branch from main to integration-candidate September 24, 2020 12:48
@yammajamma yammajamma merged commit ce187a2 into nasa:integration-candidate Sep 24, 2020
@jphickey jphickey deleted the fix-906-cmake-system branch September 29, 2020 21:52
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake build not correctly using the "${CPUNAME}_SYSTEM" directive
4 participants