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

cmake: Fix support for non-standard psp source directory #1864

Conversation

jbohren-hbr
Copy link
Contributor

Describe the contribution
This PR fixes a bug which prevents non-standard psp source directory specification via the environment variable $CFS_APP_PATH or the cmake variable ${psp_SEARCH_PATH}.

The cFE cmake build infrastructure uses the environment / cmake variable $CFS_APP_PATH
as well as the cmake meta-variable pattern ${${APP}_SEARCH_PATH} to locate apps and other modules (such as psp), and subsequently sets the variable pattern ${${APP}_MISSION_DIR} (see mission_build.cmake lines 202-222).

However, the resulting variable ${psp_MISSION_DIR} is not used in process_arch() in arch_build.cmake line 588 when including a target platform's build_options.cmake file. This causes a cmake configuration failure when trying to use a non-standard psp location.

Testing performed
Build with psp in non-standard location, with the CFS_APP_PATH environment variable set:

# set up standard cFS bundle per https://github.com/nasa/cFS
git clone --recursive https://github.com/nasa/cFS.git
cd cFS
cp -R cfe/cmake/sample_defs .
cp cfe/cmake/Makefile.sample Makefile
# hide psp
mkdir alt
mv psp alt/psp
# define alternate search path
export CFS_APP_PATH=$(pwd)/alt
# build sample mission
make prep
make

Before this PR:

...

-- Generated cfe_platform_cfg.h from /***REDACTED**/cFS/sample_defs configuration
CMake Error at cmake/arch_build.cmake:588 (include):
  include could not find load file:

    /***REDACTED***/cFS/psp/fsw/pc-linux/make/build_options.cmake
Call Stack (most recent call first):
  CMakeLists.txt:124 (process_arch)


-- Configuring incomplete, errors occurred!

...

After this PR:

...

-- Generated cfe_platform_cfg.h from /***REDACTED***/cFS/sample_defs configuration
-- BSP Selection: generic-linux at /***REDACTED***/cFS/osal/src/bsp/generic-linux
-- OSAL Compile Definitions: _LINUX_OS_;_XOPEN_SOURCE=600
-- OSAL Selection: posix at /***REDACTED**/cFS/osal/src/os/posix
-- PSP Selection: pc-linux

...

Built target mission-all

Expected behavior changes
This should only affect projects which use the environment variable $CFS_APP_PATH or the cmake variable ${psp_SEARCH_PATH}. Currently, they are being ignored for psp's build_options.cmake path resolution, which means some users may be building with different target build options than they think they are based on the intended behavior.

System(s) tested on

  • Hardware: PC
  • OS: Ubuntu 18.04
  • Versions: cFE v6.8.0-rc1+dev933

Additional context

Aside, this is also suggested as a method to specify a non-standard psp source directory in mission_defaults.cmake line 67 since PR #751 with the following:

set(psp_SEARCH_PATH ".")

However, this will lead to the same cmake configure error as above without this fix.

Third party code
No thirdy party code included in this contribution.

Contributor Info - All information REQUIRED for consideration of pull request
Jonathan Bohren, Honeybee Robotics

@skliper skliper requested a review from jphickey August 24, 2021 18:44
@astrogeco astrogeco added the community Community contribution, YAY! label Aug 24, 2021
@astrogeco
Copy link
Contributor

Thanks for the contribution @jbohren-hbr ! I opened a related issue to help us write the version description document. Can you double check it and add comments as needed?
#1921

@astrogeco astrogeco linked an issue Aug 31, 2021 that may be closed by this pull request
@jbohren-hbr
Copy link
Contributor Author

#1921 looks good to me!

@astrogeco astrogeco changed the base branch from main to integration-candidate August 31, 2021 22:48
@astrogeco astrogeco added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Aug 31, 2021
@astrogeco astrogeco merged commit f11b728 into nasa:integration-candidate Aug 31, 2021
astrogeco pushed a commit to astrogeco/cFE that referenced this pull request Sep 1, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Sep 1, 2021
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Sep 1, 2021
@astrogeco
Copy link
Contributor

CCB:2021-09-01 APPROVED

astrogeco added a commit to nasa/cFS that referenced this pull request Sep 1, 2021
**Combines**

nasa/cFE#1885,              v6.8.0-rc1+dev980
nasa/osal#1138,             v5.1.0-rc1+dev598
nasa/cFS-GroundSystem#195,  v2.2.0-rc1+dev63

**Includes**

*cFE*

nasa/cFE#1870, Add SB API test cases
nasa/cFE#1869, Add ES API test cases
nasa/cFE#1872, Add TBL API test cases
nasa/cFE#1871, Add FS API test cases
nasa/cFE#1860, Add Time Clock Test
nasa/cFE#1862, EVS coverage test
nasa/cFE#1876, SB test improvements
nasa/cFE#1865, CFE_TBL_Modified: Test CRC, updated flag
nasa/cFE#1881, Improve EVS code coverage
nasa/cFE#1877, add call to CFE_ES_ExitChildTask
nasa/cFE#1902, Incorrect OSAL Format in Users Guide Reference
nasa/cFE#1884, Improve FS coverage
nasa/cFE, Improve MSG branch coverage
nasa/cFE#1891, Improve resource ID branch coverage
nasa/cFE#1894, Improve SBR branch coverage
nasa/cFE#1896, Fix #1895, Improve TIME branch coverage
nasa/cFE#1904, Improve TBL code coverage
nasa/cFE#1864, Support custom PSP directory
nasa/cFE#1913, Update time tests to use bitmask check macros
nasa/cFE#1923, remove extra word in comment

*osal*

nasa/osal#1136, add bitmask assert macros

*cFS-GroundSystem*

nasa/cFS-GroundSystem#190, Fix #189, Virtualenv and Pipenv .gitignore support
nasa/cFS-GroundSystem#194, Fix doc, comment, and message typos

Co-authored-by: Jacob Hageman           <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey           <jphickey@users.noreply.github.com>
Co-authored-by: Alex Campbell           <zanzaben@users.noreply.github.com>
Co-authored-by: Ariel Adams             <ArielSAdamsNASA@users.noreply.github.com>
Co-authored-by: Jose F Martinez Pedraza <pepepr08@users.noreply.github.com>
Co-authored-by: Avi                     <thnkslprpt@users.noreply.github.com>
Co-authored-by: Paul                    <pavll@users.noreply.github.com>
@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
Labels
build-system CCB:Approved Indicates code review and approval by community CCB community Community contribution, YAY!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot specify custom PSP directory using psp_MISSION_DIR
4 participants