Resolve third-party dependencies from the build environment - #12331
Draft
BrianHarrisonAMD wants to merge 7 commits into
Draft
BrianHarrisonAMD wants to merge 7 commits into
BrianHarrisonAMD wants to merge 7 commits into
Conversation
github-actions
Bot
added
documentation
project: hipdnn
project: hip-kernel-provider
labels
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
TheRock owns third-party dependency acquisition and prescribes find_package as the way a sub-project depends on one. hipDNN and the dnn-providers instead fell back to FetchContent whenever a lookup missed, and the example engine plugin's tests fetched GoogleTest unconditionally. ALLOW_FETCH_DEPS replaces HIPDNN_NO_DOWNLOAD and defaults OFF. The old option was inverted -- enabling it forced FETCHCONTENT_FULLY_DISCONNECTED off -- so the only switch for suppressing downloads did nothing. The new option is shared with dnn-providers, which had none, and a miss now fails configuration naming the package and the remedy. The superbuild sets it ON: it mirrors a developer environment and already supplies dependencies by fetching them. HIPDNN_NO_DOWNLOAD works for one more release; only a truthy value counts, because the previous release declared it with option() and every existing build tree caches it as OFF. The plugin tests try find_package(GTest CONFIG) before fetching, and samples forwards what it resolved through the ExternalProject_Add cache arguments, which is otherwise an isolation boundary no package resolution crosses. Stubs under CMAKE_FIND_PACKAGE_REDIRECTS_DIR are not forwarded; they name in-tree targets the plugin's separate build does not have. When no package is forwardable but FetchContent has already populated GoogleTest, its source tree is handed over instead, so the plugin adopts it rather than downloading a second copy. The plugin's independent v1.14.0 pin becomes a cache variable fed from HIPDNN_GTEST_VERSION. The rocke wheel venv inherits rocm_kpack's runtime dependencies from Python3_EXECUTABLE through a seeded .pth rather than installing msgpack and zstandard from an index. --system-site-packages cannot express this: venv inherits from sys.base_prefix, not from the invoking interpreter, and the superbuild's interpreter is itself a venv. Verified: the superbuild builds hipdnn, the integration tests, all three providers and the samples with no fetch during the build, 12126 unit tests pass, and hipdnn, samples and hip-kernel-provider build inside a network namespace with no interfaces and no failing edges. Refs #12302. Requires ROCm/TheRock#8345 for the TheRock path.
BrianHarrisonAMD
force-pushed
the
users/bharriso/samples-gtest-find-package
branch
from
September 19, 2026 00:01
b103c3c to
07dc5f8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hipDNN and the dnn-providers fell back to
FetchContentwhenever afind_packagemissed. TheRock owns third-party acquisition and prescribesfind_package: https://github.com/ROCm/TheRock/blob/main/docs/development/dependencies.mdFetching is now opt-in through
ALLOW_FETCH_DEPS, defaultOFF. A miss fails configuration naming the package and the remedy.Risk Assessment
Risk level 3. Build system only. Changes dependency resolution for hipDNN, all three providers and the samples, and alters a default: a standalone configure against an incomplete prefix now fails where it downloaded. The superbuild sets
ALLOW_FETCH_DEPS=ON, so the documented developer flow is unchanged.Related
therock-googletestvisible tohipDNN_samples. Until it lands and the pinned TheRock ref moves, the plugin resolves GoogleTest as it does today.Technical Changes
projects/hipdnn/cmake/Dependencies.cmake,dnn-providers/cmake/Dependencies.cmake, rootCMakeLists.txtALLOW_FETCH_DEPSreplacesHIPDNN_NO_DOWNLOAD, which was inverted: enabling it forcedFETCHCONTENT_FULLY_DISCONNECTEDoff. Shared withdnn-providers, which had no such option.FETCHCONTENT_FULLY_DISCONNECTEDstays as a backstop, set as a normal variable so it does not reach sibling subprojects that run their ownFetchContent.HIPDNN_NO_DOWNLOADworks for one more release. Only a truthy value counts: the previous release declared it withoption(), so every existing build tree caches it asOFF.projects/hipdnn/samples/CMakeLists.txt,.../example_engine_plugin/tests/CMakeLists.txtfind_package(GTest CONFIG)first; the independentv1.14.0pin becomes a cache variable fed fromHIPDNN_GTEST_VERSION.samplesforwards the resolved location through theExternalProject_Addcache arguments. Stubs underCMAKE_FIND_PACKAGE_REDIRECTS_DIRare excluded — they name in-tree targets the plugin's separate build lacks.FETCHCONTENT_SOURCE_DIR_GOOGLETEST. Both entries are cleared with-U, asnlohmann_json_DIRalready is.dnn-providers/hip-kernel-provider/descriptor-packaging/cmake/HkpPackaging.cmakemsgpack/zstandardfromPython3_EXECUTABLEthrough a seeded.pthinstead of installing them from an index.--system-site-packagesinherits fromsys.base_prefix, not the invoking interpreter, and the superbuild's interpreter is a venv.site-packagesprecedes the seeded paths.hkp_require_kpack_runtime()runs againstPython3_EXECUTABLEat configure time; it previously ran only against the venv python, which does not exist until the build.Device / Architecture Coverage
No device-specific code. Passing PR CI is sufficient.
Testing Checklist
hipdnn;hipdnn-integration-tests;miopen-provider;hipblaslt-provider;hip-kernel-provider;hipdnn-samples, ingestor and rocKE on — Status: Passed, zero fetches during the build, all providers reportFound system GTestunshare -rn cmake --build ... -- -k 0over hipdnn, samples and hip-kernel-provider including ingestor packing — Status: Passed, 0 failing edgesALLOW_FETCH_DEPS=ON/OFF,HIPDNN_NO_DOWNLOAD=ON/OFF— Status: Passedspdlog— Status: Passed, error namesspdlogandALLOW_FETCH_DEPSFlags / Guardrails
ALLOW_FETCH_DEPS, defaultOFF. The superbuild sets itONwithoutFORCE, so an explicitOFFwins. Per-dependency opt-ins such asHIPKERNELPROVIDER_KPACK_ALLOW_FETCHare unaffected.Known Limitations
OVERRIDE_FIND_PACKAGE, pinningGTest_DIRto the redirect stub. The source-tree hand-off covers that path.ALLOW_FETCH_DEPSinto the plugin sub-build is not included; it requires Add therock-googletest to hipDNN_samples build deps TheRock#8345 first.HIPDNN_GTEST_VERSION1.16.0, a literal indnn-providers/cmake/Dependencies.cmake1.16.0, and the superbuild root 1.17.0. This removes the plugin's fourth.rocke-pyenvstill installs from PyPI during the build. rocKE-owned, tracked separately.