From 12e35fa4192845af42a6b5e9a5031f2c556901c3 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 27 Oct 2023 15:55:47 -0400 Subject: [PATCH 01/17] Refactor CI to return better results faster --- .github/actions/build/action.yml | 36 +++ .github/workflows/main_ci.yml | 252 +++++------------- CMakeLists.txt | 25 +- .../{boringssl_1.1 => boringssl}/vcpkg.json | 0 .../openssl_1.1/vcpkg.json | 0 lib/bytes/test/bytes.cpp | 23 +- 6 files changed, 130 insertions(+), 206 deletions(-) create mode 100644 .github/actions/build/action.yml rename alternatives/{boringssl_1.1 => boringssl}/vcpkg.json (100%) rename vcpkg.json => alternatives/openssl_1.1/vcpkg.json (100%) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml new file mode 100644 index 00000000..153f0b9d --- /dev/null +++ b/.github/actions/build/action.yml @@ -0,0 +1,36 @@ +name: Install build prerequisites + +inputs: + cache-dir: + description: Where to put vcpkg cache + required: true + +runs: + using: "composite" + steps: + - name: Capture vcpkg revision for use in cache key + shell: bash + run: | + git -C vcpkg rev-parse HEAD > vcpkg_commit.txt + + - name: Restore cache + uses: actions/cache@v3 + with: + path: ${{ inputs.cache-dir }} + key: v01-vcpkg-${{ runner.os }}-${{ hashFiles('vcpkg_commit.txt', 'alternatives/*/vcpkg.json') }} + restore-keys: | + v01-vcpkg-${{ runner.os }} + + - name: Install dependencies (macOS) + if: ${{ runner.os == 'macOS' }} + shell: bash + run: | + brew install llvm pkg-config nasm + ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format" + ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy" + + - name: Install dependencies (Ubuntu) + if: ${{ runner.os == 'Linux' }} + shell: bash + run: | + sudo apt-get install -y linux-headers-$(uname -r) nasm diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index 2efa2f4d..afbf93a4 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -11,14 +11,8 @@ on: env: CMAKE_BUILD_PARALLEL_LEVEL: 3 CTEST_OUTPUT_ON_FAILURE: 1 - CMAKE_BUILD_DIR: ${{ github.workspace }}/build - CMAKE_BUILD_OPENSSL3_DIR: ${{ github.workspace }}/build_openssl3 - CMAKE_BUILD_BORINGSSL_DIR: ${{ github.workspace }}/build_boringssl - VCPKG_BINARY_SOURCES: files,${{ github.workspace }}/build/cache,readwrite - VCPKG_TOOLCHAIN_FILE: ${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake - VCPKG_REPO: ${{ github.workspace }}/vcpkg - CACHE_VERSION: v01 - CACHE_NAME: vcpkg + VCPKG_BINARY_SOURCES: files,${{ github.workspace }}/vcpkg_cache,readwrite + CMAKE_TOOLCHAIN_FILE: ${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake jobs: formatting-check: @@ -34,226 +28,116 @@ jobs: include-regex: '^\./(src|include|test|cmd)/.*\.(cpp|h)$' fallback-style: 'Mozilla' - quick-linux-interop-check: + build-and-unit-test: needs: formatting-check - name: Quick Linux Check and Interop - runs-on: ubuntu-latest - steps: - - name: Checkout repository and submodules - uses: actions/checkout@v4 - with: - submodules: recursive - fetch-depth: 0 - - # write the commit hash of vcpkg to a text file so we can use it in the - # hashFiles for cache - - run: | - git -C ${{ env.VCPKG_REPO }} rev-parse HEAD > vcpkg_commit.txt - - # First, attempt to pull key key, if that is not present, pull one of the - # restore-keys so we do not need to build from scratch. - # CACHE_VERSION - provide a way to reset cache - # CACHE_NAME - name of the cache in order to manage it - # matrix.os - cache per OS and version - # hashFiles - Recache if the vcpkg files change - - name: Restore Cache - uses: actions/cache@v3 - with: - path: ${{ github.workspace }}/build/cache - key: ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-ubuntu-latest-${{ hashFiles('vcpkg_commit.txt', 'vcpkg.json', 'alternatives/openssl_3/vcpkg.json') }} - restore-keys: | - ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-ubuntu-latest - - - name: Dependencies - run: | - sudo apt-get install -y linux-headers-$(uname -r) nasm - - - name: Restore cache - uses: actions/cache@v3 - with: - path: ${{ github.workspace }}/build/cache - key: VCPKG-BinaryCache-${{ runner.os }} - - - name: Build (OpenSSL 1.1) - run: | - cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DTESTING=ON -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target all - - - name: Unit Test (OpenSSL 1.1) - run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target test - - - name: Build (Interop Harness) - run: | - cd cmd/interop - cmake -B build -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build build - - - name: Test self-interop - run: | - make -C cmd/interop self-test - - - name: Test interop on test vectors - run: | - make -C cmd/interop interop-test - - - name: Test gRPC live interop with self - run: | - cd cmd/interop - ./grpc-self-test.sh - - - name: Build (OpenSSL 3) - run: | - cmake -B "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" -DTESTING=ON -DVCPKG_MANIFEST_DIR="alternatives/openssl_3" -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" - - - name: Unit Test (OpenSSL 3) - run: | - cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" --target test - - - name: Build (BoringSSL) - run: | - cmake -B "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" -DTESTING=ON -DVCPKG_MANIFEST_DIR="alternatives/boringssl_1.1" -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" - - - name: Unit Test (BoringSSL) - run: | - cmake --build "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" --target test - - platform-sanitizer-tests: - if: github.event.pull_request.draft == false - needs: quick-linux-interop-check - name: Build and test platforms using sanitizers and clang-tidy + name: Build and test runs-on: ${{ matrix.os }} strategy: - fail-fast: false matrix: os: [windows-latest, ubuntu-latest, macos-latest] + crypto: [openssl_1.1, openssl_3, boringssl] include: - os: windows-latest - ossl3-vcpkg-dir: "alternatives\\openssl_3" - boringssl-vcpkg-dir: "alternatives\\boringssl_1.1" ctest-target: RUN_TESTS - os: ubuntu-latest - ossl3-vcpkg-dir: "alternatives/openssl_3" - boringssl-vcpkg-dir: "alternatives/boringssl_1.1" ctest-target: test - os: macos-latest - ossl3-vcpkg-dir: "alternatives/openssl_3" - boringssl-vcpkg-dir: "alternatives/boringssl_1.1" ctest-target: test + env: + BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}" + CRYPTO_DIR: "./alternatives/${{ matrix.crypto }}" + steps: - - name: Checkout repository and submodules - uses: actions/checkout@v4 + - uses: actions/checkout@v4 with: submodules: recursive fetch-depth: 0 - # write the commit hash of vcpkg to a text file so we can use it in the - # hashFiles for cache - - run: | - git -C ${{ env.VCPKG_REPO }} rev-parse HEAD > vcpkg_commit.txt - - # First, attempt to pull key key, if that is not present, pull one of the - # restore-keys so we do not need to build from scratch. - # CACHE_VERSION - provide a way to reset cache - # CACHE_NAME - name of the cache in order to manage it - # matrix.os - cache per OS and version - # hashFiles - Recache if the vcpkg files change - - name: Restore Cache - uses: actions/cache@v3 + - uses: ./.github/actions/build with: - path: ${{ github.workspace }}/build/cache - key: ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-${{ matrix.os }}-${{ hashFiles('vcpkg_commit.txt', 'vcpkg.json', 'alternatives/openssl_3/vcpkg.json') }} - restore-keys: | - ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-${{ matrix.os }} + cache-dir: ${{ github.workspace }}/vcpkg_cache - - name: Dependencies (macOs) - if: ${{ matrix.os == 'macos-latest' }} + - name: Build run: | - brew install llvm pkg-config nasm - ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format" - ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy" + # XXX(RLB): If we do not have SANITIZERS=ON here, the Windows CI builds + # hang in the middle of unit testing. + cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON -DSANITIZERS=ON + cmake --build "${{ env.BUILD_DIR }}" - - name: Dependencies (Ubuntu) - if: ${{ matrix.os == 'ubuntu-latest' }} + - name: Unit Test run: | - sudo apt-get install -y linux-headers-$(uname -r) nasm + cmake --build "${{ env.BUILD_DIR }}" --target "${{ matrix.ctest-target}}" + + interop-test: + if: github.event.pull_request.draft == false + needs: build-and-unit-test + name: Interop test + runs-on: ubuntu-latest - - name: Build (OpenSSL 1.1) - run: | - cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_DIR }}" + env: + BUILD_DIR: "${RUNNER_TEMP}/build_openssl_1.1" + CRYPTO_DIR: "./alternatives/openssl_1.1" + + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + fetch-depth: 0 - - name: Unit Test (OpenSSL 1.1) + - uses: ./.github/actions/build + with: + cache-dir: ${{ github.workspace }}/vcpkg_cache + + - name: Build run: | - cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target "${{ matrix.ctest-target}}" + cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" + cmake --build "${{ env.BUILD_DIR }}" - - name: Build (OpenSSL 3) + - name: Build (Interop Harness) run: | - cmake -B "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DVCPKG_MANIFEST_DIR="${{ matrix.ossl3-vcpkg-dir }}" -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" + cd cmd/interop + cmake -B build + cmake --build build - - name: Unit Test (OpenSSL 3) + - name: Test self-interop run: | - cmake --build "${{ env.CMAKE_BUILD_OPENSSL3_DIR }}" --target "${{ matrix.ctest-target}}" + make -C cmd/interop self-test - - name: Build (BoringSSL) + - name: Test interop on test vectors run: | - cmake -B "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DVCPKG_MANIFEST_DIR="${{ matrix.boringssl-vcpkg-dir }}" -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" + make -C cmd/interop interop-test - - name: Unit Test (BoringSSL) + - name: Test gRPC live interop with self run: | - cmake --build "${{ env.CMAKE_BUILD_BORINGSSL_DIR }}" --target "${{ matrix.ctest-target}}" + cd cmd/interop + ./grpc-self-test.sh - old-macos-compatibility: + clang-tidy: if: github.event.pull_request.draft == false - needs: quick-linux-interop-check - name: Build for older MacOS - runs-on: macos-latest + needs: build-and-unit-test + name: Build with clang-tidy + runs-on: ubuntu-latest + strategy: + matrix: + crypto: [openssl_1.1, openssl_3, boringssl] env: - CMAKE_BUILD_DIR: ${{ github.workspace }}/build - VCPKG_BINARY_SOURCES: files,${{ github.workspace }}/build/cache,readwrite - MACOSX_DEPLOYMENT_TARGET: 10.11 + BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}" + CRYPTO_DIR: "./alternatives/${{ matrix.crypto }}" steps: - - name: Checkout repository and submodules - uses: actions/checkout@v4 + - uses: actions/checkout@v4 with: submodules: recursive fetch-depth: 0 - # write the commit hash of vcpkg to a text file so we can use it in the - # hashFiles for cache - - run: | - git -C ${{ env.VCPKG_REPO }} rev-parse HEAD > vcpkg_commit.txt - - # First, attempt to pull key key, if that is not present, pull one of the - # restore-keys so we do not need to build from scratch. - # CACHE_VERSION - provide a way to reset cache - # CACHE_NAME - name of the cache in order to manage it - # matrix.os - cache per OS and version - # hashFiles - Recache if the vcpkg files change - - name: Restore Cache - uses: actions/cache@v3 + - uses: ./.github/actions/build with: - path: ${{ github.workspace }}/build/cache - key: ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-macos-latest-legacy-${{ hashFiles('vcpkg_commit.txt', 'vcpkg.json', 'alternatives/openssl_3/vcpkg.json') }} - restore-keys: | - ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-macos-latest-legacy - ${{ env.CACHE_VERSION }}-${{ env.CACHE_NAME }}-macos-latest + cache-dir: ${{ github.workspace }}/vcpkg_cache - - name: Dependencies + - name: Build with clang-tidy run: | - brew install llvm pkg-config - ln -s "/usr/local/opt/llvm/bin/clang-format" "/usr/local/bin/clang-format" - ln -s "/usr/local/opt/llvm/bin/clang-tidy" "/usr/local/bin/clang-tidy" - - - name: Build - run: | - cmake -B "${{ env.CMAKE_BUILD_DIR }}" -DCMAKE_TOOLCHAIN_FILE="${{ env.VCPKG_TOOLCHAIN_FILE }}" - cmake --build "${{ env.CMAKE_BUILD_DIR }}" --target mlspp - + cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" \ + -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON + cmake --build "${{ env.BUILD_DIR }}" diff --git a/CMakeLists.txt b/CMakeLists.txt index c827512c..436f8e78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.12) +cmake_minimum_required(VERSION 3.13) project(mlspp VERSION 0.1 @@ -44,6 +44,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU" add_compile_options(-Wall -pedantic -Wextra -Werror -Wmissing-declarations) elseif(MSVC) add_compile_options(/W4 /WX) + add_definitions(-DWINDOWS) # MSVC helpfully recommends safer equivalents for things like # getenv, but they are not portable. @@ -51,21 +52,16 @@ elseif(MSVC) endif() if (SANITIZERS) + message("Enabling sanitizers") + add_definitions(-DSANITIZERS) + if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") - set(SANITIZERS "-fsanitize=address -fsanitize=undefined") + add_compile_options(-fsanitize=address -fsanitize=undefined) + add_link_options(-fsanitize=address -fsanitize=undefined) elseif(MSVC) - set(SANITIZERS "/fsanitize=address") + # MSVC uses a different flag, and doesn't require passing it to the linker + add_compile_options("/fsanitize=address") endif() - - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZERS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZERS}") - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZERS}") - set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${SANITIZERS}") - set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${SANITIZERS}") - add_definitions(-DSANITIZERS) -elseif (SANITIZERS AND MSVC) - message("Enabling sanitizers") - add_definitions("/fsanitize=address") endif() if(CLANG_TIDY) @@ -96,6 +92,9 @@ endif() ### Dependencies ### +# Configure vcpkg to only build release libraries +set(VCPKG_BUILD_TYPE release) + # External libraries find_package(OpenSSL REQUIRED) if ( OPENSSL_FOUND ) diff --git a/alternatives/boringssl_1.1/vcpkg.json b/alternatives/boringssl/vcpkg.json similarity index 100% rename from alternatives/boringssl_1.1/vcpkg.json rename to alternatives/boringssl/vcpkg.json diff --git a/vcpkg.json b/alternatives/openssl_1.1/vcpkg.json similarity index 100% rename from vcpkg.json rename to alternatives/openssl_1.1/vcpkg.json diff --git a/lib/bytes/test/bytes.cpp b/lib/bytes/test/bytes.cpp index 38d3136c..39b1a909 100644 --- a/lib/bytes/test/bytes.cpp +++ b/lib/bytes/test/bytes.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -8,12 +9,14 @@ using namespace std::literals::string_literals; // To check that memory is safely zeroized on destroy, we have to deliberately // do a use-after-free. This will be caught by the sanitizers, so we only do it -// when sanitizers are not enabled. -#ifndef SANITIZERS +// when sanitizers are not enabled. This test is also disabled on Windows +// because it appears to cause Windows CI runs to fail. (In addition, Windows +// appears to overwrite freed buffers with 0xCD, so this test is unnecessary.) +#if !defined(SANITIZERS) && !defined(WINDOWS) TEST_CASE("Zeroization") { const auto size = size_t(1024); - const auto canary = uint8_t(0xff); + const auto canary = uint8_t(0xa0); auto vec = std::make_unique(size, canary); const auto* begin = vec->data(); @@ -21,15 +24,17 @@ TEST_CASE("Zeroization") const auto* end = begin + size; vec.reset(); + const auto snapshot = std::vector(begin, end); + std::cout << "snapshot = " << to_hex(snapshot) << std::endl; + // In principle, the memory previously owned by the vector should be all zero // at this point. However, since this is now unallocated memory, the // allocator can do with it what it wants, and may have written something to - // it when deallocating. For example, on macOS, the allocator appears to - // write a single pointer at the beginning. Assuming other platforms are not - // too different, we verify that no more than a few pointer's worth of bytes - // are non-zero. - const auto non_zero_threshold = 4 * sizeof(void*); - REQUIRE(std::count(begin, end, 0) >= size - non_zero_threshold); + // it when deallocating. macOS and Linux mostly leave the buffer alone, + // writing a couple of pointers to the beginning. So we look for the buffer + // to be basically all zero. + const auto threshold = size - 4 * sizeof(void*); + REQUIRE(std::count(snapshot.begin(), snapshot.end(), 0) >= threshold); } #endif From 1770db53d90a7f26d42531c8a09dceefae345cc4 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:23:22 -0400 Subject: [PATCH 02/17] Address review comments from @glhewett --- .github/actions/build/action.yml | 8 +++++++- .github/workflows/main_ci.yml | 23 ++++++++++++++--------- CMakeLists.txt | 2 +- lib/bytes/test/bytes.cpp | 5 +---- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index 153f0b9d..30dd29f1 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -1,6 +1,12 @@ name: Install build prerequisites inputs: + os: + description: The operating system on which the test is being run + required: true + crypto-dir: + description: The manifest directory for crypto dependencies + required: true cache-dir: description: Where to put vcpkg cache required: true @@ -17,7 +23,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ runner.os }}-${{ hashFiles('vcpkg_commit.txt', 'alternatives/*/vcpkg.json') }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', "${{ inputs.crypto-dir }}/vcpkg.json") }} restore-keys: | v01-vcpkg-${{ runner.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index afbf93a4..30863bba 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -36,13 +36,6 @@ jobs: matrix: os: [windows-latest, ubuntu-latest, macos-latest] crypto: [openssl_1.1, openssl_3, boringssl] - include: - - os: windows-latest - ctest-target: RUN_TESTS - - os: ubuntu-latest - ctest-target: test - - os: macos-latest - ctest-target: test env: BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}" @@ -56,6 +49,8 @@ jobs: - uses: ./.github/actions/build with: + os: ${{ matrix.os }} + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -65,9 +60,15 @@ jobs: cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON -DSANITIZERS=ON cmake --build "${{ env.BUILD_DIR }}" - - name: Unit Test + - name: Unit Test (non-Windows) + if: matrix.os != "windows-latest" run: | - cmake --build "${{ env.BUILD_DIR }}" --target "${{ matrix.ctest-target}}" + cmake --build "${{ env.BUILD_DIR }}" --target test + + - name: Unit Test (Windows) + if: matrix.os == "windows-latest" + run: | + cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS interop-test: if: github.event.pull_request.draft == false @@ -87,6 +88,8 @@ jobs: - uses: ./.github/actions/build with: + os: ubuntu-latest + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -134,6 +137,8 @@ jobs: - uses: ./.github/actions/build with: + os: ubuntu-latest + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build with clang-tidy diff --git a/CMakeLists.txt b/CMakeLists.txt index 436f8e78..8875b9fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,7 +52,7 @@ elseif(MSVC) endif() if (SANITIZERS) - message("Enabling sanitizers") + message(STATUS "Enabling sanitizers") add_definitions(-DSANITIZERS) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") diff --git a/lib/bytes/test/bytes.cpp b/lib/bytes/test/bytes.cpp index 39b1a909..3c84c9c3 100644 --- a/lib/bytes/test/bytes.cpp +++ b/lib/bytes/test/bytes.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include @@ -24,15 +23,13 @@ TEST_CASE("Zeroization") const auto* end = begin + size; vec.reset(); - const auto snapshot = std::vector(begin, end); - std::cout << "snapshot = " << to_hex(snapshot) << std::endl; - // In principle, the memory previously owned by the vector should be all zero // at this point. However, since this is now unallocated memory, the // allocator can do with it what it wants, and may have written something to // it when deallocating. macOS and Linux mostly leave the buffer alone, // writing a couple of pointers to the beginning. So we look for the buffer // to be basically all zero. + const auto snapshot = std::vector(begin, end); const auto threshold = size - 4 * sizeof(void*); REQUIRE(std::count(snapshot.begin(), snapshot.end(), 0) >= threshold); } From 82e2728a4af5f75f38cd04000d213266b076a33f Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:23:43 -0400 Subject: [PATCH 03/17] Update the Makefile to match manifest location changes --- Makefile | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 03514fb6..6b7d72b7 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,9 @@ BUILD_DIR=build TEST_DIR=build/test CLANG_FORMAT=clang-format -i CLANG_TIDY=OFF +OPENSSL11_MANIFEST=alternatives/openssl_1.1 OPENSSL3_MANIFEST=alternatives/openssl_3 +BORINGSSL_MANIFEST=alternatives/boringssl TOOLCHAIN_FILE=vcpkg/scripts/buildsystems/vcpkg.cmake .PHONY: all dev dev3 test ctest dtest dbtest libs test-libs test-all everything ci ci3 clean cclean format @@ -26,6 +28,7 @@ dev: ${TOOLCHAIN_FILE} # Only enable testing, not clang-tidy/sanitizers; the latter make the build # too slow, and we can run them in CI cmake -B${BUILD_DIR} -DTESTING=ON -DCMAKE_BUILD_TYPE=Debug \ + -DVCPKG_MANIFEST_DIR=${OPENSSL11_MANIFEST} \ -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} dev3: ${TOOLCHAIN_FILE} @@ -34,6 +37,12 @@ dev3: ${TOOLCHAIN_FILE} -DVCPKG_MANIFEST_DIR=${OPENSSL3_MANIFEST} \ -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} +devB: ${TOOLCHAIN_FILE} + # Like `dev`, but using BoringSSL + cmake -B${BUILD_DIR} -DTESTING=ON -DCMAKE_BUILD_TYPE=Debug \ + -DVCPKG_MANIFEST_DIR=${BORINGSSL_MANIFEST} \ + -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} + test: ${BUILD_DIR} test/* cmake --build ${BUILD_DIR} --target mlspp_test @@ -64,13 +73,20 @@ everything: ${BUILD_DIR} cmake --build ${BUILD_DIR} ci: ${TOOLCHAIN_FILE} - cmake -B ${BUILD_DIR} -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON \ - -DCMAKE_BUILD_TYPE=Debug -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} + cmake -B ${BUILD_DIR} -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DCMAKE_BUILD_TYPE=Debug \ + -DVCPKG_MANIFEST_DIR=${OPENSSL11_MANIFEST} \ + -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} ci3: ${TOOLCHAIN_FILE} # Like `ci`, but using OpenSSL 3 - cmake -B ${BUILD_DIR} -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON \ - -DCMAKE_BUILD_TYPE=Debug -DVCPKG_MANIFEST_DIR=${OPENSSL3_MANIFEST} \ + cmake -B ${BUILD_DIR} -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DCMAKE_BUILD_TYPE=Debug \ + -DVCPKG_MANIFEST_DIR=${OPENSSL3_MANIFEST} \ + -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} + +ciB: ${TOOLCHAIN_FILE} + # Like `ci`, but using BoringSSL + cmake -B ${BUILD_DIR} -DTESTING=ON -DCLANG_TIDY=ON -DSANITIZERS=ON -DCMAKE_BUILD_TYPE=Debug \ + -DVCPKG_MANIFEST_DIR=${BORINGSSL_MANIFEST} \ -DCMAKE_TOOLCHAIN_FILE=${TOOLCHAIN_FILE} clean: From c989c154fdae561b66f964d7f72f66eb5a750eb8 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:43:44 -0400 Subject: [PATCH 04/17] Attempt to resolve CI YAML errors --- .github/actions/{build => prepare-build}/action.yml | 3 ++- .github/workflows/main_ci.yml | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) rename .github/actions/{build => prepare-build}/action.yml (86%) diff --git a/.github/actions/build/action.yml b/.github/actions/prepare-build/action.yml similarity index 86% rename from .github/actions/build/action.yml rename to .github/actions/prepare-build/action.yml index 30dd29f1..b9764065 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -1,4 +1,5 @@ name: Install build prerequisites +description: Restores the cache and installs dependencies inputs: os: @@ -23,7 +24,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', "${{ inputs.crypto-dir }}/vcpkg.json") }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", "${{ inputs.crypto-dir }}/vcpkg.json") }} restore-keys: | v01-vcpkg-${{ runner.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index 30863bba..f710657a 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -47,7 +47,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: os: ${{ matrix.os }} crypto-dir: ${{ env.CRYPTO_DIR }} @@ -86,7 +86,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: os: ubuntu-latest crypto-dir: ${{ env.CRYPTO_DIR }} @@ -135,7 +135,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: os: ubuntu-latest crypto-dir: ${{ env.CRYPTO_DIR }} From db4ab45759dde18f4b83fbeeab59469aee8687dd Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:45:05 -0400 Subject: [PATCH 05/17] Attempt to resolve CI YAML errors --- .github/actions/prepare-build/action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/prepare-build/action.yml index b9764065..e49c45ce 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -15,7 +15,7 @@ inputs: runs: using: "composite" steps: - - name: Capture vcpkg revision for use in cache key + - name: Capture vcpkg revision and manifest for use in cache key shell: bash run: | git -C vcpkg rev-parse HEAD > vcpkg_commit.txt @@ -24,7 +24,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", "${{ inputs.crypto-dir }}/vcpkg.json") }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", 'alternatives/*/vcpkg.json') }} restore-keys: | v01-vcpkg-${{ runner.os }} From 927182ab4f58c8dc332ba29a7343efab1440056e Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:46:43 -0400 Subject: [PATCH 06/17] Revert "Attempt to resolve CI YAML errors" This reverts commit db4ab45759dde18f4b83fbeeab59469aee8687dd. --- .github/actions/prepare-build/action.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/prepare-build/action.yml index e49c45ce..b9764065 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -15,7 +15,7 @@ inputs: runs: using: "composite" steps: - - name: Capture vcpkg revision and manifest for use in cache key + - name: Capture vcpkg revision for use in cache key shell: bash run: | git -C vcpkg rev-parse HEAD > vcpkg_commit.txt @@ -24,7 +24,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", 'alternatives/*/vcpkg.json') }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", "${{ inputs.crypto-dir }}/vcpkg.json") }} restore-keys: | v01-vcpkg-${{ runner.os }} From 181cdd44c0a122ba4d0f0bb8ea1d01fa3b05bfd0 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:46:52 -0400 Subject: [PATCH 07/17] Revert "Attempt to resolve CI YAML errors" This reverts commit c989c154fdae561b66f964d7f72f66eb5a750eb8. --- .github/actions/{prepare-build => build}/action.yml | 3 +-- .github/workflows/main_ci.yml | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) rename .github/actions/{prepare-build => build}/action.yml (86%) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/build/action.yml similarity index 86% rename from .github/actions/prepare-build/action.yml rename to .github/actions/build/action.yml index b9764065..30dd29f1 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/build/action.yml @@ -1,5 +1,4 @@ name: Install build prerequisites -description: Restores the cache and installs dependencies inputs: os: @@ -24,7 +23,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles("vcpkg_commit.txt", "${{ inputs.crypto-dir }}/vcpkg.json") }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', "${{ inputs.crypto-dir }}/vcpkg.json") }} restore-keys: | v01-vcpkg-${{ runner.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index f710657a..30863bba 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -47,7 +47,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/prepare-build + - uses: ./.github/actions/build with: os: ${{ matrix.os }} crypto-dir: ${{ env.CRYPTO_DIR }} @@ -86,7 +86,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/prepare-build + - uses: ./.github/actions/build with: os: ubuntu-latest crypto-dir: ${{ env.CRYPTO_DIR }} @@ -135,7 +135,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/prepare-build + - uses: ./.github/actions/build with: os: ubuntu-latest crypto-dir: ${{ env.CRYPTO_DIR }} From 180b83412469a66ec7790dbe6387ff9713f968cd Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:47:05 -0400 Subject: [PATCH 08/17] Revert "Address review comments from @glhewett" This reverts commit 1770db53d90a7f26d42531c8a09dceefae345cc4. --- .github/actions/build/action.yml | 8 +------- .github/workflows/main_ci.yml | 23 +++++++++-------------- CMakeLists.txt | 2 +- lib/bytes/test/bytes.cpp | 5 ++++- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index 30dd29f1..153f0b9d 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -1,12 +1,6 @@ name: Install build prerequisites inputs: - os: - description: The operating system on which the test is being run - required: true - crypto-dir: - description: The manifest directory for crypto dependencies - required: true cache-dir: description: Where to put vcpkg cache required: true @@ -23,7 +17,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', "${{ inputs.crypto-dir }}/vcpkg.json") }} + key: v01-vcpkg-${{ runner.os }}-${{ hashFiles('vcpkg_commit.txt', 'alternatives/*/vcpkg.json') }} restore-keys: | v01-vcpkg-${{ runner.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index 30863bba..afbf93a4 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -36,6 +36,13 @@ jobs: matrix: os: [windows-latest, ubuntu-latest, macos-latest] crypto: [openssl_1.1, openssl_3, boringssl] + include: + - os: windows-latest + ctest-target: RUN_TESTS + - os: ubuntu-latest + ctest-target: test + - os: macos-latest + ctest-target: test env: BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}" @@ -49,8 +56,6 @@ jobs: - uses: ./.github/actions/build with: - os: ${{ matrix.os }} - crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -60,15 +65,9 @@ jobs: cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON -DSANITIZERS=ON cmake --build "${{ env.BUILD_DIR }}" - - name: Unit Test (non-Windows) - if: matrix.os != "windows-latest" + - name: Unit Test run: | - cmake --build "${{ env.BUILD_DIR }}" --target test - - - name: Unit Test (Windows) - if: matrix.os == "windows-latest" - run: | - cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS + cmake --build "${{ env.BUILD_DIR }}" --target "${{ matrix.ctest-target}}" interop-test: if: github.event.pull_request.draft == false @@ -88,8 +87,6 @@ jobs: - uses: ./.github/actions/build with: - os: ubuntu-latest - crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -137,8 +134,6 @@ jobs: - uses: ./.github/actions/build with: - os: ubuntu-latest - crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build with clang-tidy diff --git a/CMakeLists.txt b/CMakeLists.txt index 8875b9fb..436f8e78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,7 +52,7 @@ elseif(MSVC) endif() if (SANITIZERS) - message(STATUS "Enabling sanitizers") + message("Enabling sanitizers") add_definitions(-DSANITIZERS) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") diff --git a/lib/bytes/test/bytes.cpp b/lib/bytes/test/bytes.cpp index 3c84c9c3..39b1a909 100644 --- a/lib/bytes/test/bytes.cpp +++ b/lib/bytes/test/bytes.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include @@ -23,13 +24,15 @@ TEST_CASE("Zeroization") const auto* end = begin + size; vec.reset(); + const auto snapshot = std::vector(begin, end); + std::cout << "snapshot = " << to_hex(snapshot) << std::endl; + // In principle, the memory previously owned by the vector should be all zero // at this point. However, since this is now unallocated memory, the // allocator can do with it what it wants, and may have written something to // it when deallocating. macOS and Linux mostly leave the buffer alone, // writing a couple of pointers to the beginning. So we look for the buffer // to be basically all zero. - const auto snapshot = std::vector(begin, end); const auto threshold = size - 4 * sizeof(void*); REQUIRE(std::count(snapshot.begin(), snapshot.end(), 0) >= threshold); } From e837e494b3c2f91910c5d9f3fa485b3931334532 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:48:40 -0400 Subject: [PATCH 09/17] Remove from test/bytes.cpp --- lib/bytes/test/bytes.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/bytes/test/bytes.cpp b/lib/bytes/test/bytes.cpp index 39b1a909..3c84c9c3 100644 --- a/lib/bytes/test/bytes.cpp +++ b/lib/bytes/test/bytes.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include @@ -24,15 +23,13 @@ TEST_CASE("Zeroization") const auto* end = begin + size; vec.reset(); - const auto snapshot = std::vector(begin, end); - std::cout << "snapshot = " << to_hex(snapshot) << std::endl; - // In principle, the memory previously owned by the vector should be all zero // at this point. However, since this is now unallocated memory, the // allocator can do with it what it wants, and may have written something to // it when deallocating. macOS and Linux mostly leave the buffer alone, // writing a couple of pointers to the beginning. So we look for the buffer // to be basically all zero. + const auto snapshot = std::vector(begin, end); const auto threshold = size - 4 * sizeof(void*); REQUIRE(std::count(snapshot.begin(), snapshot.end(), 0) >= threshold); } From aee9d6307169f33ccc70a64112e26d20195cd35d Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:50:06 -0400 Subject: [PATCH 10/17] Manually expand test target --- .github/workflows/main_ci.yml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index afbf93a4..b472787c 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -36,13 +36,6 @@ jobs: matrix: os: [windows-latest, ubuntu-latest, macos-latest] crypto: [openssl_1.1, openssl_3, boringssl] - include: - - os: windows-latest - ctest-target: RUN_TESTS - - os: ubuntu-latest - ctest-target: test - - os: macos-latest - ctest-target: test env: BUILD_DIR: "${RUNNER_TEMP}/build_${{ matrix.crypto }}" @@ -65,9 +58,15 @@ jobs: cmake -B "${{ env.BUILD_DIR }}" -DVCPKG_MANIFEST_DIR="${{ env.CRYPTO_DIR }}" -DTESTING=ON -DSANITIZERS=ON cmake --build "${{ env.BUILD_DIR }}" - - name: Unit Test + - name: Unit Test (non-Windows) + if: matrix.os != "windows-latest" run: | - cmake --build "${{ env.BUILD_DIR }}" --target "${{ matrix.ctest-target}}" + cmake --build "${{ env.BUILD_DIR }}" --target test + + - name: Unit Test (Windows) + if: matrix.os != "windows-latest" + run: | + cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS interop-test: if: github.event.pull_request.draft == false From 75b1455209d29dbcd1236810c4537fd001eea7c3 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:50:56 -0400 Subject: [PATCH 11/17] Remove 'if' lines (will break CI) --- .github/workflows/main_ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index b472787c..eff47597 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -59,12 +59,10 @@ jobs: cmake --build "${{ env.BUILD_DIR }}" - name: Unit Test (non-Windows) - if: matrix.os != "windows-latest" run: | cmake --build "${{ env.BUILD_DIR }}" --target test - name: Unit Test (Windows) - if: matrix.os != "windows-latest" run: | cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS From 44ce49ab434462e629656698e6e95de42138c5a4 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:53:23 -0400 Subject: [PATCH 12/17] Re-add if with single quotes --- .github/workflows/main_ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index eff47597..3c3813f9 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -59,10 +59,12 @@ jobs: cmake --build "${{ env.BUILD_DIR }}" - name: Unit Test (non-Windows) + if: matrix.os != 'windows-latest' run: | cmake --build "${{ env.BUILD_DIR }}" --target test - name: Unit Test (Windows) + if: matrix.os == 'windows-latest' run: | cmake --build "${{ env.BUILD_DIR }}" --target RUN_TESTS From 0222b0d47848f0045fe335d55463d866c8215e48 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:54:57 -0400 Subject: [PATCH 13/17] Rename composite action --- .github/actions/{build => prepare-build}/action.yml | 0 .github/workflows/main_ci.yml | 6 +++--- 2 files changed, 3 insertions(+), 3 deletions(-) rename .github/actions/{build => prepare-build}/action.yml (100%) diff --git a/.github/actions/build/action.yml b/.github/actions/prepare-build/action.yml similarity index 100% rename from .github/actions/build/action.yml rename to .github/actions/prepare-build/action.yml diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index 3c3813f9..c0d75001 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -47,7 +47,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: cache-dir: ${{ github.workspace }}/vcpkg_cache @@ -84,7 +84,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: cache-dir: ${{ github.workspace }}/vcpkg_cache @@ -131,7 +131,7 @@ jobs: submodules: recursive fetch-depth: 0 - - uses: ./.github/actions/build + - uses: ./.github/actions/prepare-build with: cache-dir: ${{ github.workspace }}/vcpkg_cache From af62b68307160f731894d1be46c24ca712659da0 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 13:57:51 -0400 Subject: [PATCH 14/17] Pass more input to the composite action --- .github/actions/prepare-build/action.yml | 8 +++++++- .github/workflows/main_ci.yml | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/prepare-build/action.yml index 153f0b9d..c2b2e12c 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -1,6 +1,12 @@ name: Install build prerequisites inputs: + os: + description: The operating system on which the test is being run + required: true + crypto-dir: + description: The manifest directory for crypto dependencies + required: true cache-dir: description: Where to put vcpkg cache required: true @@ -17,7 +23,7 @@ runs: uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ runner.os }}-${{ hashFiles('vcpkg_commit.txt', 'alternatives/*/vcpkg.json') }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', '${{ inputs.crypto-dir }}/vcpkg.json') }} restore-keys: | v01-vcpkg-${{ runner.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index c0d75001..7d40d6fe 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -49,6 +49,8 @@ jobs: - uses: ./.github/actions/prepare-build with: + os: ${{ matrix.os }} + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -86,6 +88,8 @@ jobs: - uses: ./.github/actions/prepare-build with: + os: ubuntu-latest + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -133,6 +137,8 @@ jobs: - uses: ./.github/actions/prepare-build with: + os: ubuntu-latest + crypto-dir: ${{ env.CRYPTO_DIR }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build with clang-tidy From 8f7d95f9bce15c742765c31313d0431898257bde Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 14:01:53 -0400 Subject: [PATCH 15/17] Fix the filename of the vcpkg manifest --- .github/actions/prepare-build/action.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/prepare-build/action.yml index c2b2e12c..5650a90e 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -14,18 +14,19 @@ inputs: runs: using: "composite" steps: - - name: Capture vcpkg revision for use in cache key + - name: Capture vcpkg revision and manifest for use in cache key shell: bash run: | - git -C vcpkg rev-parse HEAD > vcpkg_commit.txt + git -C vcpkg rev-parse HEAD > vcpkg_commit.txt + cp "${{ inputs.crypto-dir }}/vcpkg.json" vcpkg_manifest.txt - name: Restore cache uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', '${{ inputs.crypto-dir }}/vcpkg.json') }} + key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', 'vcpkg_manifest.txt') }} restore-keys: | - v01-vcpkg-${{ runner.os }} + v01-vcpkg-${{ inputs.os }} - name: Install dependencies (macOS) if: ${{ runner.os == 'macOS' }} From 73a3bf375a8d03b8bd4aa2cdc49ad0b8cdfafa26 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 14:07:12 -0400 Subject: [PATCH 16/17] Key on library name rather than manifest hash --- .github/actions/prepare-build/action.yml | 9 ++++----- .github/workflows/main_ci.yml | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/actions/prepare-build/action.yml b/.github/actions/prepare-build/action.yml index 5650a90e..1ea9dc02 100644 --- a/.github/actions/prepare-build/action.yml +++ b/.github/actions/prepare-build/action.yml @@ -4,8 +4,8 @@ inputs: os: description: The operating system on which the test is being run required: true - crypto-dir: - description: The manifest directory for crypto dependencies + crypto: + description: The crypto library being used required: true cache-dir: description: Where to put vcpkg cache @@ -14,17 +14,16 @@ inputs: runs: using: "composite" steps: - - name: Capture vcpkg revision and manifest for use in cache key + - name: Capture vcpkg revision for use in cache key shell: bash run: | git -C vcpkg rev-parse HEAD > vcpkg_commit.txt - cp "${{ inputs.crypto-dir }}/vcpkg.json" vcpkg_manifest.txt - name: Restore cache uses: actions/cache@v3 with: path: ${{ inputs.cache-dir }} - key: v01-vcpkg-${{ inputs.os }}-${{ hashFiles('vcpkg_commit.txt', 'vcpkg_manifest.txt') }} + key: v01-vcpkg-${{ inputs.os }}-${{ inputs.crypto }}-${{ hashFiles('vcpkg_commit.txt', 'alternatives/*/vcpkg.json') }} restore-keys: | v01-vcpkg-${{ inputs.os }} diff --git a/.github/workflows/main_ci.yml b/.github/workflows/main_ci.yml index 7d40d6fe..a31fe7c6 100644 --- a/.github/workflows/main_ci.yml +++ b/.github/workflows/main_ci.yml @@ -50,7 +50,7 @@ jobs: - uses: ./.github/actions/prepare-build with: os: ${{ matrix.os }} - crypto-dir: ${{ env.CRYPTO_DIR }} + crypto: ${{ matrix.crypto }} cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -89,7 +89,7 @@ jobs: - uses: ./.github/actions/prepare-build with: os: ubuntu-latest - crypto-dir: ${{ env.CRYPTO_DIR }} + crypto-dir: openssl_1.1 cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build @@ -138,7 +138,7 @@ jobs: - uses: ./.github/actions/prepare-build with: os: ubuntu-latest - crypto-dir: ${{ env.CRYPTO_DIR }} + crypto: matrix.crypto cache-dir: ${{ github.workspace }}/vcpkg_cache - name: Build with clang-tidy From 5c6fc43b63037508369b8ce3cfbfe477fca61a41 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Fri, 3 Nov 2023 14:19:00 -0400 Subject: [PATCH 17/17] Add status line in CMakeLists.txt --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 436f8e78..8875b9fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,7 +52,7 @@ elseif(MSVC) endif() if (SANITIZERS) - message("Enabling sanitizers") + message(STATUS "Enabling sanitizers") add_definitions(-DSANITIZERS) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU")