Try building FFI in CI, including Windows

.github/actions/cache/action.yaml

@@ -5,16 +5,19 @@ inputs:
   rust_version:
     default: "latest"
     required: false
+  build_profile:
+    default: "unspecified"
+    required: false
 
 runs:
   using: composite
   steps:
     - name: ⚡ Cache
-      uses: actions/cache@v2
+      uses: actions/cache@v3
       with:
         path: |
           ~/.cargo/registry/
           ~/.cargo/git/db/
           ~/.cargo/bin/
           target/
-        key: v1-${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ inputs.rust_version }}
+        key: v1-${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ inputs.rust_version }}-${{ inputs.build_profile }}

.github/workflows/test.yml

@@ -2,18 +2,19 @@ name: Test
 on: [push]
 env: 
   CARGO_TERM_COLOR: always
+  RUST_VERSION: 1.60.0
 
 jobs:
   test:
     name: "cargo test --workspace #${{ matrix.platform }} ${{ matrix.rust_version }}"
     runs-on: ${{ matrix.platform }}
     strategy:
       matrix:
-        platform: [windows-latest, ubuntu-latest, macos-latest] 
+        platform: [windows-latest, ubuntu-latest, macos-latest]
         rust_version: [""]
         include:
           - platform: "ubuntu-latest"
-            rust_version: "1.60"
+            rust_version: "${RUST_VERSION}"
     steps:
       - name: Checkout sources
         uses: actions/checkout@v2
@@ -30,3 +31,63 @@ jobs:
         run: cargo build --workspace --verbose
       - name: "[${{ steps.rust-version.outputs.version}}] cargo test --workspace --verbose"
         run: cargo test --workspace --verbose
+
+  ffi:
+    name: "FFI #${{ matrix.platform }} ${{ matrix.rust_version }}"
+    runs-on: ${{ matrix.platform }}
+    strategy:
+      matrix:
+        platform: [windows-latest, ubuntu-latest, macos-latest]
+        rust_version: [""]
+        include:
+          - platform: "ubuntu-latest"
+            rust_version: "${RUST_VERSION}"
+    steps:
+
+      - name: Checkout sources
+        uses: actions/checkout@v2
+
+      - name: Setup output dir
+        shell: bash
+        run: |
+          WORKSPACE_PATH=${{ github.workspace }}
+          if [[ "${{ matrix.platform }}" == "windows-latest" ]]; then
+             WORKSPACE_PATH=$(cygpath -ua '${{ github.workspace }}')
+          fi
+          echo "OUTPUT_FOLDER=$WORKSPACE_PATH/artifacts" >> $GITHUB_ENV
+
+      - name: Cache
+        uses: ./.github/actions/cache
+        with:
+          rust_version: ${{ matrix.rust_version }}
+          build_profile: "release"
+
+      - name: Install Rust ${{ matrix.rust_version }}
+        if: ${{ matrix.rust_version != '' }}
+        run: rustup install ${{ matrix.rust_version }} && rustup default ${{ matrix.rust_version }}
+
+      - id: rust-version
+        run: "echo ::set-output name=version::$(rustc --version)"
+
+      - name: "Generate FFI"
+        shell: bash
+        run: |
+           chmod +x ffi-build.sh
+           ./ffi-build.sh ${OUTPUT_FOLDER}
+
+      - name: 'Publish libdatadog'
+        uses: actions/upload-artifact@v2
+        if: '${{ always() }}'
+        with:
+          if-no-files-found: error
+          name: libdatadog.${{ matrix.platform }}
+          path: ${{ github.workspace }}/artifacts
+          retention-days: 1
+
+      - name: "Test building C bindings"
+        shell: bash
+        run: |
+          mkdir examples/ffi/build
+          cd examples/ffi/build
+          cmake -S .. -DDDProf_ROOT=$OUTPUT_FOLDER
+          cmake --build .

ddprof-ffi/cbindgen.toml

