Support both external and internal dependencies (#221)

* spdlog: Support an external spdlog

* nlohmann_json: Support an internal submodule copy

* external deps: rework option as USE_EXTERNAL_FOO and use a common macro

* Use shallow submodules

* Update README for new options to support both internal and external deps

* Add missing test header dependencies to spdlog

* Update clang-format action

* Run Python checks only on our files

Restrict glob to our package directory, so that submodule files are
not checked

* Ensure flake8 uses submodules

Co-authored-by: Chuck Atkins <chuck.atkins@kitware.com>

.github/workflows/clang-format.yml

@@ -19,7 +19,7 @@ jobs:
     # Applies clang-format to all C/C++ files w/ the specified style
     - name: Apply clang-format
       id: formatting
-      uses: HSF/clang-format-action@v0.2
+      uses: HSF/clang-format-action@v0.4
       with:
         style: 'Google'
 

.github/workflows/flake8.yml

@@ -20,6 +20,8 @@ jobs:
       steps:
       # Checks-out our repository under $GITHUB_WORKSPACE, so our job can access it
       - uses: actions/checkout@v2
+        with:
+          submodules: recursive
 
       # Sets up useful environment variables
       - name: Setup environment variables

.gitmodules

@@ -1,3 +1,8 @@
+[submodule "submodules/nlohmann_json"]
+	shallow = true
+	path = submodules/nlohmann_json
+	url = https://github.com/nlohmann/json.git
 [submodule "submodules/spdlog"]
+	shallow = true
 	path = submodules/spdlog
 	url = https://github.com/gabime/spdlog.git

CMakeLists.txt

@@ -100,7 +100,11 @@ install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/LICENSE
               ${CMAKE_CURRENT_SOURCE_DIR}/NOTICE
               DESTINATION ${CMAKE_INSTALL_DOCDIR})
 
+#--- project dependencies ------------------------------------------------------
+include(cmake/prmonDependencies.cmake)
+
 #--- project specific subdirectories -------------------------------------------
+
 add_subdirectory(package)
 
 #--- include directories -------------------------------------------

README.md

@@ -36,21 +36,44 @@ As prmon has dependencies on submodules, clone the project as
 ### Building the project
 
 Building prmon requires a C++ compiler that fully supports C++11,
