This is an automated email from the ASF dual-hosted git repository. fgerlits pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
The following commit(s) were added to refs/heads/main by this push: new fe7abb230 MINIFICPP-2204 Fix build with clang16 and upgrade in CI fe7abb230 is described below commit fe7abb230d837bc5de3684e16c6533c1c02dcae4 Author: Gabor Gyimesi <gamezb...@gmail.com> AuthorDate: Mon Aug 28 13:40:13 2023 +0200 MINIFICPP-2204 Fix build with clang16 and upgrade in CI Also revise clang tidy checks: - List all checks individually to avoid alias checks being added - Remove checks cert-dcl37-c, cert-dcl51-cpp, bugprone-reserved-identifier and readability-identifier-naming as each of their runtime is over 3 minutes. This change reduced the runtime of the tested file from 13 minutes to 18 seconds. Signed-off-by: Ferenc Gerlits <fgerl...@gmail.com> This closes #1642 --- .clang-tidy | 89 +++++++++++++++++++++++++++++++----- .github/workflows/ci.yml | 42 ++++++++++------- CMakeLists.txt | 9 ---- cmake/Abseil.cmake | 10 +++- cmake/BundledLibcURL.cmake | 6 ++- libminifi/include/utils/TimeUtil.h | 13 +++++- run_clang_tidy.sh | 2 +- thirdparty/abseil/rename-crc32.patch | 74 ++++++++++++++++++++++++++++++ thirdparty/curl/strerror.patch | 31 +++++++++++++ 9 files changed, 232 insertions(+), 44 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 6dac184f4..2345a2425 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,16 +1,82 @@ Checks: > -clang-analyzer-optin.cplusplus.VirtualCall, - boost*, - portability*, - bugprone*, - -bugprone-narrowing-conversions, - -bugprone-implicit-widening-of-multiplication-result, - -bugprone-macro-parentheses, - -bugprone-easily-swappable-parameters, - -bugprone-exception-escape, - cert*, - -cert-err58-cpp, - -cert-err33-c, + boost-use-to-string, + portability-restrict-system-includes, + portability-simd-intrinsics, + portability-std-allocator-const, + bugprone-argument-comment, + bugprone-assert-side-effect, + bugprone-assignment-in-if-condition, + bugprone-bad-signal-to-kill-thread, + bugprone-bool-pointer-implicit-conversion, + bugprone-branch-clone, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-dynamic-static-initializers, + bugprone-fold-init-type, + bugprone-forward-declaration-namespace, + bugprone-forwarding-reference-overload, + bugprone-inaccurate-erase, + bugprone-incorrect-roundings, + bugprone-infinite-loop, + bugprone-integer-division, + bugprone-lambda-function-name, + bugprone-macro-repeated-side-effects, + bugprone-misplaced-operator-in-strlen-in-alloc, + bugprone-misplaced-pointer-arithmetic-in-alloc, + bugprone-misplaced-widening-cast, + bugprone-move-forwarding-reference, + bugprone-multiple-statement-macro, + bugprone-no-escape, + bugprone-not-null-terminated-result, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-redundant-branch-condition, + bugprone-shared-ptr-array-mismatch, + bugprone-signal-handler, + bugprone-signed-char-misuse, + bugprone-sizeof-container, + bugprone-sizeof-expression, + bugprone-spuriously-wake-up-functions, + bugprone-standalone-empty, + bugprone-string-constructor, + bugprone-string-integer-assignment, + bugprone-string-literal-with-embedded-nul, + bugprone-stringview-nullptr, + bugprone-suspicious-enum-usage, + bugprone-suspicious-include, + bugprone-suspicious-memory-comparison, + bugprone-suspicious-memset-usage, + bugprone-suspicious-missing-comma, + bugprone-suspicious-realloc-usage, + bugprone-suspicious-semicolon, + bugprone-suspicious-string-compare, + bugprone-swapped-arguments, + bugprone-terminating-continue, + bugprone-throw-keyword-missing, + bugprone-too-small-loop-variable, + bugprone-unchecked-optional-access, + bugprone-undefined-memory-manipulation, + bugprone-undelegated-constructor, + bugprone-unhandled-exception-at-new, + bugprone-unhandled-self-assignment, + bugprone-unused-raii, + bugprone-unused-return-value, + bugprone-use-after-move, + bugprone-virtual-near-miss, + cert-dcl21-cpp, + cert-dcl50-cpp, + cert-dcl58-cpp, + cert-err33-c, + cert-err34-c, + cert-err52-cpp, + cert-err60-cpp, + cert-flp30-c, + cert-mem57-cpp, + cert-msc50-cpp, + cert-msc51-cpp, + cert-oop57-cpp, + cert-oop58-cpp, hicpp-avoid-goto, hicpp-exception-baseclass, hicpp-multiway-paths-covered, @@ -68,7 +134,6 @@ Checks: > readability-delete-null-pointer, readability-duplicate-include, readability-function-size, - readability-identifier-naming, readability-inconsistent-declaration-parameter-name, readability-isolate-declaration, readability-make-member-function-const, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 265a5d339..d1ea8189c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -180,10 +180,10 @@ jobs: with: name: ubuntu-binaries path: build/bin - ubuntu_20_04_clang: - name: "ubuntu-20.04-clang" - runs-on: ubuntu-20.04 - timeout-minutes: 180 + ubuntu_22_04_clang: + name: "ubuntu-22.04-clang" + runs-on: ubuntu-22.04 + timeout-minutes: 240 steps: - id: checkout uses: actions/checkout@v3 @@ -191,25 +191,35 @@ jobs: uses: actions/cache/restore@v3 with: path: ~/.ccache - key: ubuntu-20.04-all-clang-ccache-${{github.ref}}-${{github.sha}} + key: ubuntu-22.04-all-clang-ccache-${{github.ref}}-${{github.sha}} restore-keys: | - ubuntu-20.04-all-clang-ccache-${{github.ref}}- - ubuntu-20.04-all-clang-ccache-refs/heads/main- + ubuntu-22.04-all-clang-ccache-${{github.ref}}- + ubuntu-22.04-all-clang-ccache-refs/heads/main- - id: install_deps run: | wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key|sudo apt-key add - - echo "deb http://apt.llvm.org/focal/ llvm-toolchain-focal main" | sudo tee -a /etc/apt/sources.list - echo "deb-src http://apt.llvm.org/focal/ llvm-toolchain-focal main" | sudo tee -a /etc/apt/sources.list - echo "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-14 main" | sudo tee -a /etc/apt/sources.list - echo "deb-src http://apt.llvm.org/focal/ llvm-toolchain-focal-14 main" | sudo tee -a /etc/apt/sources.list + echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy main" | sudo tee -a /etc/apt/sources.list + echo "deb-src http://apt.llvm.org/jammy/ llvm-toolchain-jammy main" | sudo tee -a /etc/apt/sources.list + echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-16 main" | sudo tee -a /etc/apt/sources.list + echo "deb-src http://apt.llvm.org/jammy/ llvm-toolchain-jammy-16 main" | sudo tee -a /etc/apt/sources.list sudo apt update - sudo apt install -y ccache libfl-dev libpcap-dev libusb-1.0-0-dev libpng-dev libgps-dev clang-14 clang-tidy-14 libc++-14-dev libc++abi-14-dev libsqliteodbc lua5.3 liblua5.3-dev flake8 parallel + sudo apt install -y ccache libfl-dev libpcap-dev libusb-1.0-0-dev libpng-dev libgps-dev clang-16 clang-tidy-16 libc++-16-dev libc++abi-16-dev libc++1-16 libc++abi1-16 libunwind-16 libsqliteodbc lua5.3 liblua5.3-dev flake8 parallel echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV echo -e "127.0.0.1\t$HOSTNAME" | sudo tee -a /etc/hosts > /dev/null + - id: free_disk_space + run: | + # We can gain additional disk space on the Ubuntu runners thanks to these suggestions: + # https://github.com/actions/runner-images/issues/2840#issuecomment-790492173 + # https://github.com/actions/runner-images/issues/2606#issuecomment-772683150 + sudo rm -rf /usr/share/dotnet + sudo rm -rf /usr/local/lib/android + sudo rm -rf /opt/ghc + sudo rm -rf "/usr/local/share/boost" + sudo rm -rf "$AGENT_TOOLSDIRECTORY" - name: build run: | - export CC=clang-14 - export CXX=clang++-14 + export CC=clang-16 + export CXX=clang++-16 ./bootstrap.sh -e -t cd build export CXXFLAGS="${CXXFLAGS} -stdlib=libc++" @@ -251,8 +261,8 @@ jobs: git checkout ${{ github.sha }} fi # https://stackoverflow.com/questions/58466701/clang-tidy-cant-locate-stdlib-headers - sed -i -e 's/\/usr\/lib\/ccache\/clang++-14/\/lib\/llvm-14\/bin\/clang++/g' build/compile_commands.json - sed -i -e 's/\/usr\/lib\/ccache\/clang-14/\/lib\/llvm-14\/bin\/clang/g' build/compile_commands.json + sed -i -e 's/\/usr\/lib\/ccache\/clang++-16/\/lib\/llvm-16\/bin\/clang++/g' build/compile_commands.json + sed -i -e 's/\/usr\/lib\/ccache\/clang-16/\/lib\/llvm-16\/bin\/clang/g' build/compile_commands.json parallel -j$(( $(nproc) + 1 )) ./run_clang_tidy.sh ::: ${FILES} - name: check-cores diff --git a/CMakeLists.txt b/CMakeLists.txt index ca1562295..511aa0434 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -323,15 +323,6 @@ include(ExpectedLite) # magic_enum include(MagicEnum) -# Update passthrough args used in configurations in patch commands -if (WIN32) - set(PASSTHROUGH_CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /w") - set(PASSTHROUGH_CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /w") -else() - set(PASSTHROUGH_CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -w") - set(PASSTHROUGH_CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -w") -endif() - # Setup warning flags if(MSVC) if(FAIL_ON_WARNINGS) diff --git a/cmake/Abseil.cmake b/cmake/Abseil.cmake index 1afc06757..a4497a7e4 100644 --- a/cmake/Abseil.cmake +++ b/cmake/Abseil.cmake @@ -20,9 +20,15 @@ include(FetchContent) set(ABSL_PROPAGATE_CXX_STD ON CACHE INTERNAL absl-propagate-cxx) set(ABSL_ENABLE_INSTALL ON CACHE INTERNAL "") set(BUILD_TESTING OFF CACHE STRING "" FORCE) + +set(PATCH_FILE "${CMAKE_SOURCE_DIR}/thirdparty/abseil/rename-crc32.patch") +set(PC ${Bash_EXECUTABLE} -c "set -x &&\ + (\\\"${Patch_EXECUTABLE}\\\" -p1 -R -s -f --dry-run -i \\\"${PATCH_FILE}\\\" || \\\"${Patch_EXECUTABLE}\\\" -p1 -N -i \\\"${PATCH_FILE}\\\")") + FetchContent_Declare( absl - URL https://github.com/abseil/abseil-cpp/archive/refs/tags/20220623.1.tar.gz - URL_HASH SHA256=91ac87d30cc6d79f9ab974c51874a704de9c2647c40f6932597329a282217ba8 + URL https://github.com/abseil/abseil-cpp/archive/refs/tags/20230802.0.tar.gz + URL_HASH SHA256=59d2976af9d6ecf001a81a35749a6e551a335b949d34918cfade07737b9d93c5 + PATCH_COMMAND "${PC}" ) FetchContent_MakeAvailable(absl) diff --git a/cmake/BundledLibcURL.cmake b/cmake/BundledLibcURL.cmake index 961c93781..775a2d7e6 100644 --- a/cmake/BundledLibcURL.cmake +++ b/cmake/BundledLibcURL.cmake @@ -17,9 +17,11 @@ function(use_bundled_curl SOURCE_DIR BINARY_DIR) # Define patch step - set(PATCH_FILE "${SOURCE_DIR}/thirdparty/curl/module-path.patch") + set(PATCH_FILE_1 "${SOURCE_DIR}/thirdparty/curl/module-path.patch") + set(PATCH_FILE_2 "${SOURCE_DIR}/thirdparty/curl/strerror.patch") set(PC ${Bash_EXECUTABLE} -c "set -x && \ - (\"${Patch_EXECUTABLE}\" -p1 -R -s -f --dry-run -i \"${PATCH_FILE}\" || \"${Patch_EXECUTABLE}\" -p1 -N -i \"${PATCH_FILE}\")") + (\"${Patch_EXECUTABLE}\" -p1 -R -s -f --dry-run -i \"${PATCH_FILE_1}\" || \"${Patch_EXECUTABLE}\" -p1 -N -i \"${PATCH_FILE_1}\") && \ + (\"${Patch_EXECUTABLE}\" -p1 -R -s -f --dry-run -i \"${PATCH_FILE_2}\" || \"${Patch_EXECUTABLE}\" -p1 -N -i \"${PATCH_FILE_2}\")") # Define byproducts if (WIN32) set(BYPRODUCT "lib/libcurl.lib") diff --git a/libminifi/include/utils/TimeUtil.h b/libminifi/include/utils/TimeUtil.h index 60d4f289c..a6107fa2f 100644 --- a/libminifi/include/utils/TimeUtil.h +++ b/libminifi/include/utils/TimeUtil.h @@ -35,15 +35,24 @@ #include "StringUtils.h" // libc++ doesn't define operator<=> on durations, and apparently the operator rewrite rules don't automagically make one -#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 16000 +#if defined(_LIBCPP_VERSION) #include <compare> +#include <concepts> #endif #include "date/date.h" #define TIME_FORMAT "%Y-%m-%d %H:%M:%S" -#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 16000 +#if defined(_LIBCPP_VERSION) +namespace org::apache::nifi::minifi::detail { +template<typename T> +concept has_spaceship_operator = requires(T t, T u) { t <=> u; }; // NOLINT(readability/braces) +} // namespace org::apache::nifi::minifi::detail + +static_assert(!org::apache::nifi::minifi::detail::has_spaceship_operator<std::chrono::duration<double>>, + "Current libc++ version supports spaceship operator for durations, remove this workaround!"); + template<typename Rep1, typename Period1, typename Rep2, typename Period2> std::strong_ordering operator<=>(std::chrono::duration<Rep1, Period1> lhs, std::chrono::duration<Rep2, Period2> rhs) { if (lhs < rhs) { diff --git a/run_clang_tidy.sh b/run_clang_tidy.sh index 9600227e4..bb6314da3 100755 --- a/run_clang_tidy.sh +++ b/run_clang_tidy.sh @@ -27,4 +27,4 @@ if ! [[ -f "${FILE}" ]]; then exit 0 fi -clang-tidy-14 -warnings-as-errors=* -quiet -p build "$FILE" +clang-tidy-16 -warnings-as-errors=* -quiet -p build "$FILE" diff --git a/thirdparty/abseil/rename-crc32.patch b/thirdparty/abseil/rename-crc32.patch new file mode 100644 index 000000000..45b845bf4 --- /dev/null +++ b/thirdparty/abseil/rename-crc32.patch @@ -0,0 +1,74 @@ +diff --git a/CMake/AbseilDll.cmake b/CMake/AbseilDll.cmake +index f0d984ae..6e6dc40b 100644 +--- a/CMake/AbseilDll.cmake ++++ b/CMake/AbseilDll.cmake +@@ -474,7 +474,7 @@ set(ABSL_INTERNAL_DLL_TARGETS + "crc_cord_state" + "crc_cpu_detect" + "crc_internal" +- "crc32c" ++ "crc32c_internal" + "debugging" + "debugging_internal" + "demangle_internal" +diff --git a/absl/crc/BUILD.bazel b/absl/crc/BUILD.bazel +index cdbaa9b2..78dac82e 100644 +--- a/absl/crc/BUILD.bazel ++++ b/absl/crc/BUILD.bazel +@@ -67,7 +67,7 @@ cc_library( + ) + + cc_library( +- name = "crc32c", ++ name = "crc32c_internal", + srcs = [ + "crc32c.cc", + "internal/crc32c_inline.h", +diff --git a/absl/crc/CMakeLists.txt b/absl/crc/CMakeLists.txt +index 21247160..f72b6cb3 100644 +--- a/absl/crc/CMakeLists.txt ++++ b/absl/crc/CMakeLists.txt +@@ -55,7 +55,7 @@ absl_cc_library( + + absl_cc_library( + NAME +- crc32c ++ crc32c_internal + HDRS + "crc32c.h" + "internal/crc32c.h" +@@ -89,7 +89,7 @@ absl_cc_test( + COPTS + ${ABSL_DEFAULT_COPTS} + DEPS +- absl::crc32c ++ absl::crc32c_internal + absl::strings + absl::str_format + GTest::gtest_main +@@ -129,7 +129,7 @@ absl_cc_test( + COPTS + ${ABSL_DEFAULT_COPTS} + DEPS +- absl::crc32c ++ absl::crc32c_internal + absl::memory + absl::random_random + absl::random_distributions +@@ -159,7 +159,7 @@ absl_cc_library( + COPTS + ${ABSL_DEFAULT_COPTS} + DEPS +- absl::crc32c ++ absl::crc32c_internal + absl::config + absl::strings + ) +@@ -173,6 +173,6 @@ absl_cc_test( + ${ABSL_DEFAULT_COPTS} + DEPS + absl::crc_cord_state +- absl::crc32c ++ absl::crc32c_internal + GTest::gtest_main + ) diff --git a/thirdparty/curl/strerror.patch b/thirdparty/curl/strerror.patch new file mode 100644 index 000000000..6527dd450 --- /dev/null +++ b/thirdparty/curl/strerror.patch @@ -0,0 +1,31 @@ +diff --git a/lib/strerror.c b/lib/strerror.c +index bd9cc535c..0da7b09e4 100644 +--- a/lib/strerror.c ++++ b/lib/strerror.c +@@ -861,7 +861,8 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) + } + #else /* not Windows coming up */ + +-#if defined(HAVE_STRERROR_R) && defined(HAVE_POSIX_STRERROR_R) ++#if defined(HAVE_STRERROR_R) ++#if (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE + /* + * The POSIX-style strerror_r() may set errno to ERANGE if insufficient + * storage is supplied via 'strerrbuf' and 'buflen' to hold the generated +@@ -871,7 +872,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) + if('\0' == buf[0]) + msnprintf(buf, max, "Unknown error %d", err); + } +-#elif defined(HAVE_STRERROR_R) && defined(HAVE_GLIBC_STRERROR_R) ++#else + /* + * The glibc-style strerror_r() only *might* use the buffer we pass to + * the function, but it always returns the error message as a pointer, +@@ -885,6 +886,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) + else + msnprintf(buf, max, "Unknown error %d", err); + } ++#endif + #else + { + /* !checksrc! disable STRERROR 1 */