rm CI dep on rustls-ffi-test for artifact test

Previously the artifacts.yaml CI job relied on an external repo that had a simple C/CMake application depending on librustls. When verifying artifacts we cloned this repo and tested a build against the artifact. This commit reworks the main repo's test application code/build to support this use-case. The main tweak required is to support _not_ building librustls from src, but instead finding it with pkg-config. We gate this behind a new `FORCE_SYSTEM_RUSTLS` option. The artifact test CI is updated to use this option. Annoyingly we have to patch the .pc prefix to an absolute dir to get this working nicely, which means handling Windows specially. Similarly, for yet to be determined reasons, on Win32 we have to add some extra target_link_libraries that don't propagate automatically through the pc config. For this we just shift the existing Win32 specific target_link_libraries invocation up and use it for both pkg-config librustls and build-it-ourselves librustls.
rustls · Feb 10, 2025 · ff72516 · ff72516
1 parent 2f0599f
commit ff72516
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 89 deletions.
diff --git a/.github/workflows/artifacts.yaml b/.github/workflows/artifacts.yaml
@@ -175,10 +175,10 @@ jobs:
           - os: macos-13
             artifact: rustls-ffi-x86_64-macos
     steps:
-      - name: Checkout rustls-ffi-test sources
+      - name: Checkout sources
         uses: actions/checkout@v4
         with:
-          repository: 'cpu/rustls-ffi-test'
+          persist-credentials: false
       - name: Download rustls-ffi artifact
         uses: actions/download-artifact@v4
         with:
@@ -194,19 +194,29 @@ jobs:
       # the correct location that we extracted the archive. This seems more reliable
       # than using `--define-prefix` - it seems to tack an extra 'lib/' subcomponent
       # onto the include path that breaks the build.
-      - name: Fix pkg-config prefix
-        # We use bash shell explicitly to avoid PowerShell on Windows and to ensure we have 'sed'.
+      - name: Fix pkg-config prefix (UNIX)
+        if: matrix.os != 'windows-latest'
         shell: bash
         # For further fun, sed isn't consistent between macOS and Linux.
         run: |
           case "${{ runner.os }}" in
             "macOS")
-              sed -i '' "s|prefix=.*|prefix=${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc
+              sed -i '' "s|prefix=.*|prefix=$(pwd)/${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc
               ;;
             *)
-              sed -i "s|prefix=.*|prefix=${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc
+              sed -i "s|prefix=.*|prefix=$(pwd)/${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc
               ;;
           esac
+      - name: Fix pkg-config prefix (Windows)
+        if: matrix.os == 'windows-latest'
+        shell: pwsh
+        run: |
+          $prefix = (Get-Location).Path + "/${{ matrix.artifact }}"
+          $prefix = $prefix -replace '\\', '/'
+          
+          $content = Get-Content "${{ matrix.artifact }}\lib\pkgconfig\rustls.pc"
+          $content = $content -replace "prefix=.*", "prefix=$prefix"
+          Set-Content "${{ matrix.artifact }}\lib\pkgconfig\rustls.pc" $content
       # Dump out what pkg-config says about the rustls package.
       - name: Debug pkg-config
         run: |
@@ -220,7 +230,7 @@ jobs:
         if: matrix.os != 'windows-latest'
         env:
           PKG_CONFIG_PATH: ${{ matrix.artifact }}/lib/pkgconfig
-        run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release
+        run: cmake -S librustls -B build -DCMAKE_BUILD_TYPE=Release -DFORCE_SYSTEM_RUSTLS=ON
       # Set up the cmake build, overriding PKG_CONFIG_PATH to
       # point to the extracted rustls-ffi archive.
       #
@@ -230,26 +240,30 @@ jobs:
         if: matrix.os == 'windows-latest'
         env:
           PKG_CONFIG_PATH: ${{ matrix.artifact }}/lib/pkgconfig
-        run: cmake -DPKG_CONFIG_EXECUTABLE=C:\Strawberry\perl\bin\pkg-config.bat -S . -B build
-      # Build the rustls-ffi-test binary.
-      - name: Build rustls-ffi-test (UNIX)
+        run: cmake -DPKG_CONFIG_EXECUTABLE=C:\Strawberry\perl\bin\pkg-config.bat -DFORCE_SYSTEM_RUSTLS=ON -S librustls -B build
+      # Build the client and server binaries
+      - name: Build rustls-ffi client/server (UNIX)
         if: matrix.os != 'windows-latest'
         run: cmake --build build -v
-      # Build the rustls-ffi-test binary.
+      # Build the client and server binaries
       # On Windows we need to specify a configuration to avoid a warning about using the default
       # debug MSCRT runtime with a lib built with the release MSCRT runtime.
-      - name: Build rustls-ffi-test (Windows)
+      - name: Build rustls-ffi client/server (Windows)
         if: matrix.os == 'windows-latest'
         run: cmake --build build --config Release -v
