Enable building with THEROCK_BUNDLE_SYSDEPS set to OFF#3538
Conversation
marbre
changed the title
OFFTHEROCK_BUNDLE_SYSDEPS set to OFF
marbre
force-pushed
the
users/marbre/bundled-deps
branch
2 times, most recently
from
4745790 to
3bd7504
Compare
|
@marbre FTR, this is the debug agent fix to allow unbundled sysdeps builds: ROCm/rocm-systems#3451 |
Thanks! Left two comments. |
Replace hardcoded therock-* target names with the corresponding
${THEROCK_BUNDLED_*} variables in all consumer CMakeLists.txt files
outside of third-party/sysdeps. This allows components to build
correctly when THEROCK_BUNDLE_SYSDEPS=OFF, where those targets do
not exist and the build should fall back to system-provided libraries.
Co-Authored-By: Claude <noreply@anthropic.com>
When THEROCK_BUNDLE_SYSDEPS=OFF, rocgdb's CMakeLists.txt previously
issued a fatal error via fetch_library_prefix() for each missing bundled
staging path (gmp, mpfr, expat, ncurses). This blocked any configuration
that uses system-installed libraries.
Changes:
- debug-tools/CMakeLists.txt: forward THEROCK_BUNDLE_SYSDEPS into the
rocgdb subproject cmake invocation.
- debug-tools/rocgdb/CMakeLists.txt:
- Declare THEROCK_BUNDLE_SYSDEPS option (default OFF).
- Make fetch_library_prefix() non-fatal: missing paths now emit
STATUS and return an empty string instead of SEND_ERROR.
- Guard fetch_library_prefix calls and ncurses include/rpath setup
behind if(THEROCK_BUNDLE_SYSDEPS).
- Conditionalize --with-expat-prefix, --with-gmp, --with-mpfr
configure flags; when empty, autotools discovers the libraries
via pkg-config from standard system paths.
Co-Authored-By: Claude <noreply@anthropic.com>
When building with system-provided libraries, the rocm_sysdeps/lib RPATH entry serves no purpose and adds noise to the binary's RUNPATH. Conditionalize it alongside the other bundled-sysdep logic. ROCGDB_SYSDEPS_RPATH_SEGMENT is set to the $ORIGIN-relative segment when bundling, and to empty string otherwise. CMake expands it into the install(CODE) strings at configure time; bare $ORIGIN is not a CMake variable reference so it is preserved literally for patchelf. Co-Authored-By: Claude <noreply@anthropic.com>
marbre
force-pushed
the
users/marbre/bundled-deps
branch
from
d1b77b2 to
cdf0d90
Compare
|
@lumachad I rebased on the latest main, which also includes the elfutils patch. PTAL. |
|
@ScottTodd I would appreciate if you could take a look at the non-rcgdb specific changes. |
| ${THEROCK_BUNDLED_BZIP2} | ||
| ${THEROCK_BUNDLED_GMP} | ||
| ${THEROCK_BUNDLED_MPFR} | ||
| ${THEROCK_BUNDLED_EXPAT} | ||
| ${THEROCK_BUNDLED_NCURSES} | ||
| ${THEROCK_BUNDLED_LIBLZMA} | ||
| ${THEROCK_BUNDLED_ZLIB} | ||
| ${THEROCK_BUNDLED_ZSTD} |
There was a problem hiding this comment.
makes rocGDB buildable with system-provided (non-bundled) sysdeps
Can you elaborate a bit more on the intended usage here?
- Do you expect some subprojects to always build with "unbundled" (i.e. coming from the OS, outside the build system) sysdeps? Wouldn't that defeat the purpose of sysdeps? Is that okay/intended for rocgdb in particular?
- Or do you expect that all subprojects should use the same
THEROCK_BUNDLE_SYSDEPSvalue? Would we use that in our releases or would it be for developer use / non-portable deployments somehow?
There was a problem hiding this comment.
That particular comment
makes rocGDB buildable with system-provided (non-bundled) sysdeps
refers to how rocDB sets CMake prefixes and other variables. The prefixes always where pointing to the bundled sysdeps even with THEROCK_BUNDLE_SYSDEPS set to OFF.
From my perspective THEROCK_BUNDLE_SYSDEPS should be a global setting respected by all components. Either build with what TheRock ships or with the system-installed libs (option 2). We had this a bit ago but this broke by not using the ${THEROCK_BUNDLED_* variables and instead hard coding the deps on packages that are only build with THEROCK_BUNDLE_SYSDEPS set to ON.
We won't use this in our releases but Linux distributions (which normally strictly disallow bundling deps) as well as the community (dev/user) might (see my comment #3477 (comment) or #3132).
Hope this makes it more clear?
There was a problem hiding this comment.
Thanks. We might want a ~weekly (or some other reduced frequency) CI job that builds for e.g. Ubuntu in this mode to ensure that it continues working.
There was a problem hiding this comment.
Absolutely! Started as a drive by and ended up in more work than I expected.
lumachad
left a comment
lumachad
left a comment
There was a problem hiding this comment.
OK for the rocgdb parts.
3 participants
When
THEROCK_BUNDLE_SYSDEPS=OFFis set, thetherock-*targets for sysdep libraries do not exist. Any component that hardcoded therock-zlib, therock-gmp, etc. as RUNTIME_DEPS or SUBPROJECT_DEPS would fail to configure, and rocgdb would emit fatal errors for each missing bundled staging path.This makes progress to allow building with with bundles sysdeps disabled. Further work on rocr-debug-agents will be addressed upstream.
This patch
therock-*target names inbase/,debug-tools/, andthird-party/grpc/with the correspondingTHEROCK_BUNDLED_*CMake variables.