[hipDNN] remove spdlog & fmt logging option from plugin_sdk#4507
Conversation
Remove the dual-mode logging system from plugin_sdk, keeping only stream-style logging. This eliminates the spdlog/fmt dependencies from the plugin SDK, allowing plugins to be built without these external dependencies. Changes: - PluginLogging.hpp: Remove #ifdef HIPDNN_PLUGIN_USE_SPDLOG block, keep only stream-style macros - PluginHelpers.hpp: Remove conditional LOG_API_* macros, keep only stream-style versions - Delete CallbackSink.hpp: No longer needed without spdlog support - Simplify hipdnn_plugin_sdkConfig.cmake.in: Remove hipdnn_enable_spdlog() function and hipdnn_add_dependency_includes() helper - Spdlog.cmake: Remove HIPDNN_PLUGIN_USE_SPDLOG compile definition (only used by plugin SDK, not backend) The backend continues to use spdlog internally via cmake/Spdlog.cmake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3d76113 to
faaf2b0
Compare
PR Review SummaryOverviewThis PR completes the logging infrastructure cleanup initiated by PRs #4440 and #4442, which migrated all providers to stream-style logging. The changes are well-scoped, focused, and successfully remove all spdlog/fmt dependencies from the plugin SDK without introducing regressions. Changes ReviewedBuild System:
Configuration:
Headers:
Impact: Total reduction of approximately 311 lines of legacy logging infrastructure. Verification
RecommendationAPPROVE - This is a clean, low-risk refactoring that successfully removes technical debt and simplifies the plugin SDK logging infrastructure. Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (76.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #4507 +/- ##
========================================
Coverage 65.49% 65.49%
========================================
Files 1608 1608
Lines 256545 256545
Branches 36093 36093
========================================
Hits 168006 168006
Misses 73350 73350
Partials 15189 15189
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Enabling auto-merge |
|
Auto-merge on for real |
Upstream changes in ROCm/rocm-libraries#4507 require a few header updates.
Upstream changes in ROCm/rocm-libraries#4507 require a few header updates.
## Motivation With the removal of spdlog & fmt from the data_sdk (#4312) we moved this capability to the plugin SDK to minimize changes. #4442 & #4440 swapped existing providers to stream style logging, and the Fusilli plugin has decided to do the same. Now that no provider will depend on spdlog / fmt from plugin_sdk we are removing this option. Plugin authors can decide on their preferred logging style / macros, but the provided ones from the hipDNN SDK will be unified. ## Technical Details - Remove existing spdlog / fmt options from plugin_sdk. ## Test Plan Build and tests pass. ## Test Result Build and tests passing locally (waiting on CI). --- ### Note this is branched off the logging cleanup branch! --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Motivation With the removal of spdlog & fmt from the data_sdk (#4312) we moved this capability to the plugin SDK to minimize changes. #4442 & #4440 swapped existing providers to stream style logging, and the Fusilli plugin has decided to do the same. Now that no provider will depend on spdlog / fmt from plugin_sdk we are removing this option. Plugin authors can decide on their preferred logging style / macros, but the provided ones from the hipDNN SDK will be unified. ## Technical Details - Remove existing spdlog / fmt options from plugin_sdk. ## Test Plan Build and tests pass. ## Test Result Build and tests passing locally (waiting on CI). --- ### Note this is branched off the logging cleanup branch! --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
## Motivation With the removal of spdlog & fmt from the data_sdk (#4312) we moved this capability to the plugin SDK to minimize changes. #4442 & #4440 swapped existing providers to stream style logging, and the Fusilli plugin has decided to do the same. Now that no provider will depend on spdlog / fmt from plugin_sdk we are removing this option. Plugin authors can decide on their preferred logging style / macros, but the provided ones from the hipDNN SDK will be unified. ## Technical Details - Remove existing spdlog / fmt options from plugin_sdk. ## Test Plan Build and tests pass. ## Test Result Build and tests passing locally (waiting on CI). --- ### Note this is branched off the logging cleanup branch! --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Motivation
With the removal of spdlog & fmt from the data_sdk (#4312) we moved this capability to the plugin SDK to minimize changes.
#4442 & #4440 swapped existing providers to stream style logging, and the Fusilli plugin has decided to do the same.
Now that no provider will depend on spdlog / fmt from plugin_sdk we are removing this option.
Plugin authors can decide on their preferred logging style / macros, but the provided ones from the hipDNN SDK will be unified.
Technical Details
Test Plan
Build and tests pass.
Test Result
Build and tests passing locally (waiting on CI).
Note this is branched off the logging cleanup branch!