Fix some logger issues (#1739)

This fixes a handful of issues uncovered in downstream CI after #1722.

The following are bugs introduced in #1722:
- When using rmm from the build directory rather than an installation, the namespaced targets are not present so we must generate aliases.
- The `RMM_LOGGING_ASSERT` macro is never used in rmm itself, so we didn't catch that it was still using the old version of the logger.

While fixing the above, I also uncovered that building fmt in this environment unearths a gcc bug.

The following are underlying issues uncovered by #1722:
- spdlog's fmt CMake linkage is determined at build time. As a result, the conda package for spdlog is hardcoded to use fmt as a library (static or shared depends on what the `fmt::fmt` target winds up being when a consumer using spdlog finds fmt in CMake), which means that is propagated to all consumers of the librmm package via its CMake. This means that we often wind up with both fmt_header_only and fmt as link targets for many RAPIDS libraries. For now, this PR makes it so that if `rapids_cpm_find(spdlog)` does not find a copy of spdlog locally, the cloned version will use an external header-only fmt via rapids-cmake's logic, which ensures that packages like wheels do not export a libfmt or libspdlog dependency. However, in environments where `rapids_cpm_find(spdlog)` does find a preexisting package, we allow that package's fmt linkage to propagate. In conda environments, we know that this fmt linkage is to the library, so we keep fmt as part of rmm's runtime dependencies (by placing it in host and relying on the run export) so that libfmt is always available in environments using rmm.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #1739

CMakeLists.txt

@@ -75,6 +75,8 @@ rapids_cpm_init()
 
 add_subdirectory(rapids_logger)
 rapids_make_logger(rmm EXPORT_SET rmm-exports)
+add_library(rmm::rmm_logger ALIAS rmm_logger)
+add_library(rmm::rmm_logger_impl ALIAS rmm_logger_impl)
 
 include(cmake/thirdparty/get_cccl.cmake)
 include(cmake/thirdparty/get_nvtx.cmake)

ci/check_symbols.sh

@@ -53,7 +53,9 @@ for dso_file in ${dso_files}; do
     fi
 
     echo "checking for 'spdlog::' symbols..."
-    if grep -E 'spdlog\:\:' < "${symbol_file}"; then
+    if grep -E 'spdlog\:\:' < "${symbol_file}" \
+        | grep -v 'std\:\:_Destroy_aux'
+    then
         raise-symbols-found-error 'spdlog::'
     fi
     echo "No symbol visibility issues found"

cmake/thirdparty/get_spdlog.cmake

@@ -17,6 +17,14 @@ function(find_and_configure_spdlog)
 
   include(${rapids-cmake-dir}/cpm/spdlog.cmake)
   rapids_cpm_spdlog(
+    # The conda package for fmt is hard-coded to assume that we use a preexisting fmt library. This
+    # is why we have always had a libfmt linkage despite choosing to specify the header-only version
+    # of fmt. We need a more robust way of modifying this to support fully self-contained build and
+    # usage even in environments where fmt and/or spdlog are already present. The crudest solution
+    # would be to modify the interface compile definitions and link libraries of the spdlog target,
+    # if necessary. For now I'm specifying EXTERNAL_FMT_HO here so that in environments where spdlog
+    # is cloned and built from source we wind up with the behavior that we expect, but we'll have to
+    # resolve this properly eventually.
     FMT_OPTION "EXTERNAL_FMT_HO"
     INSTALL_EXPORT_SET rmm-exports
     BUILD_EXPORT_SET rmm-exports)

conda/environments/all_cuda-118_arch-x86_64.yaml

@@ -35,7 +35,6 @@ dependencies:
 - python>=3.10,<3.13
 - rapids-build-backend>=0.3.0,<0.4.0.dev0
 - scikit-build-core >=0.10.0
-- spdlog>=1.14.1,<1.15
 - sphinx
 - sphinx-copybutton
 - sphinx-markdown-tables

conda/environments/all_cuda-125_arch-x86_64.yaml

@@ -34,7 +34,6 @@ dependencies:
 - python>=3.10,<3.13
 - rapids-build-backend>=0.3.0,<0.4.0.dev0
 - scikit-build-core >=0.10.0
-- spdlog>=1.14.1,<1.15
 - sphinx
 - sphinx-copybutton
 - sphinx-markdown-tables

conda/recipes/librmm/meta.yaml

@@ -26,13 +26,13 @@ requirements:
     - {{ stdlib("c") }}
   host:
     - cuda-version ={{ cuda_version }}
-    # We require spdlog and fmt (which was de-vendored from spdlog
-    # conda-forge packages in 1.11.0) so that the spdlog headers are not
-    # pulled by CPM and installed as a part of the rmm packages. However,
-    # building against librmm still requires these headers. They are also
-    # added as a run requirement via the packages' run_exports.
+    # We need fmt here for now because the conda spdlog package is hard-coded
+    # to use fmt as a compiled library, not header-only, so we must ensure that
+    # the library is present for now so that if a downstream library tries to
+    # build against rmm and some other package in its build environment uses
+    # fmt (or spdlog) that the default rmm build is consistent with such
+    # environments.
     - fmt {{ fmt_version }}
-    - spdlog {{ spdlog_version }}
 
 build:
   script_env:
@@ -77,8 +77,8 @@ outputs:
         {% if cuda_major == "11" %}
         - cudatoolkit
         {% endif %}
+        # See comment about fmt in the build section above.
         - fmt {{ fmt_version }}
-        - spdlog {{ spdlog_version }}
     test:
       commands:
         - test -d "${PREFIX}/include/rmm"

dependencies.yaml

@@ -112,7 +112,6 @@ dependencies:
           - c-compiler
           - cxx-compiler
           - fmt>=11.0.2,<12
-          - spdlog>=1.14.1,<1.15
     specific:
       - output_types: conda
         matrices:

include/rmm/detail/logging_assert.hpp

@@ -38,7 +38,7 @@
     if (!success) {                                                                               \
       RMM_LOG_CRITICAL(                                                                           \
         "[" __FILE__ ":" RMM_STRINGIFY(__LINE__) "] Assertion " RMM_STRINGIFY(_expr) " failed."); \
-      rmm::detail::logger().flush();                                                              \
+      rmm::default_logger().flush();                                                              \
       /* NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) */                   \
       assert(success);                                                                            \
     }                                                                                             \

rapids_logger/logger_impl.hpp.in

@@ -26,10 +26,16 @@
 
 // Start hiding before including spdlog headers.
 #pragma GCC visibility push(hidden)
+// This issue claims to have been resolved in gcc 8, but we still seem to encounter it here.
+// The code compiles and links and all tests pass, and nm shows symbols resolved as expected.
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80947
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattributes"
 
 #include <spdlog/sinks/basic_file_sink.h>
 #include <spdlog/sinks/ostream_sink.h>
 #include <spdlog/spdlog.h>
+#pragma GCC diagnostic pop
 
 #include <memory>
 #include <sstream>