Fix absolute paths in installed CMake configs. - #482
Conversation
In microsoft/vcpkg#53159 GPT 5.6 Sol observes: > Upstream adds `${CUDAToolkit_INCLUDE_DIRS}` directly to the interface target in [`CMakeLists.txt`](https://github.com/NVIDIA/cudnn-frontend/blob/35fd7b0d0e1d4952b904c79341c5e84e3af0a328/CMakeLists.txt#L38-L47), then exports that target. The generated PR package consequently contains: > > ```cmake > INTERFACE_INCLUDE_DIRECTORIES > "C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v13.3/include; > C:/Program Files/NVIDIA GPU Computing Toolkit/CUDA/v13.3/include/cccl" > ``` This makes the resulting packages broken if the configs are used on a system where that absolute path is not the same. GPT 5.6 Sol wrote this change: Use CMake targets and then let find_dependency find the real location of those dependencies when the config gets used.
📝 WalkthroughWalkthroughCMake now discovers CUDA and cuDNN packages for both build configuration and installed package configuration. The ChangesCMake dependency wiring
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 43-48: Update the cuDNN setup before target_link_libraries by
including the project cuDNN module, then replace the undefined CuDNN::CuDNN
target with the module-provided CUDNN::cudnn target while preserving the
existing required package handling.
In `@cudnn_frontend-config.cmake.in`:
- Around line 6-8: Update the installed package discovery flow around
cudnn_frontend-config.cmake.in so consumers can resolve CUDNN without relying on
unavailable FindCUDNN.cmake or CUDNNConfig.cmake files. Include the installed
cmake/cuDNN.cmake helper or ship an equivalent discovery module, and ensure the
dependency exposes the exact CuDNN::CuDNN target expected by consumers, adapting
the existing CUDNN::cudnn or CUDNN::cudnn_all targets as needed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 35985302-7bf1-421c-8920-61b7d2cfc43e
📒 Files selected for processing (2)
CMakeLists.txtcudnn_frontend-config.cmake.in
| include(CMakeFindDependencyMacro) | ||
| find_dependency(CUDAToolkit) | ||
| find_dependency(CUDNN) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'cuDNN\.cmake|FindCUDNN|CUDNNConfig|CUDNN::cudnn|CuDNN::CuDNN|find_dependency\(CUDNN' \
--glob 'CMakeLists.txt' \
--glob '*.cmake' \
--glob '*.in' .Repository: NVIDIA/cudnn-frontend
Length of output: 9184
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- package and installation declarations ---'
rg -n -C 6 \
'configure_package_config_file|write_basic_package_version_file|install\(.*EXPORT|install\(FILES|cudnn_frontend-config|find_package\(CUDNN|find_dependency\(CUDNN|CUDNN::cudnn|CuDNN::CuDNN|CUDNNConfig|FindCUDNN|CMAKE_MODULE_PATH|CMAKE_PREFIX_PATH' \
--glob 'CMakeLists.txt' \
--glob '*.cmake' \
--glob '*.cmake.in' \
--glob '*.in' \
.
printf '%s\n' '--- relevant tracked files ---'
git ls-files | rg '(^|/)(CMakeLists\.txt|.*(cmake|CMake|config).*)$' | head -200Repository: NVIDIA/cudnn-frontend
Length of output: 12679
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- top-level CMakeLists.txt ---'
cat -n CMakeLists.txt | sed -n '1,125p'
printf '%s\n' '--- cuDNN discovery implementation ---'
cat -n cmake/cuDNN.cmake | sed -n '1,190p'
printf '%s\n' '--- all CUDNN package/target references ---'
rg -n -i \
'find_package[[:space:]]*\([[:space:]]*cudnn|find_dependency[[:space:]]*\([[:space:]]*cudnn|CUDNN::|CuDNN::|CUDNNConfig|FindCUDNN|cudnn_frontend-targets' \
--glob '!*.py' \
.Repository: NVIDIA/cudnn-frontend
Length of output: 14395
🌐 Web query:
NVIDIA cuDNN CMake package find_package CUDNN target CUDNN::cudnn CuDNN::CuDNN
💡 Result:
NVIDIA cuDNN does not provide official CMake package configuration files (such as CUDNNConfig.cmake or cudnn-config.cmake) [1]. Consequently, you cannot use find_package(CUDNN) without providing your own FindCUDNN.cmake module or using third-party package managers [2][3][1]. Because there is no standard upstream CMake support, the targets CUDNN::cudnn and CuDNN::CuDNN are not official; they are names typically defined by user-provided CMake scripts or third-party wrappers (such as vcpkg) to facilitate linking [2][4][5]. Key points for integrating cuDNN with CMake: 1. Absence of Official Config: Do not expect find_package(CUDNN) to work out of the box in a standard cuDNN installation [1]. You must define your own mechanism to locate the library and header files [6][5][3]. 2. Custom Find Modules: Projects requiring cuDNN usually include a custom FindCUDNN.cmake file in their CMAKE_MODULE_PATH [3][1]. These scripts manually search for cudnn.h and the library files (e.g., libcudnn.so or cudnn.lib) in standard locations or paths defined by environment variables like CUDNN_PATH or CUDA_PATH [6][2][5][7]. 3. Target Creation: Since the library does not provide CMake targets, these custom find modules typically create imported targets (e.g., CUDNN::cudnn or CuDNN::CuDNN) themselves using add_library(... IMPORTED) and target_link_libraries [6][2][5]. 4. Alternatives: If you are using package managers like vcpkg, they often provide a FindCUDNN.cmake that exposes these targets for you [2][4]. 5. C++ Frontend: Note that the NVIDIA cuDNN C++ Frontend is a header-only library [8][9]. It does not require linking against a binary cuDNN library to use its header-only features, though it still requires the base cuDNN library to be installed and available in your build system for the final link step [9][10][7]. If you are writing a CMake project that depends on cuDNN, you should implement a find module that searches for the library and creates the necessary interface targets to maintain consistency across your build, or leverage an existing community-maintained find module [6][2][3].
Citations:
- 1: catkin build CUDNN Problem Hiroaki-K4/tkdnn_ros#1
- 2: https://github.com/microsoft/vcpkg/blob/d592849579fb1fb22f87406b2184522ea21a8783/ports/cudnn/FindCUDNN.cmake
- 3: https://github.com/pytorch/pytorch/blob/main/cmake/Modules_CUDA_fix/FindCUDNN.cmake
- 4: https://github.com/microsoft/vcpkg/blob/502154139ddf315552a9e7dd27d8edcb5c9f7bd4/ports/cudnn/usage
- 5: https://github.com/NVIDIA/cutlass/blob/main/cuDNN.cmake
- 6: https://github.com/NVIDIA/cudnn-frontend/blob/develop/cmake/cuDNN.cmake
- 7: https://docs.nvidia.com/deeplearning/cudnn/frontend/v1.9.0/samples.html
- 8: https://docs.nvidia.com/deeplearning/cudnn/installation/latest/cpp-frontend-install.html
- 9: https://nvidia.github.io/cudnn-frontend/guides/frontend-installation/
- 10: https://docs.nvidia.com/deeplearning/cudnn/installation/latest/index.html
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
config = Path("cudnn_frontend-config.cmake.in").read_text()
top = Path("CMakeLists.txt").read_text()
helper = Path("cmake/cuDNN.cmake").read_text()
tracked = "\n".join(__import__("subprocess").check_output(["git", "ls-files"], text=True).splitlines())
checks = {
"installed config calls find_dependency(CUDNN)": bool(re.search(r"find_dependency\s*\(\s*CUDNN\s*\)", config)),
"installed config does not include the repository helper": "cmake/cuDNN.cmake" not in config,
"top-level target is CuDNN::CuDNN": "CuDNN::CuDNN" in top,
"repository helper defines CUDNN::cudnn": bool(re.search(r"add_library\s*\(\s*CUDNN::cudnn\b", helper)),
"repository helper defines CUDNN::cudnn_all": bool(re.search(r"add_library\s*\(\s*CUDNN::cudnn_all\b", helper)),
"repository has no FindCUDNN.cmake": not any(Path(p).name.lower() == "findcudnn.cmake" for p in tracked.splitlines()),
"repository has no CUDNNConfig.cmake": not any(Path(p).name.lower() == "cudnnconfig.cmake" for p in tracked.splitlines()),
"repository does not install a CUDNN discovery module/config": not bool(re.search(r"install\s*\([^)]*(?:FindCUDNN|CUDNNConfig)", top, re.I | re.S)),
}
for name, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {name}")
print("top-level link target:", re.findall(r"\b(?:CUDNN|CuDNN)::\w+", top))
print("helper target declarations:", re.findall(r"add_library\s*\(\s*((?:CUDNN|CuDNN)::\w+)", helper))
PYRepository: NVIDIA/cudnn-frontend
Length of output: 659
Provide a discoverable CUDNN package for installed consumers. The installation provides neither FindCUDNN.cmake nor CUDNNConfig.cmake, and the installed config does not include cmake/cuDNN.cmake. It also requires CuDNN::CuDNN, while that helper defines CUDNN::cudnn and CUDNN::cudnn_all. Ship a discovery module or depend on a package that defines the exact target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cudnn_frontend-config.cmake.in` around lines 6 - 8, Update the installed
package discovery flow around cudnn_frontend-config.cmake.in so consumers can
resolve CUDNN without relying on unavailable FindCUDNN.cmake or
CUDNNConfig.cmake files. Include the installed cmake/cuDNN.cmake helper or ship
an equivalent discovery module, and ensure the dependency exposes the exact
CuDNN::CuDNN target expected by consumers, adapting the existing CUDNN::cudnn or
CUDNN::cudnn_all targets as needed.
|
Based on the CodeRabbit comments here and given that this issue predates the update I went ahead and merged microsoft/vcpkg#53159 without this fixed for now. It's not clear to me how you want the cudNN upstream dependency expressed to callers; I assume you don't want to merge this as-is as it depends on a vcpkg-ism. Sorry for the noise! (In particular cudnn is unconditionally included and it isn't clear to me how downstream CMake customers are conventionally expected to wire that up given that the existing cudnn-frontend targets don't appear to be doing: cudnn-frontend/include/cudnn_frontend.h Line 89 in ac9356f ) |
Anerudhan
added
mod-frontend
|
@cudnn-ci-bot run |
1 similar comment
|
@cudnn-ci-bot run |
|
🚀 Running mirror pipeline Branch: cudnn-gh/pr-482-224f31d |
|
Running with this gives for linux builds. Similar error for windows as well |
|
@Anerudhan Sorry for the confusion; this is what I meant by "depends on a vcpkgism" ; it works for us because we install https://github.com/microsoft/vcpkg/blob/master/ports/cudnn/FindCUDNN.cmake . I submitted this PR based on a patch that we would have added in vcpkg but it's not clear to me how callers are expected to get |
3 participants
Before submitting
pre-commit runand committed any formatting changes.As there are no code changes here I hope there is no formatting that would emit anyways.
Affected area
Summary
In microsoft/vcpkg#53159 GPT 5.6 Sol observes:
This makes the resulting packages broken if the configs are used on a system where that absolute path is not the same.
GPT 5.6 Sol wrote this change: Use CMake targets and then let find_dependency find the real location of those dependencies when the config gets used.
Why
(See "Summary")
Related issues
microsoft/vcpkg#53159
API and compatibility impact
None expected. The CUDAToolkin module was added in CMake 3.17: https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html
Testing
None yet. (I wanted to make the upstream submission before making the PR against microsoft/vcpkg#53159)
Summary by CodeRabbit