Skip to content

Merge global and config interface targets into unified project interface#171

Open
devin-ai-integration[bot] wants to merge 2 commits into
develfrom
devin/1781141221-merge-interface-targets
Open

Merge global and config interface targets into unified project interface#171
devin-ai-integration[bot] wants to merge 2 commits into
develfrom
devin/1781141221-merge-interface-targets

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 11, 2026

Copy link
Copy Markdown
Related Issue(s) N/A
Has Unit Tests (y/n) n (updated existing test data in test_recursion.cmake)
Documentation Included (y/n) n
Generative AI was used in this contribution (y/n) AI

Change Description

Replaces two separate INTERFACE library targets that served overlapping roles with a single unified target:

BEFORE:
  _fprime_global_interface_target  (API.cmake)        → global include dirs
  __fprime_config                  (config_assembler.cmake) → config module aggregation

AFTER:
  _fprime_project_interface        (project_interface.cmake) → all of the above

New file cmake/project_interface.cmake defines the target. Key structural changes:

  1. FPrime.cmake: Includes project_interface early; after FPRIME_BUILD_LOCATIONS init, populates the target with FPRIME_SOURCE_LOCATIONS / FPRIME_BINARY_LOCATIONS properties and INTERFACE_INCLUDE_DIRECTORIES. fprime_setup_global_includes bridges from the target.

  2. register_fprime_project: Stores source/binary dirs as target properties and include dirs on _fprime_project_interface.

  3. fprime_add_config_build_target: Links config modules and stores FPRIME_CHOSEN_IMPLEMENTATIONS on the unified target. Config module include dirs are explicitly propagated since INTERFACE_INCLUDE_DIRECTORIES reads are not transitive.

  4. fpp.cmake / fpp_ut.cmake: Read -p paths from INTERFACE_INCLUDE_DIRECTORIES via get_target_property instead of FPRIME_BUILD_LOCATIONS.

  5. utilities.cmake: get_nearest_build_root, build_relative_path, full_path_from_build_relative_path read from target properties instead of FPRIME_BUILD_LOCATIONS.

FPRIME_BUILD_LOCATIONS is retained and kept in sync for backwards compatibility.

Rationale

The global interface target and config interface target both served as INTERFACE libraries rolling up global properties. Merging them into a single target with well-defined properties (FPRIME_SOURCE_LOCATIONS, FPRIME_BINARY_LOCATIONS) provides a cleaner architecture and a single source of truth for build location data.

Testing/Review Recommendations

  • Verify a full build of Ref deployment succeeds (configure + build)
  • Verify unit test build (BUILD_TESTING=ON) still works
  • Check that fpp-to-cpp -p receives the correct comma-separated paths
  • Confirm config overrides still work correctly
  • The test_recursion.cmake expected dependency list was updated from __fprime_config_fprime_project_interface

Future Work

  • FPRIME_BUILD_LOCATIONS variable can be fully deprecated once downstream consumers migrate to the target properties
  • fprime_setup_global_includes could be simplified further or removed if all consumers use the target

AI Usage (see policy)

AI was used for code generation and implementation of the refactoring across all affected cmake files. IAMAI

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/96b773c1c96148adb45e9ae1e96830b6


Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Author
Original prompt from michael.d.starch

the fprime cmake system has a global interface target in API.cmake. It has a configuration interface target in config_assembler.cmake

These nominally do the same thing, an interface library to roll up global properties.

Would you merge them.

  1. Creat a new file to store the singular interface target
  2. Use this target in place of both existing targets
  3. Add the properties to store the source and binary locations in register_fprime_project
  4. Add target include directories with the above location
  5. Merge FPRIME_BUILD_LOCATIONS into the above structure
  6. In fpp autocoder use the interface include headers property to set the paths -p flag
  7. In the find nearest build root function use the properties in 2
  8. If reading the interface properties are not recursive in the call to fpp, we will need to add the config module includes to this new target

@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment thread cmake/FPrime.cmake
Comment thread cmake/autocoder/fpp.cmake Outdated
Comment thread Fw/Types/CMakeLists.txt
"${CMAKE_CURRENT_LIST_DIR}/Types.fpp"
DEPENDS
__fprime_config # Only module that should ever list __fprime_config in DEPENDS. Use Fw_Types instead.
_fprime_project_interface # Only module that should ever list _fprime_project_interface in DEPENDS. Use Fw_Types instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Fw_Types now transitively inherits all global include directories via link dependency