@@ -14,8 +14,24 @@ sys_includes = ["stdbool.h", "stddef.h", "stdint.h"]
 
 after_includes = """
 
+#if defined(_MSC_VER)
 #define DDPROF_FFI_CHARSLICE_C(string) \\
-  /* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice) {.ptr = "" string, .len = sizeof(string) - 1})"""
+/* NOTE: Compilation fails if you pass in a char* instead of a literal */ {.ptr = "" string, .len = sizeof(string) - 1}
+#else
+#define DDPROF_FFI_CHARSLICE_C(string) \\
+/* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice){ .ptr = "" string, .len = sizeof(string) - 1 })
+#endif
+
+#if defined(__cplusplus) && (__cplusplus >= 201703L)
+#  define DD_CHECK_RETURN [[nodiscard]]
+#elif defined(_Check_return_) /* SAL */
+#  define DD_CHECK_RETURN _Check_return_
+#elif (defined(__has_attribute) && __has_attribute(warn_unused_result)) || \\
+      (defined(__GNUC__) && (__GNUC__ >= 4))
+#  define DD_CHECK_RETURN __attribute__((__warn_unused_result__))
+#else
+#  define DD_CHECK_RETURN
+#endif"""
 
 [export]
 prefix = "ddprof_ffi_"
@@ -28,7 +44,7 @@ prefix_with_name = true
 rename_variants = "ScreamingSnakeCase"
 
 [fn]
-must_use = "__attribute__((warn_unused_result))"
+must_use = "DD_CHECK_RETURN"
 
 [parse]
 parse_deps = true

examples/ffi/CMakeLists.txt

@@ -4,7 +4,22 @@ cmake_minimum_required(VERSION 3.19)
 find_package(DDProf)
 
 add_executable(exporter exporter.cpp)
-target_compile_features(exporter PRIVATE cxx_std_11)
-target_link_libraries(exporter DDProf::FFI)
+
 add_executable(profiles profiles.c)