-      # Run the rustls-ffi-test binary.
-      - name: Run rustls-ffi-test (UNIX)
+      # Run the rustls-ffi client binary.
+      - name: Run rustls-ffi client (UNIX)
         if: matrix.os != 'windows-latest'
-        run: ./build/rustls-ffi-test
+        env:
+          RUSTLS_PLATFORM_VERIFIER: 1
+        run: ./build/tests/client example.com 443 / 1
       # Run the rustls-ffi-test binary.
       # On Windows it's in a different output location under build.
-      - name: Run rustls-ffi-test (Windows)
+      - name: Run rustls-ffi client (Windows)
         if: matrix.os == 'windows-latest'
-        run: ./build/Release/rustls-ffi-test.exe
+        env:
+          RUSTLS_PLATFORM_VERIFIER: 1
+        run: .\build\tests\Release\client.exe example.com 443 / 1
 
   test-deb:
     name: "Test Linux Deb (${{ matrix.os }})"
@@ -259,10 +273,10 @@ jobs:
       matrix:
         os: [ ubuntu-latest, ubuntu-20.04 ]
     steps:
-      - name: Checkout rustls-ffi-test sources
+      - name: Checkout sources
         uses: actions/checkout@v4
         with:
-          repository: 'cpu/rustls-ffi-test'
+          persist-credentials: false
       - name: Download rustls-ffi deb artifact
         uses: actions/download-artifact@v4
         with:
@@ -278,10 +292,12 @@ jobs:
           pkg-config --libs rustls
       # Set up the cmake build, no pkg-config ENV overrides needed.
       - name: Setup cmake build
-        run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release
-      # Build the rustls-ffi-test binary.
-      - name: Build rustls-ffi-test
+        run: cmake -S librustls -B build -DCMAKE_BUILD_TYPE=Release -DFORCE_SYSTEM_RUSTLS=ON
+      # Build the client and server binaries
+      - name: Build rustls-ffi client/server
         run: cmake --build build -v
-      # Run the rustls-ffi-test binary.
-      - name: Run rustls-ffi-test
-        run: ./build/rustls-ffi-test
+      # Run the rustls-ffi client binary.
+      - name: Run rustls-ffi client
+        env:
+          RUSTLS_PLATFORM_VERIFIER: 1
+        run: ./build/tests/client example.com 443 / 1
diff --git a/librustls/cmake/options.cmake b/librustls/cmake/options.cmake
@@ -76,3 +76,10 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
         FORCE
     )
 endif()
+
+# Useful for testing the client/server examples with a pre-built rustls-ffi.
+option(
+    FORCE_SYSTEM_RUSTLS
+    "Require system-installed rustls-ffi, never build"
+    OFF
+)
diff --git a/librustls/cmake/rust.cmake b/librustls/cmake/rust.cmake
@@ -1,27 +1,41 @@
 include(ExternalProject)
 set_directory_properties(PROPERTIES EP_PREFIX ${CMAKE_BINARY_DIR}/rust)
 
