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

add_psp_module cmake include bug #1070

Closed
jmahoney-dev opened this issue Jan 6, 2021 · 2 comments · Fixed by #1072, #1139 or #1150
Closed

add_psp_module cmake include bug #1070

jmahoney-dev opened this issue Jan 6, 2021 · 2 comments · Fixed by #1072, #1139 or #1150

Comments

@jmahoney-dev
Copy link

Describe the bug
When making PSP modules and calling add_psp_module() from the respective PSP module's CMakeLists.txt file, the necessary includes are not available and the code will fail to compile due to #include "cfe_psp_module.h" not being found.

To Reproduce
Steps to reproduce the behavior:

  1. Create a PSP module in psp/modules/
  2. Create a CMakeLists.txt file in psp/modules/ and add line for add_psp_module(<your module> src/<your module c file>).
  3. Make a .c source file in psp/modules/<your module> and #include "cfe_psp_module.h" in it.
  4. Make sure to include this psp module in target.cmake so it will be built w/ SET(TGT1_PSP_MODULELIST <your module>)
  5. Build cFS

Expected behavior
If you have the proper PSP module boilerplate set up the compile error you should see is that compiler couldn't find "cfe_psp_module.h"

Code snips
I went ahead and modified cfe/cmake/arch_build.cmake function add_cfe_module include_directories line from
include_directories(${MISSION_SOURCE_DIR}/psp/fsw/shared
to
include_directories(${MISSION_SOURCE_DIR}/psp/fsw/shared/inc

...which fixed the problem.

System observed on:

  • Native Linux
  • OS: Ubuntu 18.04
  • Versions [cFE v6.8.0-rc2, OSAL v5.1.0-rc2, PSP v1.5.0-rc1]

Reporter Info
Joe Mahoney - LTA Research

@jphickey
Copy link
Contributor

jphickey commented Jan 6, 2021

I checked the history on this one - looks like this actually introduced by nasa/PSP#172 which split the PSP targets into subdirectories. As part of this the public include files for PSP got moved from fsw/shared to fsw/shared/inc but the reference in the arch_build.cmake in CFE never got updated:

include_directories(${MISSION_SOURCE_DIR}/psp/fsw/shared)

My recommendation:

  1. Patch "bootes" RC to include the missing /inc subdirectory to make it match again. It's definitely an incorrect ref here. The CI builds don't actually depend on this, apparently.
  2. Going forward as we migrate toward CMake interface libraries and target/interface-based includes rather than directory-based includes it should hopefully prevent this from happening again. See issue Utilize CMake interface libraries #626.

FWIW, I think this is a good example of the benefit of interface libraries - it allows the PSP CMakeLists.txt file to declare where the headers actually reside, as opposed to having direct subdir paths sprinkled in other makefiles.

@skliper skliper added this to the 7.0.0 milestone Jan 6, 2021
jphickey added a commit to jphickey/cFE that referenced this issue Jan 6, 2021
The PSP header files are located in fsw/shared/inc, not fsw/shared.
This corrects the reference.
@jphickey jphickey linked a pull request Jan 6, 2021 that will close this issue
astrogeco added a commit that referenced this issue Jan 27, 2021
Fix #1070, patch PSP include directory reference
@astrogeco
Copy link
Contributor

Merged fix into 6.8.x @jphickey can you open a PR to go into main or are you doing the "permanent" fix ?

skliper pushed a commit to skliper/cFE that referenced this issue Jan 28, 2021
The PSP header files are located in fsw/shared/inc, not fsw/shared.
This corrects the reference.
astrogeco added a commit that referenced this issue Feb 3, 2021
Fix #1070 for main branch (merge of 6.8.x after bookkeeping)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment