Refactored the entire library architecture processing logic.

MrPointer · MrPointer · commit b0df5ab174d7 · 2018-09-25T19:59:39.000+03:00
It also came with a cost of requiring the library's root directory in the `add_library` API.
diff --git a/cmake/Platform/Arduino.cmake b/cmake/Platform/Arduino.cmake
@@ -21,7 +21,7 @@ include(TargetFlagsManager)
 include(SourcesManager)
 include(SketchManager)
 include(DefaultsManager)
-include(ArchitectureValidator)
+include(SupportedArchitecturesRetriever)
 
 include(Libraries)
 
diff --git a/cmake/Platform/Libraries/Libraries.cmake b/cmake/Platform/Libraries/Libraries.cmake
@@ -1,3 +1,4 @@
 include(LibraryArchitectureParser)
+include(LibrarySourcesArchitectureResolver)
 include(LibraryFlagsManager)
 include(LibrariesFinder)
diff --git a/cmake/Platform/Libraries/LibrariesFinder.cmake b/cmake/Platform/Libraries/LibrariesFinder.cmake
@@ -28,16 +28,6 @@ function(find_arduino_library _target_name _library_name _board_id)
         message(SEND_ERROR "Couldn't find library named ${_library_name}")
     else () # Library is found
         get_filename_component(library_path ${library_properties_file} DIRECTORY)
-        get_library_architecture("${library_properties_file}" lib_arch)
-        if (lib_arch)
-            if ("${lib_arch}" MATCHES "UNSUPPORTED")
-                string(CONCAT error_message
-                        "${_library_name} "
-                        "library isn't supported on the platform's architecture "
-                        "${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE}")
-                message(SEND_ERROR ${error_message})
-            endif ()
-        endif ()
 
         find_library_header_files("${library_path}" library_headers)
         if (NOT library_headers)
@@ -46,7 +36,7 @@ function(find_arduino_library _target_name _library_name _board_id)
         else ()
             if (parsed_args_HEADER_ONLY)
                 add_arduino_header_only_library(${_target_name} ${_board_id}
-                        ARCH ${lib_arch}
+                        ARCH ${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE}
                         HEADERS ${library_headers})
             else ()
                 find_library_source_files("${library_path}" library_sources)
@@ -58,8 +48,8 @@ function(find_arduino_library _target_name _library_name _board_id)
                     message(SEND_ERROR "${error_message}")
                 else ()
                     set(sources ${library_headers} ${library_sources})
-                    add_arduino_library(${_target_name} ${_board_id} "${sources}"
-                            ARCH ${lib_arch})
+                    add_arduino_library(${_target_name} ${_board_id} ${library_path} "${sources}"
+                            LIB_PROPS_FILE ${library_properties_file})
                 endif ()
             endif ()
         endif ()
diff --git a/cmake/Platform/Libraries/LibraryArchitectureParser.cmake b/cmake/Platform/Libraries/LibraryArchitectureParser.cmake
@@ -1,40 +1,38 @@
+function(get_arduino_library_supported_architectures _library_properties_file _return_var)
+
+    file(STRINGS ${_library_properties_file} library_properties)
+
+    list(FILTER library_properties INCLUDE REGEX "arch")
+    _get_property_value("${library_properties}" _library_arch_list)
+    string(REPLACE "," ";" _library_arch_list ${_library_arch_list}) # Turn into a valid list
+
+    set(${_return_var} ${_library_arch_list} PARENT_SCOPE)
+
+endfunction()
+
 #=============================================================================#
 # Gets the library architecure if any, read from the given library properties file
 # which includes this information.
-#       _library_properties_file - Full path to a library's properties file.
+#       _library_arch_list - List of architectures supported by the library,
+#                    inferred from its' 'library.properties' file.
 #       _return_var - Name of variable in parent-scope holding the return value.
-#       Returns - If library is architecure neutral, nothing is returned.
+#       Returns - If library is architecure agnostic (Supports all), nothing is returned.
 #                 If library doesn't support the current platform's architecture,
 #                 "UNSUPPORTED" string is returned.
 #                 Otherwise, the platform's architecture is returned.
 #=============================================================================#
-function(get_library_architecture _library_properties_file _return_var)
-
-    file(STRINGS ${_library_properties_file} library_properties)
-
-    list(FILTER library_properties INCLUDE REGEX "arch")
-    _get_property_value("${library_properties}" arch_list)
-    string(REPLACE "," ";" arch_list ${arch_list}) # Turn into a valid list
+function(is_library_supports_platform_architecture _library_arch_list _return_var)
 