-ExternalProject_Add(
-    rustls-ffi
-    DOWNLOAD_COMMAND ""
-    CONFIGURE_COMMAND ""
-    BUILD_COMMAND
-        cargo capi build --locked ${CARGO_FEATURES}
-        "$<IF:$<CONFIG:Release>,--release,-->"
-    # Rely on cargo checking timestamps, rather than tell CMake where every
-    # output is.
-    BUILD_ALWAYS true
-    INSTALL_COMMAND
-        cargo capi install --libdir=lib --prefix=${CMAKE_BINARY_DIR}/rust
-        --locked ${CARGO_FEATURES} "$<IF:$<CONFIG:Release>,--release,--debug>"
-    # Run cargo test with --quiet because msbuild will treat the presence
-    # of "error" in stdout as an error, and we have some test functions that
-    # end in "_error". Quiet mode suppresses test names, so this is a
-    # sufficient workaround.
-    TEST_COMMAND
-        cargo test --locked ${CARGO_FEATURES}
-        "$<IF:$<CONFIG:Release>,--release,-->" --quiet
-)
+if(FORCE_SYSTEM_RUSTLS)
+    find_package(PkgConfig REQUIRED)
+    pkg_check_modules(RUSTLS_FFI REQUIRED rustls)
+
+    if(NOT RUSTLS_FFI_FOUND)
+        message(FATAL_ERROR "System rustls-ffi required but not found")
+    endif()
+
+    message(STATUS "RUSTLS_FFI_INCLUDE_DIRS: ${RUSTLS_FFI_INCLUDE_DIRS}")
+    message(STATUS "RUSTLS_FFI_LIBRARY_DIRS: ${RUSTLS_FFI_LIBRARY_DIRS}")
+    message(STATUS "RUSTLS_FFI_LIBRARIES: ${RUSTLS_FFI_LIBRARIES}")
+else()
+    ExternalProject_Add(
+        rustls-ffi
+        DOWNLOAD_COMMAND ""
+        CONFIGURE_COMMAND ""
+        BUILD_COMMAND
+            cargo capi build --locked ${CARGO_FEATURES}
+            "$<IF:$<CONFIG:Release>,--release,-->"
+        # Rely on cargo checking timestamps, rather than tell CMake where every
+        # output is.
+        BUILD_ALWAYS true
+        INSTALL_COMMAND
+            cargo capi install --libdir=lib --prefix=${CMAKE_BINARY_DIR}/rust
+            --locked ${CARGO_FEATURES}
+            "$<IF:$<CONFIG:Release>,--release,--debug>"
+        # Run cargo test with --quiet because msbuild will treat the presence
+        # of "error" in stdout as an error, and we have some test functions that
+        # end in "_error". Quiet mode suppresses test names, so this is a
+        # sufficient workaround.
+        TEST_COMMAND
+            cargo test --locked ${CARGO_FEATURES}
+            "$<IF:$<CONFIG:Release>,--release,-->" --quiet
+    )
+endif()
 
 add_custom_target(
     cbindgen

diff --git a/librustls/tests/CMakeLists.txt b/librustls/tests/CMakeLists.txt
@@ -46,62 +46,79 @@ endif()
 function(test_binary target_name)
     add_executable(${target_name})
     target_sources(${target_name} PRIVATE ${target_name}.c common.c common.h)
-    add_dependencies(${target_name} rustls-ffi)
-
-    target_include_directories(
-        ${target_name}
-        PRIVATE ${CMAKE_BINARY_DIR}/rust/include
-    )
 
     if(WIN32)
-        target_compile_options(${target_name} PRIVATE ${sanitizer_flags})
         target_link_libraries(
             ${target_name}
-            "${CMAKE_BINARY_DIR}/rust/lib/rustls.${lib_extension}"
+            PRIVATE
+                advapi32.lib
+                bcrypt.lib
+                crypt32.lib
+                cryptnet.lib
+                kernel32.lib
+                ncrypt.lib
+                bcrypt.lib
+                advapi32.lib
+                legacy_stdio_definitions.lib
+                kernel32.lib
+                advapi32.lib
+                kernel32.lib
+                ntdll.lib
+                userenv.lib
+                ws2_32.lib
+                synchronization.lib
+                kernel32.lib
+                ws2_32.lib
+                kernel32.lib
+                msvcrt.lib
         )
-        target_link_libraries(
+    endif()
+
+    if(RUSTLS_FFI_FOUND)
+        target_include_directories(
             ${target_name}
-            advapi32.lib
-            bcrypt.lib
-            crypt32.lib
-            cryptnet.lib
-            kernel32.lib
-            ncrypt.lib
-            bcrypt.lib
-            advapi32.lib
-            legacy_stdio_definitions.lib
-            kernel32.lib
-            advapi32.lib
-            kernel32.lib
-            ntdll.lib
-            userenv.lib
-            ws2_32.lib
-            synchronization.lib
-            kernel32.lib
-            ws2_32.lib
-            kernel32.lib
-            msvcrt.lib
+            PRIVATE ${RUSTLS_FFI_INCLUDE_DIRS}
         )
-        set_property(
-            TARGET ${target_name}
-            PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreadedDLL"
+        target_link_directories(
+            ${target_name}
+            PRIVATE ${RUSTLS_FFI_LIBRARY_DIRS}
         )
-    elseif(UNIX)
-        target_compile_options(${target_name} PRIVATE ${sanitizer_flags})
-        target_link_options(${target_name} PRIVATE ${sanitizer_flags})
-        target_link_libraries(
+        target_link_libraries(${target_name} PRIVATE ${RUSTLS_FFI_LIBRARIES})
+    else()
+        add_dependencies(${target_name} rustls-ffi)
+
+        target_include_directories(
             ${target_name}
-            "${CMAKE_BINARY_DIR}/rust/lib/librustls.${lib_extension}"
+            PRIVATE ${CMAKE_BINARY_DIR}/rust/include
         )
-        if(CERT_COMPRESSION)
-            target_link_libraries(${target_name} m)
-        endif()
-        if(APPLE)
+
+        if(WIN32)
+            target_compile_options(${target_name} PRIVATE ${sanitizer_flags})
+            target_link_libraries(
+                ${target_name}
+                PRIVATE "${CMAKE_BINARY_DIR}/rust/lib/rustls.${lib_extension}"
+            )
+            set_property(
+                TARGET ${target_name}
+                PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreadedDLL"
+            )
+        elseif(UNIX)
+            target_compile_options(${target_name} PRIVATE ${sanitizer_flags})
+            target_link_options(${target_name} PRIVATE ${sanitizer_flags})
             target_link_libraries(
                 ${target_name}
-                "-framework Foundation"
-                "-framework Security"
+                "${CMAKE_BINARY_DIR}/rust/lib/librustls.${lib_extension}"
             )
+            if(CERT_COMPRESSION)
+                target_link_libraries(${target_name} m)
+            endif()
+            if(APPLE)
+                target_link_libraries(
+                    ${target_name}
+                    "-framework Foundation"
+                    "-framework Security"
+                )
+            endif()
         endif()
     endif()
 endfunction()