From fb20ef809df3e9c3c3a9ba230b336eab8716ef6a Mon Sep 17 00:00:00 2001 From: Philip Top Date: Tue, 1 Oct 2024 16:28:53 -0700 Subject: [PATCH] Cmake update (#342) * move addGoogleTest add some more cmake code update supported versions of CMAKE and some ThirdParty Dependencies * update googletest * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update cmake * only support google test for cmake 3.13 and higher * update pipeline and fuzz build --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .github/workflows/tests.yml | 12 ++ .gitmodules | 6 + CMakeLists.txt | 37 ++++-- ThirdParty/CLI11.hpp | 127 +++++++++++++++------ ThirdParty/cmake/updateGitSubmodules.cmake | 118 +++++++++++++++++++ ThirdParty/googletest | 1 + ThirdParty/json.hpp | 43 ++++++- azure-pipelines.yml | 1 + config/AddGoogletest.cmake | 114 +++--------------- config/spelling_ignorelines.txt | 1 + test/CMakeLists.txt | 2 +- webserver/CMakeLists.txt | 2 +- 12 files changed, 322 insertions(+), 142 deletions(-) create mode 100644 ThirdParty/cmake/updateGitSubmodules.cmake create mode 160000 ThirdParty/googletest diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4830bcbb..e22c074f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -172,4 +172,16 @@ jobs: uses: ./.github/actions/quick_cmake with: cmake-version: "3.28" + if: success() || failure() + + - name: Check CMake 3.29 + uses: ./.github/actions/quick_cmake + with: + cmake-version: "3.29" + if: success() || failure() + + - name: Check CMake 3.30 + uses: ./.github/actions/quick_cmake + with: + cmake-version: "3.30" if: success() || failure() \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index e69de29b..7f7182b0 100644 --- a/.gitmodules +++ b/.gitmodules @@ -0,0 +1,6 @@ +[submodule "googletest"] + path = ThirdParty/googletest + url = https://github.com/google/googletest.git +[submodule "ThirdParty/googletest"] + path = ThirdParty/googletest + url = https://github.com/google/googletest.git diff --git a/CMakeLists.txt b/CMakeLists.txt index b02de677..fc2c7af9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -# Copyright (c) 2019-2023, +# Copyright (c) 2019-2024, # Lawrence Livermore National Security, LLC; # See the top-level NOTICE for additional details. All rights reserved. # SPDX-License-Identifier: BSD-3-Clause @@ -9,7 +9,7 @@ # most of the policies, the new version is better (hence the change). We don't use the # 3.0...3.17 syntax because of a bug in an older MSVC's built-in and modified CMake 3.11 if(${CMAKE_VERSION} VERSION_GREATER 3.20) - cmake_minimum_required(VERSION 3.20...3.28) + cmake_minimum_required(VERSION 3.20...3.30) else() cmake_minimum_required(VERSION 3.0) cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION}) @@ -23,7 +23,7 @@ endif() project( ${UNITS_CMAKE_PROJECT_NAME} LANGUAGES C CXX - VERSION 0.9.1 + VERSION 0.9.2 ) include(CMakeDependentOption) include(CTest) @@ -87,10 +87,15 @@ set(UNITS_NAMESPACE CACHE STRING "Top-level namespace name. Default is `units`." ) -cmake_dependent_option( - UNITS_INSTALL "Generate and install cmake package files" ON - "CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME;NOT UNITS_BINARY_ONLY_INSTALL" OFF -) +if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME AND NOT UNITS_BINARY_ONLY_INSTALL) + option(UNITS_INSTALL + "Generate and install cmake package files and shared library if built" ON + ) +else() + option(UNITS_INSTALL + "Generate and install cmake package files and shared library if built" OFF + ) +endif() mark_as_advanced(UNITS_INSTALL) @@ -200,15 +205,23 @@ add_subdirectory(units) if(UNITS_BUILD_FUZZ_TARGETS) add_subdirectory(FuzzTargets) -elseif(UNITS_ENABLE_TESTS) +elseif(UNITS_ENABLE_TESTS AND NOT CMAKE_VERSION VERSION_LESS 3.13) + include(updateGitSubmodules) enable_testing() + if(NOT EXISTS "${PROJECT_SOURCE_DIR}/ThirdParty/googletest/CMakeLists.txt") + submod_update(ThirdParty/googletest) + endif() if(BUILD_TESTING) add_subdirectory(test) endif() - if(NOT UNITS_HEADER_ONLY) - add_subdirectory(webserver) - add_subdirectory(converter) - endif() + +elseif(UNITS_ENABLE_TESTS) + message(WARNING "UNITS unit tests only supported under cmake 3.13 or greater") +endif() + +if(NOT UNITS_HEADER_ONLY AND NOT UNITS_BUILD_FUZZ_TARGETS) + add_subdirectory(webserver) + add_subdirectory(converter) endif() if(UNITS_INSTALL) diff --git a/ThirdParty/CLI11.hpp b/ThirdParty/CLI11.hpp index 62c12a37..2e8c021a 100644 --- a/ThirdParty/CLI11.hpp +++ b/ThirdParty/CLI11.hpp @@ -3,7 +3,7 @@ // https://github.com/CLIUtils/CLI11 // // This is a standalone header file generated by MakeSingleHeader.py in CLI11/scripts -// from: v2.4.1-22-gacf4263 +// from: v2.4.2-29-g2be38c5 // // CLI11 2.4.2 Copyright (c) 2017-2024 University of Cincinnati, developed by Henry // Schreiner under NSF AWARD 1414736. All rights reserved. @@ -476,7 +476,12 @@ template std::string join(const T &v, std::string delim = ",") { while(beg != end) { s << delim << *beg++; } - return s.str(); + auto rval = s.str(); + if(!rval.empty() && delim.size() == 1 && rval.back() == delim[0]) { + // remove trailing delimiter if the last entry was empty + rval.pop_back(); + } + return rval; } /// Simple function to join a string from processed elements @@ -1118,7 +1123,8 @@ CLI11_INLINE std::string binary_escape_string(const std::string &string_to_escap if(escaped_string != string_to_escape) { auto sqLoc = escaped_string.find('\''); while(sqLoc != std::string::npos) { - escaped_string.replace(sqLoc, sqLoc + 1, "\\x27"); + escaped_string[sqLoc] = '\\'; + escaped_string.insert(sqLoc + 1, "x27"); sqLoc = escaped_string.find('\''); } escaped_string.insert(0, "'B\"("); @@ -1344,8 +1350,8 @@ class BadNameString : public ConstructionError { static BadNameString BadPositionalName(std::string name) { return BadNameString("Invalid positional Name: " + name); } - static BadNameString DashesOnly(std::string name) { - return BadNameString("Must have a name, not just dashes: " + name); + static BadNameString ReservedName(std::string name) { + return BadNameString("Names '-','--','++' are reserved and not allowed as option names " + name); } static BadNameString MultiPositionalNames(std::string name) { return BadNameString("Only one positional name allowed, remove: " + name); @@ -3433,8 +3439,8 @@ get_names(const std::vector &input) { long_names.push_back(name); else throw BadNameString::BadLongName(name); - } else if(name == "-" || name == "--") { - throw BadNameString::DashesOnly(name); + } else if(name == "-" || name == "--" || name == "++") { + throw BadNameString::ReservedName(name); } else { if(!pos_name.empty()) throw BadNameString::MultiPositionalNames(name); @@ -6021,7 +6027,9 @@ CLI11_NODISCARD CLI11_INLINE std::string Option::get_name(bool positional, bool } CLI11_INLINE void Option::run_callback() { + bool used_default_str = false; if(force_callback_ && results_.empty()) { + used_default_str = true; add_result(default_str_); } if(current_option_state_ == option_state::parsing) { @@ -6031,16 +6039,18 @@ CLI11_INLINE void Option::run_callback() { if(current_option_state_ < option_state::reduced) { _reduce_results(proc_results_, results_); - current_option_state_ = option_state::reduced; } - if(current_option_state_ >= option_state::reduced) { - current_option_state_ = option_state::callback_run; - if(!(callback_)) { - return; - } + + current_option_state_ = option_state::callback_run; + if(callback_) { const results_t &send_results = proc_results_.empty() ? results_ : proc_results_; bool local_result = callback_(send_results); - + if(used_default_str) { + // we only clear the results if the callback was actually used + // otherwise the callback is the storage of the default + results_.clear(); + proc_results_.clear(); + } if(!local_result) throw ConversionError(get_name(), results_); } @@ -6048,26 +6058,27 @@ CLI11_INLINE void Option::run_callback() { CLI11_NODISCARD CLI11_INLINE const std::string &Option::matching_name(const Option &other) const { static const std::string estring; + bool bothConfigurable = configurable_ && other.configurable_; for(const std::string &sname : snames_) { if(other.check_sname(sname)) return sname; - if(other.check_lname(sname)) + if(bothConfigurable && other.check_lname(sname)) return sname; } for(const std::string &lname : lnames_) { if(other.check_lname(lname)) return lname; - if(lname.size() == 1) { + if(lname.size() == 1 && bothConfigurable) { if(other.check_sname(lname)) { return lname; } } } - if(snames_.empty() && lnames_.empty() && !pname_.empty()) { + if(bothConfigurable && snames_.empty() && lnames_.empty() && !pname_.empty()) { if(other.check_sname(pname_) || other.check_lname(pname_) || pname_ == other.pname_) return pname_; } - if(other.snames_.empty() && other.fnames_.empty() && !other.pname_.empty()) { + if(bothConfigurable && other.snames_.empty() && other.fnames_.empty() && !other.pname_.empty()) { if(check_sname(other.pname_) || check_lname(other.pname_) || (pname_ == other.pname_)) return other.pname_; } @@ -6500,7 +6511,7 @@ class App { /// if error error on an extra argument, and if capture feed it to the app config_extras_mode allow_config_extras_{config_extras_mode::ignore}; - /// If true, return immediately on an unrecognized option (implies allow_extras) INHERITABLE + /// If true, cease processing on an unrecognized option (implies allow_extras) INHERITABLE bool prefix_command_{false}; /// If set to true the name was automatically generated from the command line vs a user set name @@ -6832,9 +6843,10 @@ class App { return this; } - /// Do not parse anything after the first unrecognized option and return - App *prefix_command(bool allow = true) { - prefix_command_ = allow; + /// Do not parse anything after the first unrecognized option (if true) all remaining arguments are stored in + /// remaining args + App *prefix_command(bool is_prefix = true) { + prefix_command_ = is_prefix; return this; } @@ -7730,6 +7742,11 @@ class Option_group : public App { : App(std::move(group_description), "", parent) { group(group_name); // option groups should have automatic fallthrough + if(group_name.empty() || group_name.front() == '+') { + // help will not be used by default in these contexts + set_help_flag(""); + set_help_all_flag(""); + } } using App::add_option; /// Add an existing option to the Option_group @@ -7982,19 +7999,19 @@ CLI11_INLINE Option *App::add_option(std::string option_name, } auto *op = get_option_no_throw(test_name); - if(op != nullptr) { + if(op != nullptr && op->get_configurable()) { throw(OptionAlreadyAdded("added option positional name matches existing option: " + test_name)); } } else if(parent_ != nullptr) { for(auto &ln : myopt.lnames_) { auto *op = parent_->get_option_no_throw(ln); - if(op != nullptr) { + if(op != nullptr && op->get_configurable()) { throw(OptionAlreadyAdded("added option matches existing positional option: " + ln)); } } for(auto &sn : myopt.snames_) { auto *op = parent_->get_option_no_throw(sn); - if(op != nullptr) { + if(op != nullptr && op->get_configurable()) { throw(OptionAlreadyAdded("added option matches existing positional option: " + sn)); } } @@ -8592,7 +8609,14 @@ CLI11_INLINE std::vector App::get_options(const std::functionget_name().empty() && !subc->get_group().empty() && subc->get_group().front() == '+') { + std::vector subcopts = subc->get_options(filter); + options.insert(options.end(), subcopts.begin(), subcopts.end()); + } + } return options; } @@ -8606,7 +8630,13 @@ CLI11_INLINE std::vector