-target_link_libraries(profiles DDProf::FFI)
+
+set(EXTERNAL_LIBS DDProf::FFI)
+
+if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
+    set(EXTERNAL_LIBS "${EXTERNAL_LIBS};NtDll;UserEnv;Bcrypt;crypt32;wsock32;ws2_32;shlwapi")
+endif()
+
+if (CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
+    target_compile_features(exporter PRIVATE cxx_std_20) # needed for designated initializers
+    target_compile_definitions(exporter PUBLIC _CRT_SECURE_NO_WARNINGS)
+else()
+    target_compile_features(exporter PRIVATE cxx_std_11)
+endif()
+
+target_link_libraries(exporter ${EXTERNAL_LIBS})
+
+target_link_libraries(profiles ${EXTERNAL_LIBS})

examples/ffi/exporter.cpp

@@ -9,7 +9,7 @@ extern "C" {
 #include <thread>
 
 static ddprof_ffi_Slice_c_char to_slice_c_char(const char *s) {
-  return (ddprof_ffi_Slice_c_char){.ptr = s, .len = strlen(s)};
+  return {.ptr = s, .len = strlen(s)};
 }
 
 struct Deleter {

examples/ffi/profiles.c

@@ -31,7 +31,7 @@ int main(void) {
 
   struct ddprof_ffi_Location root_location = {
       // yes, a zero-initialized mapping is valid
-      .mapping = (struct ddprof_ffi_Mapping){},
+      .mapping = (struct ddprof_ffi_Mapping){0},
       .lines = (struct ddprof_ffi_Slice_line){&root_line, 1},
   };
   int64_t value = 10;

ffi-build.sh

@@ -13,6 +13,8 @@ mkdir -v -p "$destdir/include/ddprof" "$destdir/lib/pkgconfig" "$destdir/cmake"
 version=$(awk -F\" '$1 ~ /^version/ { print $2 }' < ddprof-ffi/Cargo.toml)
 target="$(rustc -vV | awk '/^host:/ { print $2 }')"
 shared_library_suffix=".so"
+static_library_suffix=".a"
+library_prefix="lib"
 remove_rpath=0
 fix_macos_rpath=0
 
@@ -32,17 +34,28 @@ case "$target" in
         # on alpine musl, Rust adds some weird runpath to cdylibs
         remove_rpath=1
         ;;
+
     "x86_64-apple-darwin")
         expected_native_static_libs=" -framework Security -framework CoreFoundation -liconv -lSystem -lresolv -lc -lm -liconv"
         native_static_libs="${expected_native_static_libs}"
         shared_library_suffix=".dylib"
         # fix usage of library in macos via rpath
         fix_macos_rpath=1
         ;;
+
     "x86_64-unknown-linux-gnu"|"aarch64-unknown-linux-gnu")
         expected_native_static_libs=" -ldl -lrt -lpthread -lgcc_s -lc -lm -lrt -lpthread -lutil -ldl -lutil"
         native_static_libs=" -ldl -lrt -lpthread -lc -lm -lrt -lpthread -lutil -ldl -lutil"
         ;;
+
+    "x86_64-pc-windows-msvc")
+        expected_native_static_libs="" # I don't know what to expect
+        native_static_libs="" # I don't know what to expect
+        shared_library_suffix=".dll"
+        static_library_suffix=".lib"
+        library_prefix=""
+        ;;
+
     *)
         >&2 echo "Unknown platform '${target}'"
         exit 1
@@ -71,28 +84,31 @@ cp -v LICENSE LICENSE-3rdparty.yml NOTICE "$destdir/"
 
 export RUSTFLAGS="${RUSTFLAGS:- -C relocation-model=pic}"
 
-echo "Building the libddprof_ffi library (may take some time)..."
+echo "Building the ddprof_ffi library (may take some time)..."
 cargo build --release --target "${target}"
-cp -v "target/${target}/release/libddprof_ffi.a" "target/${target}/release/libddprof_ffi${shared_library_suffix}" "$destdir/lib/"
+
+shared_library_name="${library_prefix}ddprof_ffi${shared_library_suffix}"
+static_library_name="${library_prefix}ddprof_ffi${static_library_suffix}"
+cp -v "target/${target}/release/$static_library_name" "target/${target}/release/$shared_library_name" "$destdir/lib/"
 
 if [[ "$remove_rpath" -eq 1 ]]; then
-    patchelf --remove-rpath "$destdir/lib/libddprof_ffi${shared_library_suffix}"
+    patchelf --remove-rpath "$destdir/lib/${shared_library_name}"
 fi
 
 if [[ "$fix_macos_rpath" -eq 1 ]]; then
-    install_name_tool -id @rpath/libddprof_ffi${shared_library_suffix} "$destdir/lib/libddprof_ffi${shared_library_suffix}"
+    install_name_tool -id @rpath/${shared_library_name} "$destdir/lib/${shared_library_name}"
 fi
 
 # objcopy might not be available on macOS
-if command -v objcopy > /dev/null; then
+if command -v objcopy > /dev/null && [[ "$target" != "x86_64-pc-windows-msvc" ]]; then
     # Remove .llvmbc section which is not useful for clients
-    objcopy --remove-section .llvmbc "$destdir/lib/libddprof_ffi.a"
+    objcopy --remove-section .llvmbc "$destdir/lib/${static_library_name}"
 
     # Ship debug information separate from shared library, so that downstream packages can selectively include it
     # https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html
-    objcopy --only-keep-debug "$destdir/lib/libddprof_ffi${shared_library_suffix}" "$destdir/lib/libddprof_ffi${shared_library_suffix}.debug"
-    strip -S "$destdir/lib/libddprof_ffi${shared_library_suffix}"
-    objcopy --add-gnu-debuglink="$destdir/lib/libddprof_ffi${shared_library_suffix}.debug" "$destdir/lib/libddprof_ffi${shared_library_suffix}"
+    objcopy --only-keep-debug "$destdir/lib/$shared_library_name" "$destdir/lib/$shared_library_name.debug"
+    strip -S "$destdir/lib/$shared_library_name"
+    objcopy --add-gnu-debuglink="$destdir/lib/$shared_library_name.debug" "$destdir/lib/$shared_library_name"
 fi
 
 echo "Checking that native-static-libs are as expected for this platform..."
@@ -118,7 +134,7 @@ if [ -n "$unexpected_native_libs" ]; then
 fi
 cd -
 
-echo "Generating the ddprof/ffi.h header..."
+echo "Generating the $destdir/include/ddprof/ffi.h header..."
 cbindgen --crate ddprof-ffi --config ddprof-ffi/cbindgen.toml --output "$destdir/include/ddprof/ffi.h"
 
 # CI doesn't have any clang tooling