The old __fprime_config target was a pure aggregation target that only carried config module link dependencies and FPRIME_CHOSEN_IMPLEMENTATIONS. The new _fprime_project_interface target also carries INTERFACE_INCLUDE_DIRECTORIES (all build root paths, config dirs, etc.). Since Fw_Types depends on _fprime_project_interface (at Fw/Types/CMakeLists.txt:20), and all modules transitively depend on Fw_Types, every module now inherits these include directories via the target dependency chain IN ADDITION to the existing directory-scoped include_directories(). This is functionally redundant (compilation behavior unchanged) but could have implications for INTERFACE_INCLUDE_DIRECTORIES propagation in shared library exports or install targets. This is a subtle behavioral change that may be intentional but should be confirmed.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation. This is an intentional consequence of merging the two targets — the old __fprime_config was already a dependency of Fw_Types, and all modules already received these include directories via the directory-scoped include_directories() bridge. The transitive propagation through the target dependency chain is functionally redundant and shouldn't affect compilation behavior. For install/export scenarios this is worth monitoring but the existing include_directories() bridge already makes these globally visible.

Comment thread cmake/FPrime.cmake
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown

Coverage report — base devel

No baseline branch coverage/devel found. This run becomes the seed once it lands on devel.

Overall (line): 81.12% (no baseline)
Regression threshold: 0.50% (line).

Regressions

(none over threshold)

Modules changed

(no measurable change)

New modules

Module Line Function Branch
CFDP/Checksum 71.15 57.14 53.85
Drv/AsyncByteStreamBufferAdapter 100.00 100.00 100.00
Drv/ByteStreamBufferAdapter 100.00 100.00 100.00
Drv/Ip 44.32 57.38 25.36
Drv/TcpClient 75.00 100.00 47.37
Drv/TcpServer 84.72 100.00 60.00
Drv/Udp 68.09 90.91 42.86
Fw/Buffer 81.25 89.47 58.62
Fw/DataStructures 98.48 97.14 83.21
Fw/Dp 94.83 96.67 95.95
Fw/FilePacket 75.24 89.06 54.30
Fw/Log 75.71 84.21 60.71
Fw/Logger 100.00 100.00 100.00
Fw/SerializableFile 90.00 100.00 79.41
Fw/Time 88.62 85.48 86.84
Fw/Tlm 53.76 60.00 37.50
Fw/Types 53.84 56.01 34.32
Os 18.05 19.48 14.52
Os/Generic 89.10 96.30 67.18
Os/Generic/Types 92.45 91.67 82.14
Os/Posix 62.40 84.21 44.10
Svc/ActiveRateGroup 100.00 100.00 92.31
Svc/ActiveTextLogger 79.05 90.00 71.23
Svc/AssertFatalAdapter 94.74 100.00 86.67
Svc/BufferAccumulator 88.00 94.12 74.49
Svc/BufferLogger 92.62 86.96 80.46
Svc/BufferManager 99.05 100.00 83.64
Svc/BufferRepeater 91.67 100.00 75.00
Svc/ChronoTime 100.00 100.00 100.00
Svc/CmdDispatcher 96.97 91.67 91.75
Svc/CmdSequencer 93.89 97.12 84.63
Svc/CmdSplitter 100.00 100.00 100.00
Svc/ComLogger 97.37 91.67 83.91
Svc/ComSplitter 100.00 100.00 100.00
Svc/ComStub 98.51 100.00 85.19
Svc/DpCatalog 78.00 100.00 66.27
Svc/DpManager 97.44 100.00 100.00
Svc/DpWriter 97.58 90.00 97.22
Svc/FileDownlink 84.15 90.91 71.90
Svc/FileManager 88.41 93.33 82.44
Svc/FileUplink 92.35 96.55 80.95
Svc/FileWorker 90.29 100.00 84.87
Svc/FprimeDeframer 100.00 100.00 97.92
Svc/FprimeFramer 100.00 100.00 95.45
Svc/FprimeRouter 88.89 100.00 78.26
Svc/FpySequencer 86.76 99.04 76.89
Svc/GenericHub 100.00 100.00 85.64
Svc/Health 100.00 100.00 88.66
Svc/LinuxTimer 97.06 100.00 82.35
Svc/OsTime 70.00 83.33 60.98
Svc/PassiveRateGroup 100.00 100.00 89.47
Svc/PolyDb 100.00 100.00 53.57
Svc/PosixTime 100.00 100.00 75.00
Svc/PrmDb 92.75 89.47 88.24
Svc/RateGroupDriver 100.00 100.00 68.75
Svc/SeqDispatcher 73.08 80.00 71.05
Svc/StaticMemory 100.00 100.00 100.00
Svc/SystemResources 98.63 100.00 76.19
Svc/TlmChan 77.59 85.71 63.03
Svc/TlmPacketizer 92.18 100.00 77.45
Svc/Version 96.10 100.00 86.96
Utils 20.04 26.44 24.07
Utils/Types 92.11 95.83 77.08