-    if ("${arch_list}" MATCHES "\\*")
-        return() # Any architecture is supported, return nothing
+    if ("${_library_arch_list}" MATCHES "\\*") # Any architecture is supported
+        set(result TRUE)
     else ()
-        list(LENGTH arch_list num_of_supported_archs)
-        if (${num_of_supported_archs} GREATER 1) # Number of architectures is supported
-            list(FIND arch_list ${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE} platform_arch_index)
-            if (${platform_arch_index} LESS 0) # Our arch isn't supported
-                set(__arch "UNSUPPORTED")
-            else () # Our arch is indeed supported
-                set(__arch ${arch_list})
-            endif ()
+        if (${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE} IN_LIST _library_arch_list)
+            set(result TRUE) # Our platform's arch is supported
         else ()
-            list(GET arch_list 0 __arch)
-            if (NOT "${__arch}" STREQUAL "${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE}")
-                set(__arch "UNSUPPORTED") # Our arch isn't supported
-            endif ()
+            set(result FALSE) # Our arch isn't supported
         endif ()
     endif ()
 
-    set(${_return_var} ${__arch} PARENT_SCOPE)
+    set(${_return_var} ${result} PARENT_SCOPE)
 
 endfunction()
diff --git a/cmake/Platform/Libraries/LibrarySourcesArchitectureResolver.cmake b/cmake/Platform/Libraries/LibrarySourcesArchitectureResolver.cmake
@@ -0,0 +1,59 @@
+function(_filter_unsupported_arch_sources _unsupported_archs_regex _sources _return_var)
+
+    #string(LENGTH _unsupported_archs_regex num_of_unsupported_archs)
+
+    if (NOT "${_unsupported_archs_regex}" STREQUAL "") # Not all architectures are supported
+        # Filter sources dependant on unsupported architectures
+        list(FILTER _sources EXCLUDE REGEX ${_unsupported_archs_regex})
+    endif ()
+
+    set(${_return_var} ${_sources} PARENT_SCOPE)
+
+endfunction()
+
+function(resolve_library_sources_by_architecture _library_root_dir _library_sources _return_var)
+
+    cmake_parse_arguments(parsed_args "" "LIB_PROPS_FILE" "" ${ARGN})
+
+    if (parsed_args_LIB_PROPS_FILE) # Library properties file is given
+        set(lib_props_file ${parsed_args_LIB_PROPS_FILE})
+    else () # Try to automatically find file from sources
+
+        # Get the absolute root directory (full path)
+        get_filename_component(absolute_lib_root_dir ${_library_root_dir} ABSOLUTE)
+
+        if (EXISTS ${absolute_lib_root_dir}/library.properties)
+            set(lib_props_file ${absolute_lib_root_dir}/library.properties)
+
+        else () # Properties file can't be found - Warn user and assume library is arch-agnostic
+
+            get_filename_component(library_name ${absolute_lib_root_dir} NAME)
+            message(WARNING "\"${library_name}\" library's properties file can't be found "
+                    "under its' root directory - Assuming the library "
+                    "is architecture-agnostic (supports all architectures)")
+            set(${_return_var} ${_library_sources} PARENT_SCOPE)
+            return()
+
+        endif ()
+
+    endif ()
+
+    get_arduino_library_supported_architectures("${lib_props_file}" lib_archs)
+    is_library_supports_platform_architecture(${lib_archs} arch_supported_by_lib)
+
+    if (NOT ${arch_supported_by_lib})
+        string(CONCAT error_message
+                "The ${_library_name} "
+                "library isn't supported on the platform's architecture "
+                "${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE}")
+        message(SEND_ERROR ${error_message})
+    endif ()
+
+    get_unsupported_architectures("${lib_archs}" unsupported_archs REGEX)
+
+    # Filter any sources that aren't supported by the platform's architecture
+    _filter_unsupported_arch_sources("${unsupported_archs}" "${_library_sources}" valid_sources)
+
+    set(${_return_var} "${valid_sources}" PARENT_SCOPE)
+
+endfunction()
diff --git a/cmake/Platform/Other/ArchitectureValidator.cmake b/cmake/Platform/Other/ArchitectureValidator.cmake
diff --git a/cmake/Platform/Other/SupportedArchitecturesRetriever.cmake b/cmake/Platform/Other/SupportedArchitecturesRetriever.cmake
@@ -0,0 +1,40 @@
+#=============================================================================#
+# Gets a filtered list of architectures that aren't compliant with the platform's architecture.
+# e.g If a list contains 'avr' and 'nrf52', while our arch is 'avr', 'nrf52' will be returned.
+#       _arch_list - List of all architectures probably read from a library's properties file
+#       [REGEX] - Returns list in a regex-compatible mode, allowing caller to
+#                 use result in search patterns. This is currently the only supported mode.
+#       _return_var - Name of variable in parent-scope holding the return value.
+#       Returns - Filtered list of architectures.
+#=============================================================================#
+function(get_unsupported_architectures _arch_list _return_var)
+
+    cmake_parse_arguments(parsed_args "REGEX" "" "" ${ARGN})
+
+    if ("${_arch_list}" MATCHES "\\*") # All architectures are supported, return nothing
+        return()
+    endif ()
+
+    list(FILTER _arch_list EXCLUDE REGEX ${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE})
+    set(unsupported_arch_list ${_arch_list}) # Just for better readability
+
+    if (parsed_args_REGEX) # Return in regex format         
+
+        foreach (arch ${unsupported_arch_list})
+            # Append every unsupported-architecture and "|" to represent "or" in regex-fomart
+            string(APPEND unsupported_archs_regex "${arch}" "|")
+        endforeach ()
+
+        # Remove last "|" as it's unnecessary - There's no element after it
+        string(LENGTH ${unsupported_archs_regex} str_len)
+        decrement_integer(str_len 1) # Decrement string's length by 1 to trim last char ('|')
+        string(SUBSTRING ${unsupported_archs_regex} 0 ${str_len} unsupported_archs_regex)
+
+        # prepare for generalized function return
+        set(unsupported_arch_list ${unsupported_archs_regex})
+
+    endif ()
+
+    set(${_return_var} ${unsupported_arch_list} PARENT_SCOPE)
+
+endfunction()
diff --git a/cmake/Platform/Targets/ArduinoCMakeLibraryTarget.cmake b/cmake/Platform/Targets/ArduinoCMakeLibraryTarget.cmake
@@ -10,21 +10,7 @@
 #=============================================================================#
 function(_add_arduino_cmake_library _target_name _board_id _sources)
 
-    cmake_parse_arguments(parsed_args "INTERFACE" "ARCH" "" ${ARGN})
-
-    if (parsed_args_ARCH) # Treat architecture-specific libraries differently
-        # Filter any sources that aren't supported by the platform's architecture
-        list(LENGTH library_ARCH num_of_libs_archs)
-        if (${num_of_libs_archs} GREATER 1)
-            # Exclude all unsupported architectures, request filter in regex mode
-            get_unsupported_architectures("${parsed_args_ARCH}" arch_filter REGEX)
-            set(filter_type EXCLUDE)
-        else ()
-            set(arch_filter "src\\/[^/]+\\.|${parsed_args_ARCH}")
-            set(filter_type INCLUDE)
-        endif ()
-        list(FILTER _sources ${filter_type} REGEX ${arch_filter})
-    endif ()
+    cmake_parse_arguments(parsed_args "INTERFACE" "" "" ${ARGN})
 
     if (parsed_args_INTERFACE)
         add_library(${_target_name} INTERFACE)
@@ -40,11 +26,9 @@ function(_add_arduino_cmake_library _target_name _board_id _sources)
 
     set_library_flags(${_target_name} ${_board_id} ${scope})
 
-    if (parsed_args_ARCH)
-        string(TOUPPER ${parsed_args_ARCH} upper_arch)
-        set(arch_definition "ARDUINO_ARCH_${upper_arch}")
-        target_compile_definitions(${_target_name} ${scope} ${arch_definition})
-    endif ()
+    string(TOUPPER ${ARDUINO_CMAKE_PLATFORM_ARCHITECTURE} upper_arch)
+    set(arch_definition "ARDUINO_ARCH_${upper_arch}")
+    target_compile_definitions(${_target_name} ${scope} ${arch_definition})
 
 endfunction()
 
diff --git a/cmake/Platform/Targets/ArduinoLibraryTarget.cmake b/cmake/Platform/Targets/ArduinoLibraryTarget.cmake
@@ -4,11 +4,17 @@
 #       _target_name - Name of the library target to be created. Usually library's real name.
 #       _board_id - Board ID associated with the linked Core Lib.
 #       _sources - Source and header files to create target from.
+#       [LIB_PROPS_FILE] - Full path to the library's properties file. Optional.
 #=============================================================================#
-function(add_arduino_library _target_name _board_id _sources)
+function(add_arduino_library _target_name _board_id _library_root_dir _sources)
+
+    resolve_library_sources_by_architecture(${_library_root_dir} "${_sources}" arch_resolved_sources
+            "${ARGN}")
+
+    _add_arduino_cmake_library(${_target_name} ${_board_id} "${arch_resolved_sources}" "${ARGN}")
+
+    find_dependent_platform_libraries("${arch_resolved_sources}" lib_platform_libs)
 
-    _add_arduino_cmake_library(${_target_name} ${_board_id} "${_sources}" "${ARGN}")
-    find_dependent_platform_libraries("${_sources}" lib_platform_libs)
     foreach (platform_lib ${lib_platform_libs})
         link_platform_library(${_target_name} ${platform_lib} ${_board_id})
     endforeach ()
diff --git a/examples/3rd-party-library/3rd_party.cpp b/examples/3rd-party-library/3rd_party.cpp
@@ -0,0 +1,14 @@
+#include <Arduino.h>
+#include "NeoPixelTest.hpp"
+#include "GFXTest.h"
+
+void setup()
+{
+    testNeoPixel();
+    testGFX();
+}
+
+void loop()
+{
+
+}
diff --git a/examples/3rd-party-library/CMakeLists.txt b/examples/3rd-party-library/CMakeLists.txt
@@ -3,17 +3,24 @@ cmake_minimum_required(VERSION 3.8.2)
 project(3rd_Party_Arduino_Library)
 get_board_id(board_id nano atmega328)
 
-add_arduino_executable(3rd_Party_Arduino_Library ${board_id} NeoPixelTest.cpp GFXTest.cpp)
+# First, declare and create our executable - It'll use 4 sources
+add_arduino_executable(3rd_Party_Arduino_Library ${board_id} 3rd_party.cpp
+        NeoPixelTest.cpp GFXTest.cpp)
 target_include_directories(3rd_Party_Arduino_Library PRIVATE include)
 
-add_arduino_library(Adafruit_NeoPixel ${board_id} libraries/Adafruit_NeoPixel/Adafruit_NeoPixel.cpp)
+# Add the "NeoPixel" library manually using the library addition API
+add_arduino_library(Adafruit_NeoPixel ${board_id} libraries/Adafruit_NeoPixel
+        libraries/Adafruit_NeoPixel/Adafruit_NeoPixel.cpp)
 target_include_directories(Adafruit_NeoPixel PUBLIC libraries/Adafruit_NeoPixel)
 
-# 'Trick' the framework to use current directory as Sketchbook, allowing us to use the 'find' API
+# Find the "GFX" library by 'tricking' the framework to use current directory as the Sketchbook path,
+# allowing us to use the 'find' API
 set(ARDUINO_CMAKE_SKETCHBOOK_PATH "${CMAKE_CURRENT_LIST_DIR}")
 find_arduino_library(Adafruit_GFX Adafruit-GFX-Library ${board_id} 3RD_PARTY)
-target_source_directories(Adafruit_GFX
-        DIRS libraries/Adafruit-GFX-Library/Fonts)
+# We can also explicitly add additional directories to the target,
+# as only root dir and 'src' sub-dir are added by default
+target_source_directories(Adafruit_GFX DIRS libraries/Adafruit-GFX-Library/Fonts)
 
+# Link all libraries to our previously created target
 link_arduino_library(3rd_Party_Arduino_Library Adafruit_NeoPixel ${board_id})
 link_arduino_library(3rd_Party_Arduino_Library Adafruit_GFX ${board_id})
diff --git a/examples/3rd-party-library/GFXTest.cpp b/examples/3rd-party-library/GFXTest.cpp
@@ -2,7 +2,7 @@
 
 static Adafruit_GFX_Button gfxButton;
 
-void doSomething()
+void testGFX()
 {
     gfxButton.isPressed();
 }
diff --git a/examples/3rd-party-library/NeoPixelTest.cpp b/examples/3rd-party-library/NeoPixelTest.cpp
@@ -1,15 +1,8 @@
-#include <Adafruit_NeoPixel.h>
-#include "GFXTest.h"
+#include "include/NeoPixelTest.hpp"
 
 Adafruit_NeoPixel neoPixel;
 
-void setup()
+void testNeoPixel()
 {
     neoPixel.clear();
-    doSomething();
-}
-
-void loop()
-{
-
 }
diff --git a/examples/3rd-party-library/include/GFXTest.h b/examples/3rd-party-library/include/GFXTest.h
@@ -3,6 +3,6 @@
 
 #include <Adafruit_GFX.h>
 
-void doSomething();
+void testGFX();
 
 #endif //EXAMPLES_GFXTEST_HPP
diff --git a/examples/3rd-party-library/include/NeoPixelTest.hpp b/examples/3rd-party-library/include/NeoPixelTest.hpp
@@ -0,0 +1,8 @@
+#ifndef EXAMPLES_NEOPIXELTEST_HPP
+#define EXAMPLES_NEOPIXELTEST_HPP
+
+#include <Adafruit_NeoPixel.h>
+
+void testNeoPixel();
+
+#endif //EXAMPLES_NEOPIXELTEST_HPP
