Remove unused CMake functions and default.cmake#172
Remove unused CMake functions and default.cmake#172devin-ai-integration[bot] wants to merge 2 commits into
Conversation
* Moving more towards independent builds of F Prime * Replace missing settings.ini * Minor changes
Original prompt from michael.d.starch
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Coverage report — base
|
| 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 |
97.94 | 97.14 | 82.29 |
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.00 | 84.21 | 43.87 |
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
Remove the following never-called functions from the CMake build system: - cmake/target/default.cmake: add_global_target, add_deployment_target, add_module_target (unprefixed defaults never dispatched by the target system) - cmake/utilities.cmake: build_relative_path, full_path_from_build_relative_path, on_any_changed, on_changed, read_from_lines, filter_lists Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>
9197ffe to
5b89ff1
Compare
| set(LINK_SCOPE) | ||
| if (NOT FPRIME_USE_PLAIN_LINK_SIGNATURE) | ||
| set(LINK_SCOPE PRIVATE) | ||
| endif() | ||
| fprime_target_dependencies("${BUILD_SYSTEM_TARGET}" "${LINK_SCOPE}" "${INTERNAL_ALL_IMPLEMENTATIONS}") |
There was a problem hiding this comment.
🔴 Empty LINK_SCOPE passed as empty string "" to target_link_libraries when FPRIME_USE_PLAIN_LINK_SIGNATURE is ON
When FPRIME_USE_PLAIN_LINK_SIGNATURE is set to a truthy value, LINK_SCOPE remains empty (set(LINK_SCOPE)). It is then passed quoted as "${LINK_SCOPE}" to fprime_target_dependencies, which propagates it through fprime_target_link_libraries (cmake/utilities.cmake:775) → fprime__internal_target_interceptor (cmake/utilities.cmake:746) → and finally to cmake_language(CALL "target_link_libraries" "${BUILD_SYSTEM_TARGET}" "" ${ARGN}) at cmake/utilities.cmake:735. The empty string "" is not a valid scope keyword (PUBLIC/PRIVATE/INTERFACE) and CMake will interpret it as a library name in the "plain" signature, causing a build error or invalid link. The intent was presumably to call target_link_libraries(target dep1 dep2...) without a scope keyword, but the interceptor at line 735 always forwards "${SCOPE}" as a separate argument even when empty.
Prompt for agents
The problem is that when FPRIME_USE_PLAIN_LINK_SIGNATURE is ON, LINK_SCOPE is empty and passed as an empty string through the chain of fprime_target_dependencies -> fprime_target_link_libraries -> fprime__internal_target_interceptor. The interceptor at cmake/utilities.cmake:735 always passes "${SCOPE}" quoted, so an empty scope becomes an empty string argument to target_link_libraries rather than being omitted.
To fix this, there are two approaches:
1. Change the interceptor (cmake/utilities.cmake:735) to use unquoted ${SCOPE} instead of "${SCOPE}", so an empty scope gets omitted. This also requires similar changes in fprime_target_link_libraries (line 746) and fprime_target_dependencies (line 775).
2. Alternatively, bypass the interceptor chain entirely when LINK_SCOPE is empty and call target_link_libraries directly with just the target and dependencies (no scope keyword).
Note that fprime_target_include_directories was already updated to use unquoted ${SCOPE} at line 759, but the link libraries path was not similarly updated, creating an inconsistency.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is from the upstream commit Build Fixes (#5280), not from this PR's changes. See nasa#5284 for the actual upstream PR.
| function(fprime_target_include_directories BUILD_TARGET_NAME SCOPE) | ||
| fprime__internal_target_interceptor("target_include_directories" "${BUILD_TARGET_NAME}" "${SCOPE}" ${ARGN}) | ||
| fprime__internal_target_interceptor("target_include_directories" "${BUILD_TARGET_NAME}" ${SCOPE} ${ARGN}) | ||
| endfunction() |
There was a problem hiding this comment.
🚩 Asymmetric quoting between fprime_target_include_directories and fprime_target_link_libraries
At cmake/utilities.cmake:759, the SCOPE variable is now passed unquoted (${SCOPE}) to fprime__internal_target_interceptor, while the parallel function fprime_target_link_libraries at cmake/utilities.cmake:746 still uses quoted "${SCOPE}". This is functionally equivalent for the current single caller (cmake/API.cmake:471 which always passes PUBLIC), but creates an inconsistency in the API contract. If fprime_target_include_directories is ever called with an empty SCOPE, the unquoted expansion would cause the first ARGN element to be captured as SCOPE in the interceptor. This appears to be an intentional change to support the plain signature pattern (matching the FPRIME_USE_PLAIN_LINK_SIGNATURE intent in implementation.cmake), but only this one function was updated while the rest of the chain was not.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This asymmetry is from the upstream commit Build Fixes (#5280), not from this PR's changes.
| set(LINK_SCOPE) | ||
| if (NOT FPRIME_USE_PLAIN_LINK_SIGNATURE) | ||
| set(LINK_SCOPE PRIVATE) | ||
| endif() | ||
| fprime_target_dependencies("${BUILD_SYSTEM_TARGET}" "${LINK_SCOPE}" "${INTERNAL_ALL_IMPLEMENTATIONS}") |
There was a problem hiding this comment.
🚩 FPRIME_USE_PLAIN_LINK_SIGNATURE is never defined anywhere in the codebase
The variable FPRIME_USE_PLAIN_LINK_SIGNATURE is only referenced at cmake/implementation.cmake:37 and is never defined or set anywhere in the repository. In CMake, if (NOT FPRIME_USE_PLAIN_LINK_SIGNATURE) evaluates to TRUE for an undefined variable, so LINK_SCOPE will always be set to PRIVATE. This means the current runtime behavior is identical to the old code (fprime_target_dependencies(... PRIVATE ...)). The variable appears to be a hook for future/external configuration, but since the code path when it IS set is broken (reported as BUG-0001), this feature is not usable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
These three findings are all from the upstream commit Build Fixes (#5280) (12604c6) which was included in this fork PR's diff after rebasing onto upstream/devel. None of them are related to the changes in this PR (which only deletes default.cmake and removes unused functions from utilities.cmake). The real PR targeting upstream is nasa#5284.
Change Description
Removes 9 never-called functions from the CMake build system (keeping
autocoder_setup_for_individual_sourcesas it's part of the autocoder API):Deleted
cmake/target/default.cmake(entire file):add_global_target,add_deployment_target,add_module_target— unprefixed default implementations that are never dispatched. The target system calls${TARGET_NAME}_add_*_target(prefixed), anddefault.cmakeis never registered viaregister_fprime_target.Removed from
cmake/utilities.cmake:build_relative_path/full_path_from_build_relative_path— path helpers with no callerson_any_changed/on_changed— file-change detection (on_changedonly called fromon_any_changed, both dead)read_from_lines— newline parser with no callersfilter_lists— list filtering with no callersRationale
Dead code removal. These functions were identified by cross-referencing all
function()/macro()definitions incmake/(excludingAPI.cmakeand test data) against all call sites, accounting for dynamic dispatch viacmake_language(CALL ...).Testing/Review Recommendations
autocoder_setup_for_individual_sourceswhich is kept)Future Work
None.
AI Usage (see policy)
AI was used to perform static analysis of the CMake codebase to identify unused functions, and to implement the removal.
IAMAI
Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/9bcc74ce1ff640b3a4152b1d44c1e59b