-CMake version 3.3 or
-higher and the [Niels Lohmann JSON libraries](https://github.com/nlohmann/json).
+and CMake version 3.3 or higher.  It also has dependencies on:
 
-Building is usually as simple as
+  - [Niels Lohmann JSON libraries](https://github.com/nlohmann/json)
+    - `nlohmann-json-dev` in Ubuntu 18, `nlohmann-json3-dev` in Ubuntu 20
+  - [spdlog: Fast C++ logging library](https://github.com/gabime/spdlog)
+    - `libspdlog-dev` in Ubuntu
+
+and can use either external system-supplied versions or internal copies
+provided by submodules.
+
+Building is usually as simple as:
 
     mkdir build
     cd build
     cmake -DCMAKE_INSTALL_PREFIX=<installdir> <path to sources>
     make -j<number of cores on your machine>
     make install
 
-If your installation of JSON is in a non-standard location then
-setting `nlohmann_json_DIR` may be required as a hint to CMake.
-(e.g. this is necessary on Ubuntu18 when using the standard `nlohmann-json-dev`
-package: `-Dnlohmann_json_DIR=/usr/lib/cmake`.)
+Unless otherwise specified, the default behavior for dependencies is to first
+try to find an external version and fall back to the internal submodule copy
+if not found.  To explicitly force the use of either add any of the following
+configure options:
+  - `-DUSE_EXTERNAL_NLOHMANN_JSON={TRUE,FALSE,`**`AUTO`**`}`
+    - `ON`, `TRUE`: Force an external version and fail if not found.
+    - `OFF`, `FALSE`: Require the internal copy be used.
+    - **`AUTO`**: Search for an external version and fall back to the
+       internal copy if not found.
+  - `-Dnlohmann_json_DIR=/path/to/config`
+    - The path to the directory containing `nlohmann_jsonConfig.cmake`.
+      Necessary if nlohmann_json is not installed into CMake's search path.
+  - `-DUSE_EXTERNAL_SPDLOG={TRUE,FALSE,`**`AUTO`**`}`
+    - `ON`, `TRUE`: Force an external version and fail if not found.
+    - `OFF`, `FALSE`: Require the internal copy be used.
+    - **`AUTO`**: Search for an external version and fall back to the
+       internal copy if not found.
+  - `-Dspdlog_DIR=/path/to/config`
+    - The path to the directory containing `spdlogConfig.cmake`.
+      Necessary if spdlog is not installed into CMake's search path.
 
 The option `-DCMAKE_BUILD_TYPE` can switch between all of the standard
 build types. The default is `Release`; use `RelWithDebInfo` if you want

cmake/prmonDependencies.cmake

@@ -0,0 +1,47 @@
+# A helper function to add dependencies supporting both internal and external
+# versions.  This adds a dependency with the following arguments:
+#   _cmake_name - The name of the dependency's CMake pacakge used in
+#                 find_package(${_cmake_name})
+#   _opt_name   - The configure option to create as USE_EXTERNAL_${_opt_name}
+#   _sub_name   - The name of the submodule the intal copy resides in at
+#                 submodules/${_sub_name}
+#
+# Note: The reason for all the manual logging is only some package configs
+#       give useful output so we're just calling find_package(... QUIET)
+#       and doing the logging ourselves.
+macro(_prmon_add_internal_external_dependency _cmake_name _opt_name _sub_name)
+  set(USE_EXTERNAL_${_opt_name} AUTO CACHE STRING "Use an external ${_cmake_name}")
+  set_property(CACHE USE_EXTERNAL_${_opt_name} PROPERTY
+    STRINGS "ON;TRUE;AUTO;OFF;FALSE")
+  mark_as_advanced(USE_EXTERNAL_${_opt_name})
+  if(USE_EXTERNAL_${_opt_name} STREQUAL AUTO)
+    find_package(${_cmake_name} CONFIG QUIET)
+  elseif(USE_EXTERNAL_${_opt_name})
+    find_package(${_cmake_name} CONFIG REQUIRED QUIET)
+  endif()
+  if(${_cmake_name}_FOUND)
+    message(STATUS "Found ${_cmake_name}: ${${_cmake_name}_CONFIG} (found version \"${${_cmake_name}_VERSION}\")")
+  else() # USE_EXTERNAL_${_opt_name} = OFF or AUTO and failed to find above
+    if(USE_EXTERNAL_${_opt_name} STREQUAL AUTO)
+      message(STATUS "External ${_cmake_name} not found, using internal submodule")
+    else()
+      message(STATUS "Forcing internal submodule for ${_cmake_name}")
+    endif()
+    if (NOT EXISTS ${PROJECT_SOURCE_DIR}/submodules/${_sub_name}/CMakeLists.txt)
+      message(FATAL_ERROR "${_cmake_name} submodule is not available")
+    endif()
+    add_subdirectory(submodules/${_sub_name} EXCLUDE_FROM_ALL)
+  endif()
+endmacro()
+
+
+#--- nlohmann-json -------------------------------------------------------------
+_prmon_add_internal_external_dependency(nlohmann_json NLOHMANN_JSON nlohmann_json)
+
+# Setup the imported target alias if an older un-aliased version is found
+if(TARGET nlohmann_json AND NOT TARGET nlohmann_json::nlohmann_json)
+  add_library(nlohmann_json::nlohmann_json ALIAS nlohmann_json)
+endif()
+
+#--- spdlog --------------------------------------------------------------------
+_prmon_add_internal_external_dependency(spdlog SPDLOG spdlog)

cmake/python-format.cmake

@@ -3,8 +3,8 @@
 # Requires black/flake8 to be available in the environment
 
 
-# Get all Python files
-file(GLOB_RECURSE ALL_PYTHON_FILES *.py)
+# Get all our Python files (submodules are excluded!)
+file(GLOB_RECURSE ALL_PYTHON_FILES ${PROJECT_SOURCE_DIR}/package/*.py)
 
 # Black is rather simple because there are no options...
 add_custom_target(

package/CMakeLists.txt

@@ -1,5 +1,3 @@
-find_package(nlohmann_json REQUIRED)
-
 add_executable(prmon src/prmon.cpp
            src/prmonutils.cpp
            src/utils.cpp
@@ -13,17 +11,16 @@ add_executable(prmon src/prmon.cpp
            src/memmon.cpp
            src/nvidiamon.cpp
            )
+
+target_link_libraries(prmon PRIVATE
+           nlohmann_json::nlohmann_json
+           spdlog::spdlog_header_only
+           )
+
 if (BUILD_BENCHMARK_LOG)
   add_executable(benchmark-log benchmarks/benchmark-log.cpp)
 endif(BUILD_BENCHMARK_LOG)
 
-# Some older versions of the nlohmann_json package don't define
-# this target (< 3.0.0 ?). The build on these platforms seems to
-# go ok even without this "link library" definition.
-if(TARGET nlohmann_json::nlohmann_json)
-  target_link_libraries(prmon PRIVATE nlohmann_json::nlohmann_json)
-endif(TARGET nlohmann_json::nlohmann_json)
-
 # Set flags based on more unusual build flags
 if(BUILD_STATIC)
   set_target_properties(prmon PROPERTIES LINK_FLAGS "-static")

package/tests/CMakeLists.txt

@@ -38,6 +38,7 @@ if (${CMAKE_VERSION} VERSION_GREATER "3.14.0" AND BUILD_GTESTS)
                             PRIVATE
                             Threads::Threads
                             gtest_main
+                            spdlog::spdlog_header_only
     )
 
     gtest_discover_tests(test_parameter_classes)
@@ -71,6 +72,7 @@ if (${CMAKE_VERSION} VERSION_GREATER "3.14.0" AND BUILD_GTESTS)
                             PRIVATE
                             Threads::Threads
                             gtest_main
+                            spdlog::spdlog_header_only
     )
     gtest_discover_tests(test_values)
 
@@ -93,6 +95,7 @@ if (${CMAKE_VERSION} VERSION_GREATER "3.14.0" AND BUILD_GTESTS)
         ${CMAKE_CURRENT_SOURCE_DIR}/../include
         ${CMAKE_CURRENT_SOURCE_DIR}/../src
         ${CMAKE_CURRENT_SOURCE_DIR}/../..
+        spdlog::spdlog_header_only
     )
     if(TARGET nlohmann_json::nlohmann_json)
     target_link_libraries(test_fields
@@ -104,6 +107,7 @@ if (${CMAKE_VERSION} VERSION_GREATER "3.14.0" AND BUILD_GTESTS)
                             PRIVATE
                             Threads::Threads
                             gtest_main
+                            spdlog::spdlog_header_only
     )
     add_test(NAME testFieldsAll COMMAND test_fields)
     add_test(NAME testFieldsSomeDisabled COMMAND test_fields --disable netmon --disable cpumon)