Modules without UTs

CFDP/Checksum/GTest, Drv/ByteStreamDriverModel, Drv/Interfaces, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/LinuxUartDriver, Drv/Ports, Drv/Ports/DataTypes, FppTestProject/FppTest/interfaces, FppTestProject/FppTest/topology/async, FppTestProject/FppTest/topology/components/Comp, FppTestProject/FppTest/topology/components/Framework, FppTestProject/FppTest/topology/components/Receiver, FppTestProject/FppTest/topology/components/Sender, FppTestProject/FppTest/topology/guarded, FppTestProject/FppTest/topology/ports, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Cmd, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Interfaces, Fw/Obj, Fw/Port, Fw/Ports/CompletionStatus, Fw/Ports/Ready, Fw/Ports/Signal, Fw/Ports/SuccessCondition, Fw/Prm, Fw/SerializableFile/test/TestSerializable, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Cycle, Svc/DpPorts, Svc/Fatal, Svc/FatalHandler, Svc/FileDownlinkPorts, Svc/FprimeProtocol, Svc/Interfaces, Svc/PassiveConsoleTextLogger, Svc/Ping, Svc/PolyIf, Svc/Ports/CommsPorts, Svc/Ports/FilePorts, Svc/Ports/OsTimeEpoch, Svc/Ports/TlmPacketizerPorts, Svc/Ports/VersionPorts, Svc/Sched, Svc/Seq, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/FileHandling, Svc/Types/TlmPacketizerTypes, Svc/WatchDog, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestChainedAutocoder, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestTargetAutocoder, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent

devin-ai-integration Bot and others added 2 commits June 12, 2026 00:54
Replace the two separate INTERFACE library targets:
- _fprime_global_interface_target (API.cmake) for global include dirs
- __fprime_config (config_assembler.cmake) for config module aggregation

with a single _fprime_project_interface target defined in the new
project_interface.cmake file. This target stores:
- FPRIME_SOURCE_LOCATIONS / FPRIME_BINARY_LOCATIONS properties
- INTERFACE_INCLUDE_DIRECTORIES for all build locations
- FPRIME_CHOSEN_IMPLEMENTATIONS from config modules
- Config module link dependencies

Key changes:
- FPrime.cmake populates the target with initial build locations
- register_fprime_project adds source/binary location properties
- fpp/fpp_ut autocoders read -p paths from INTERFACE_INCLUDE_DIRECTORIES
- get_nearest_build_root reads from target properties
- Config module include dirs propagated to unified target (step 8)
- FPRIME_BUILD_LOCATIONS retained for backwards compatibility

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>
- Resolve symlinks on FPRIME_SOURCE_BUILD_LOCATIONS and
  FPRIME_BINARY_BUILD_LOCATIONS before storing in target properties,
  matching the existing resolve_path_variables(FPRIME_BUILD_LOCATIONS)
  behavior. Fixes sub-build failures where symlinked framework paths
  didn't match resolved input paths in get_nearest_build_root.

- Use FPRIME_SOURCE_LOCATIONS + FPRIME_BINARY_LOCATIONS (module root
  paths only) instead of INTERFACE_INCLUDE_DIRECTORIES for the fpp -p
  flag. INTERFACE_INCLUDE_DIRECTORIES includes non-root config include
  dirs that could cause incorrect module path resolution.

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>
@devin-ai-integration devin-ai-integration Bot force-pushed the devin/1781141221-merge-interface-targets branch from 03f24fa to 1d7e92b Compare June 12, 2026 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants