Review fixes

Author: LeStarch

cmake/FPrime.cmake

@@ -180,7 +180,6 @@ endmacro(fprime_initialize_build_system)
 # registered.
 ####
 function(fprime_setup_included_code)
-    #choose_fprime_implementation(Fw_StringFormat snprintf-format FRAMEWORK_DEFAULT) # Default choice is snprintf
     # Must be done before code is registered but after custom target registration
     setup_global_targets()
     # For BUILD_TESTING builds then set up libraries that support testing

cmake/implementation.cmake

@@ -1,8 +1,52 @@
-
+####
+# implementation.cmake:
+#
+# This file handles enables deployments (and other executables) to choose implementations that resolve
+# at link time. This enables projects to choose different underlying implementations for specific concepts
+# (e.g. OS, Memory, etc.) at link time.
+####
 include_guard()
 include(config_assembler)
 
+####
+# Function `fprime_target_implementations`:
+#
+# This function will calculate and add the implementations required for a target. It will use the default
+# implementations if no OVERRIDES are provided. Default implementations are the final set of chosen
+# implementations as specified in `register_fprime_config` using the CHOOSES_IMPLEMENTATIONS call.
+#
+# Choosing implementations ensures that there is some implementation of all globally required
+# implementations specified by `register_fprime_*` via the REQUIRED_IMPLEMENTATIONS argument.
+#
+# > [!WARNING]
+# > This function may only be called on targets that already exist
+#
+# > [!NOTE]
+# > Target will be updated with FPRIME_CHOSEN_IMPLEMENTATIONS property and FPRIME_CHOSEN_<IMPLEMENTATION>
+# > properties for each implementation.  `target_link_libraries` will be updated.
+#
+# Args:
+#  **BUILD_SYSTEM_TARGET**: the target in the build system to add implementations dependencies to
+#  **ARGN**: a list of implementations to override the default implementations with
+####
+function(fprime_target_implementations BUILD_SYSTEM_TARGET)
+    append_list_property("${ARGN}" TARGET "${BUILD_SYSTEM_TARGET}" PROPERTY FPRIME_CHOSEN_IMPLEMENTATIONS)
+    fprime__internal_choose_implementations("${BUILD_SYSTEM_TARGET}" INTERNAL_ALL_IMPLEMENTATIONS)
+    fprime_target_dependencies("${BUILD_SYSTEM_TARGET}" PRIVATE "${INTERNAL_ALL_IMPLEMENTATIONS}")
+endfunction()
 
+####
+# Function `fprime__internal_implementation_detect_implementations`:
+#
+# This function will scan the list of global default implementations (detected via the config modules and set in
+# the sub-build) and the implementations chosen by the current module. It will then choose one implementation for
+# each "IMPLEMENTS" type.
+#
+# If IMPLEMENTS cannot be determined it is added to the list of unknown implementations.
+#
+# Args:
+#  CURRENT_MODULE: the module to check for implementations
+####
 function(fprime__internal_implementation_detect_implementations CURRENT_MODULE)
     get_target_property(MODULE_CHOSEN "${CURRENT_MODULE}" FPRIME_CHOSEN_IMPLEMENTATIONS)
     get_property(CONFIG_CHOSEN GLOBAL PROPERTY FPRIME_BASE_CHOSEN_IMPLEMENTATIONS)
@@ -28,7 +72,26 @@ function(fprime__internal_implementation_detect_implementations CURRENT_MODULE)
     endforeach()
 endfunction()
 
-
+####
+# Function `fprime__internal_choose_implementations`:
+#
+# This function will scan the list of global default implementations module choices via
+# `fprime__internal_implementation_detect_implementations`. It will then ensure that each REQUIRED implementation
+# has a chosen implementation.
+#
+# It is an error to not have a chosen implementation for required unless there is a set of unknown implementations
+# and in that case it is a warning.
+#
+# > [!WARNING]
+# > If UNKNOWN implementations are set, it is assumed that one of them fills any missing REQUIRED implementations
+# > but users should update those modules to explicitly set IMPLEMENTS flag to avoid this warning.
+# >
+# > If the UNKNOWN implementations do not provided the required implementations a linker error will occur.
+#
+# Args:
+#  CURRENT_MODULE: the module to check for implementations
+#  OUTPUT_VARIABLE: the variable to set with the list of chosen implementations
+####
 function(fprime__internal_choose_implementations CURRENT_MODULE OUTPUT_VARIABLE)
     set(IMPLEMENTATION_DEPENDENCIES)
     fprime__internal_implementation_detect_implementations("${CURRENT_MODULE}")
@@ -56,120 +119,3 @@ function(fprime__internal_choose_implementations CURRENT_MODULE OUTPUT_VARIABLE)
     append_list_property("${IMPLEMENTATION_DEPENDENCIES}" TARGET "${CURRENT_MODULE}" PROPERTY FPRIME_CHOSEN_IMPLEMENTATIONS)
     set("${OUTPUT_VARIABLE}" "${IMPLEMENTATION_DEPENDENCIES}" PARENT_SCOPE)
 endfunction()
-
-
-# ####
-# # implementation.cmake:
-# #
-# # Functions used to handle different registered implementations of various cmake packages within F´. The process breaks
-# # down as follows:
-# #   1. A module will "require" an implementation X
-# #   2. A number of modules will "register" as an implementation of X
-# #   3. A platform package will "choose" a single implementation for X
-# #   4. Linked modules (executable, deployment, unit-test) may override and "choose" an alternate implementation for X
-# #
-# # For example: Os requires an implementation of Os_File. Os_File_Posix implements Os_File. The Linux platform chooses
-# # Os_File_Posix as the implementation of Os_File. This applies to all linked modules in the system. Os_Baremetal_ut may
-# # then override this choose using Os_File_MicroFs instead.
-# #
-# # > All required implementations apply system-wide. Platforms specifically disallowing use of a given package should
-# # > "choose" the special implementation "None" to bypass the check.
-# #
-# # Internally this uses several properties. Each of these properties are GLOBAL such that these may be set before the
-# # various targets have been explicitly created. These pro
-# #
-# #
-# ####
-# include_guard()
-
-# ####
-# # Function `remap_implementation_choices`:
-# #
-# # When a target is declared with a different name than the module name, we need to pull the implementation choices into
-# # a property for the declared target instead of the inferred target. This is done by reading each implementation choice
-# # and setting the new property.
-# #
-# # Args:
-# #  INFERRED: inferred target name (FPRIME_CURRENT_MODULE)
-# #  DECLARED: real target name
-# ####
-# function(remap_implementation_choices INFERRED DECLARED)
-#     # Check and setup implementors
-#     get_property(GLOBAL_REQUIRED GLOBAL PROPERTY "REQUIRED_IMPLEMENTATIONS")
-#     if (GLOBAL_REQUIRED)
-#         foreach (IMPLEMENTATION IN LISTS GLOBAL_REQUIRED)
-#             get_property(IMPLEMENTOR GLOBAL PROPERTY "${INFERRED}_${IMPLEMENTATION}")
-#             if (IMPLEMENTOR)
-#                 set_property(GLOBAL PROPERTY "${DECLARED}_${IMPLEMENTATION}" "${IMPLEMENTOR}")
-#             endif()
-#         endforeach()
-#     endif ()
-# endfunction()
-
-# ####
-# # Function `setup_executable_implementations`:
-# #
-# # Sets up all the needed implementations for the supplied module. This looks up the global set of required
-# # implementations and delegates each to `setup_executable_implementation` for final setup.
-# #
-# # Args:
-# #    MODULE: module to setup implementation choices for
-# #####
-# function(setup_executable_implementations MODULE)
-#     # Check and setup implementors
-#     get_property(LOCAL_REQUIRED GLOBAL PROPERTY "REQUIRED_IMPLEMENTATIONS")
-#     if (LOCAL_REQUIRED)
-#         init_variables(IMPLEMENTATION_SOURCES IMPLEMENTATION_DEPENDENCIES)
-#         foreach (IMPLEMENTATION IN LISTS LOCAL_REQUIRED)
-#             setup_executable_implementation("${IMPLEMENTATION}" "${MODULE}")
-#             list(APPEND IMPLEMENTATION_SOURCES ${NEW_IMPLEMENTATION_SOURCES})
-#             list(APPEND IMPLEMENTATION_DEPENDENCIES ${NEW_IMPLEMENTATION_DEPENDENCIES})
-#         endforeach ()
-#     endif ()
-#     set(IMPLEMENTATION_SOURCES "${IMPLEMENTATION_SOURCES}" PARENT_SCOPE)
-#     set(IMPLEMENTATION_DEPENDENCIES "${IMPLEMENTATION_DEPENDENCIES}" PARENT_SCOPE)
-# endfunction()
-
-# ####
-# # Function `setup_executable_implementation`:
-# #
-# # Sets up the given implementation for the given module. Ensures that choices have been made and linking is setup
-# # without causing global link dependencies.
-# #
-# # Args:
-# #    MODULE: module to setup implementation choices for
-# #####
-# function(setup_executable_implementation IMPLEMENTATION MODULE)
-#     # Get the chosen implementor for the module
-#     get_property(IMPLEMENTOR GLOBAL PROPERTY "${MODULE}_${IMPLEMENTATION}")
-#     # Fallback to the platform in the case that the module does not specify
-#     if (NOT IMPLEMENTOR)
-#         get_property(IMPLEMENTOR GLOBAL PROPERTY "${FPRIME_PLATFORM}_${IMPLEMENTATION}")
-#     endif()
-#     # Fallback to the FRAMEWORK_DEFAULT in the case that the platform does not specify
-#     if (NOT IMPLEMENTOR)
-#         get_property(IMPLEMENTOR GLOBAL PROPERTY "FRAMEWORK_DEFAULT_${IMPLEMENTATION}")
-#     endif()
-   
-#     # Handle a failure to choose anything
-#     if (NOT IMPLEMENTOR)
-#         get_property(LOCAL_IMPLEMENTATIONS GLOBAL PROPERTY "${IMPLEMENTATION}_IMPLEMENTORS")
-#         if (NOT LOCAL_IMPLEMENTATIONS)
-#             set(LOCAL_IMPLEMENTATIONS "")
-#         endif ()
-#         string(REPLACE ";" ", " POSSIBLE "${LOCAL_IMPLEMENTATIONS}")
-#         message(FATAL_ERROR "An implementation of ${IMPLEMENTATION} is required.  Choose from: ${POSSIBLE}")
-#     elseif (IMPLEMENTOR STREQUAL "${IMPLEMENTATION}_None")
-#         if (CMAKE_DEBUG_OUTPUT)
-#             message(STATUS "[${MODULE}] Declining Implementation of ${IMPLEMENTATION}")
-#         endif ()
-#         return()
-#     endif ()
-#     if (CMAKE_DEBUG_OUTPUT)
-#         message(STATUS "[${MODULE}] Using Implementation: ${IMPLEMENTOR} for ${IMPLEMENTATION}")
-#     endif()
-#     target_sources("${MODULE}" PRIVATE "$<TARGET_PROPERTY:${IMPLEMENTOR},REQUIRED_SOURCE_FILES>")
-#     target_link_libraries("${MODULE}" PRIVATE ${IMPLEMENTOR})
-#     append_list_property("${IMPLEMENTOR}" TARGET "${MODULE}" PROPERTY FPRIME_DEPENDENCIES)
-#     add_dependencies("${MODULE}" ${IMPLEMENTOR})
-# endfunction()

cmake/module.cmake

@@ -219,14 +219,12 @@ endfunction()
 function(fprime__internal_add_build_target_helper TARGET_NAME TYPE SOURCES AUTOCODER_INPUTS HEADERS DEPENDENCIES REQUIRES_IMPLEMENTATIONS CHOOSES_IMPLEMENTATIONS FPRIME_CMAKE_ADD_OPTIONS)
     # Historical status message for posterity...and to prevent panic amongst users
     message(STATUS "Adding ${TYPE}: ${TARGET_NAME}")
-    set(IMPLEMENTATION_DEPENDENCIES)
     # Remap F Prime target type to CMake targe type
     if (INTERFACE IN_LIST FPRIME_CMAKE_ADD_OPTIONS AND SOURCES)
         fprime_cmake_fatal_error("INTERFACE libraries cannot have SOURCES")
     elseif (TYPE STREQUAL "Executable" OR TYPE STREQUAL "Deployment" OR TYPE STREQUAL "Unit Test")
         add_executable("${TARGET_NAME}" ${FPRIME_CMAKE_ADD_OPTIONS} "${SOURCES}")
-        set_target_properties("${TARGET_NAME}" PROPERTIES FPRIME_CHOSEN_IMPLEMENTATIONS "${CHOOSES_IMPLEMENTATIONS}")
-        fprime__internal_choose_implementations("${TARGET_NAME}" IMPLEMENTATION_DEPENDENCIES)
+        fprime_target_implementations("${TARGET_NAME}" ${CHOOSES_IMPLEMENTATIONS})
     elseif(TYPE STREQUAL "Library")
         add_library("${TARGET_NAME}" ${FPRIME_CMAKE_ADD_OPTIONS} ${SOURCES})
     else()
@@ -243,7 +241,7 @@ function(fprime__internal_add_build_target_helper TARGET_NAME TYPE SOURCES AUTOC
     target_sources("${TARGET_NAME}" PRIVATE "${FPRIME__INTERNAL_EMPTY_CPP}")
 
     # Add the link libraries safely in both real and INTERFACE libraries
-    fprime_target_dependencies("${TARGET_NAME}" PUBLIC ${DEPENDENCIES} ${REQUIRED_IMPLEMENTATIONS} ${IMPLEMENTATION_DEPENDENCIES})
+    fprime_target_dependencies("${TARGET_NAME}" PUBLIC ${DEPENDENCIES} ${REQUIRED_IMPLEMENTATIONS})
 
     # Set F Prime target properties
     set_target_properties("${TARGET_NAME}"

cmake/platform/Darwin.cmake

@@ -19,7 +19,6 @@ register_fprime_config(
       PlatformDarwin
    INTERFACE # No buildable files generated
    CHOOSES_IMPLEMENTATIONS
-      #### TODO: Fix these defaults ####
       Os_Cpu_Darwin
       Os_Memory_Darwin
 )

cmake/platform/Linux.cmake

@@ -17,7 +17,6 @@ register_fprime_config(
       PlatformLinux
    INTERFACE # No buildable files generated
    CHOOSES_IMPLEMENTATIONS
-      #### TODO: Fix these defaults ####
       Os_Cpu_Linux
       Os_Memory_Linux
 )

cmake/platform/unix/Platform/CMakeLists.txt

@@ -9,7 +9,6 @@ register_fprime_config(
     HEADERS
         "${CMAKE_CURRENT_LIST_DIR}/PlatformTypes.h"
     CHOOSES_IMPLEMENTATIONS
-        #### TODO: Fix these defaults ####
         Os_File_Posix
         Os_Console_Posix
         Os_Task_Posix