Use -fvisibility=hidden for spdlog (#5427)

This is a branch coded up by @teo-tsirpanis . I'm putting up a PR for it, at his expicit request. The problem to be solved is intermittent errors in [TileDB-SOMA](https://github.com/single-cell-data/TileDB-SOMA), which uses this repo as a dependency. For dev work, on a Mac, I build both TileDB Core and TileDB-SOMA from source. The intersection of those three things makes me, perhaps, unusual. Regardless, I have (until this PR) codepaths which, quite simply, always segfault. The segfaulting patterns involve innocuous logging statements. The stack traces are of this form: https://gist.github.com/johnkerl/b1e07a70906177f498f6e9efa2979376 Notes: * Note the arrowed lines * A variable in one frame called `formatted` is being passed to another with argument name `dest` * In `lldb`, `p sizeof(formatted)` in the upper frame shows 24 while `p sizeof(dest)` in the lower frame shows 288 * This is across the boundary from `libtiledb.dylib spdlog::logger` to `libtiledbsoma.dylib spdlog::logger`, and back -- so it's an ABI issue apparently. * I compiled/reran with 1.14.1 for soma & core `spdlog` both, yet the issue persists I also tried `-fvisibility=hidden` within TileDB-SOMA's copy of `spdlog` and was not able to resolve the problem. Meanwhile @teo-tsirpanis's solution here does resolve the problem. [[sc-62143]](https://app.shortcut.com/tiledb-inc/story/62143/tiledb-soma-segfaults-only-on-my-mac-tracking#activity-62145) --- TYPE: IMPROVEMENT DESC: Use `-fvisibility=hidden` for `spdlog` --------- Co-authored-by: Theodore Tsirpanis <[email protected]>
TileDB-Inc · Jan 17, 2025 · ba3aed3 · ba3aed3
1 parent 855e8ee
commit ba3aed3
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 0 deletions.
diff --git a/ports/README.md b/ports/README.md
@@ -36,3 +36,4 @@ For ease of review when patching existing ports, you are recommended to make one
 | `aws-sdk-cpp` | Patching to fix MinGW build failures, and to avoid building test-only SDKs (https://github.com/aws/aws-sdk-cpp/pull/3061). |
 | `libmagic`    | Using a custom CMake-based port that is not accepted upstream.                                                             |
 | `libfaketime` | Port does not yet exist upstream                                                                                           |
+| `spdlog`      | Patching to compile with `-fvisibility=hidden`                                                                             |
diff --git a/ports/spdlog/portfile.cmake b/ports/spdlog/portfile.cmake
@@ -0,0 +1,67 @@
+vcpkg_from_github(
+    OUT_SOURCE_PATH SOURCE_PATH
+    REPO gabime/spdlog
+    REF "v${VERSION}"
+    SHA512 d8f36a3d65a43d8c64900e46137827aadb05559948b2f5a389bea16ed1bfac07d113ee11cf47970913298d6c37400355fe6895cda8fa6dcf6abd9da0d8f199e9
+    HEAD_REF v1.x
+)
+
+vcpkg_check_features(OUT_FEATURE_OPTIONS FEATURE_OPTIONS
+    FEATURES
+        benchmark SPDLOG_BUILD_BENCH
+        wchar     SPDLOG_WCHAR_SUPPORT
+)
+
+# SPDLOG_WCHAR_FILENAMES can only be configured in triplet file since it is an alternative (not additive)
+if(NOT DEFINED SPDLOG_WCHAR_FILENAMES)
+    set(SPDLOG_WCHAR_FILENAMES OFF)
+endif()
+if(NOT VCPKG_TARGET_IS_WINDOWS AND SPDLOG_WCHAR_FILENAMES)
+    message(FATAL_ERROR "Build option 'SPDLOG_WCHAR_FILENAMES' is for Windows.")
+endif()
+
+string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" SPDLOG_BUILD_SHARED)
+
+vcpkg_cmake_configure(
+    SOURCE_PATH "${SOURCE_PATH}"
+    OPTIONS
+        ${FEATURE_OPTIONS}
+        -DCMAKE_CXX_VISIBILITY_PRESET=hidden
+        -DSPDLOG_FMT_EXTERNAL=ON
+        -DSPDLOG_INSTALL=ON
+        -DSPDLOG_BUILD_SHARED=${SPDLOG_BUILD_SHARED}
+        -DSPDLOG_WCHAR_FILENAMES=${SPDLOG_WCHAR_FILENAMES}
+        -DSPDLOG_BUILD_EXAMPLE=OFF
+)
+
+vcpkg_cmake_install()
+vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake/spdlog)
+vcpkg_fixup_pkgconfig()
+vcpkg_copy_pdbs()
+
+# add support for integration other than cmake
+vcpkg_replace_string(${CURRENT_PACKAGES_DIR}/include/spdlog/tweakme.h
+    "// #define SPDLOG_FMT_EXTERNAL"
+    "#ifndef SPDLOG_FMT_EXTERNAL\n#define SPDLOG_FMT_EXTERNAL\n#endif"
+)
+if(SPDLOG_WCHAR_SUPPORT)
+    vcpkg_replace_string(${CURRENT_PACKAGES_DIR}/include/spdlog/tweakme.h
+        "// #define SPDLOG_WCHAR_TO_UTF8_SUPPORT"
+        "#ifndef SPDLOG_WCHAR_TO_UTF8_SUPPORT\n#define SPDLOG_WCHAR_TO_UTF8_SUPPORT\n#endif"
+    )
+endif()
+if(SPDLOG_WCHAR_FILENAMES)
+    vcpkg_replace_string(${CURRENT_PACKAGES_DIR}/include/spdlog/tweakme.h
+        "// #define SPDLOG_WCHAR_FILENAMES"
+        "#ifndef SPDLOG_WCHAR_FILENAMES\n#define SPDLOG_WCHAR_FILENAMES\n#endif"
+    )
+endif()
+
+file(REMOVE_RECURSE
+    "${CURRENT_PACKAGES_DIR}/include/spdlog/fmt/bundled"
+    "${CURRENT_PACKAGES_DIR}/debug/include"
+    "${CURRENT_PACKAGES_DIR}/debug/share"
+)
+
+file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/usage" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")
+vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")
diff --git a/ports/spdlog/usage b/ports/spdlog/usage
@@ -0,0 +1,8 @@
+The package spdlog provides CMake targets:
+
+    find_package(spdlog CONFIG REQUIRED)
+    target_link_libraries(main PRIVATE spdlog::spdlog)
+
+    # Or use the header-only version
+    find_package(spdlog CONFIG REQUIRED)
+    target_link_libraries(main PRIVATE spdlog::spdlog_header_only)
diff --git a/ports/spdlog/vcpkg.json b/ports/spdlog/vcpkg.json
@@ -0,0 +1,30 @@
+{
+  "name": "spdlog",
+  "version-semver": "1.14.1",
+  "description": "Very fast, header-only/compiled, C++ logging library.",
+  "homepage": "https://github.com/gabime/spdlog",
+  "license": "MIT",
+  "dependencies": [
+    "fmt",
+    {
+      "name": "vcpkg-cmake",
+      "host": true
+    },
+    {
+      "name": "vcpkg-cmake-config",
+      "host": true
+    }
+  ],
+  "features": {
+    "benchmark": {
+      "description": "Use google benchmark",
+      "dependencies": [
+        "benchmark"
+      ]
+    },
+    "wchar": {
+      "description": "Build with wchar_t (Windows only)",
+      "supports": "windows"
+    }
+  }
+}