Add CMake opt to get HWLOC sources from path instead of github by bratpiorka · Pull Request #1610 · oneapi-src/unified-memory-framework

bratpiorka · 2026-09-30T18:47:17Z

Add CMake option UMF_HWLOC_SOURCE_DIR to get HWLOC sources from path instead of fetching hwloc github repo.


+    - name: Clone hwloc sources
+      if: matrix.local_hwloc == 'ON'
+      run: git clone --depth 1 -b hwloc-2.3.0 https://github.com/open-mpi/hwloc.git ${{env.HWLOC_SRC_DIR}}


        -DUMF_BUILD_LIBUMF_POOL_JEMALLOC=ON
        -DUMF_TESTS_FAIL_ON_SKIP=ON
        -DUMF_LINK_HWLOC_STATICALLY=${{matrix.link_hwloc_statically}}
+        ${{ matrix.local_hwloc == 'ON' && format('-DUMF_HWLOC_SOURCE_DIR={0}', env.HWLOC_SRC_DIR) || '' }}


Copilot

Copilot review overview

🟡 Changes recommended

The Windows companion Debug build does not receive the local hwloc path and can unexpectedly fetch a different source.

Review effort: Balanced
Findings: 1

Open (1)

Forward UMF_HWLOC_SOURCE_DIR to nested build_umfd configure · New

What changed in this PR

Adds support for building statically linked hwloc from a local source directory.

Changes:

Adds and documents UMF_HWLOC_SOURCE_DIR.
Supports local sources on Windows and Unix builds.
Adds CI coverage using hwloc 2.3.0 and fixes test compatibility.

File	Description
`CMakeLists.txt`	Configures local hwloc source handling.
`README.md`	Documents the new option.
`.github/workflows/reusable_basic.yml`	Tests a local hwloc checkout.
`test/memspaces/memspace_highest_bandwidth.cpp`	Supports the older hwloc enum scope.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

The distinct Windows local-source build path lacks CI coverage.

Review effort: Balanced
Findings: 1

Open (1)

Add Windows local-source coverage for static hwloc · New

Resolved since last review (1)

Forward UMF_HWLOC_SOURCE_DIR to nested build_umfd configure

+    - name: Clone hwloc sources
+      if: matrix.local_hwloc == 'ON'
+      # Note: We are using hwloc-2.11.0 because contrib/windows-cmake is not available in older hwloc releases
+      run: git clone --depth 1 -b hwloc-2.11.0 https://github.com/open-mpi/hwloc.git ${{env.HWLOC_SRC_DIR}}


        -DUMF_BUILD_CUDA_PROVIDER=ON
        -DUMF_TESTS_FAIL_ON_SKIP=ON
        -DUMF_LINK_HWLOC_STATICALLY=ON
+        ${{ matrix.local_hwloc == 'ON' && format('-DUMF_HWLOC_SOURCE_DIR={0}', env.HWLOC_SRC_DIR) || '' }}


Copilot

Copilot review overview

🟢 Approval recommended

The local-source flow is validated across supported platforms and no blocking correctness issues were found.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Add Windows local-source coverage for static hwloc

lukaszstolarczuk

LGTM

lukaszstolarczuk · 2026-10-01T14:29:22Z

+        if(UMF_HWLOC_SOURCE_DIR)
+            # Drop build artifacts copied over from an already built local tree
+            set(UMF_HWLOC_PATCH_CMD
+                "(test ! -f Makefile || make distclean) && (${UMF_HWLOC_PATCH_CMD})"


just a question, why we have to run this patch cmd here and right in the next step (ExternalProject_Add)?

bratpiorka · 2026-10-01T16:30:26Z

NOTE: This needs to be set as a draft, as I need to add a separate option to use a mirrored HWLOC repository path.
E.g.

cmake -B build -DUMF_LINK_HWLOC_STATICALLY=ON -DUMF_HWLOC_REPO=/home/user/hwloc.git
# CMake
git clone -b hwloc-2.13.0 /home/user/hwloc.git /home/user/hwloc-src

ldorau · 2026-10-02T06:49:04Z

        set(UMF_LINK_HWLOC_STATICALLY ON)
    endif()
 endif()



The UMF_HWLOC_SOURCE_DIR option is silently ignored if hwloc is linked dynamically. If someone sets UMF_HWLOC_SOURCE_DIR but leaves UMF_LINK_HWLOC_STATICALLY=OFF and system hwloc is found, nothing tells them the path wasn't used. A message(WARNING ...) would help, or the option could turn on static linking automatically. For example:

if(UMF_HWLOC_SOURCE_DIR AND NOT UMF_LINK_HWLOC_STATICALLY) message(WARNING "UMF_HWLOC_SOURCE_DIR is ignored, because UMF_LINK_HWLOC_STATICALLY is OFF") endif()

ldorau · 2026-10-02T06:51:39Z

+                FATAL_ERROR
+                    "UMF_HWLOC_SOURCE_DIR (${UMF_HWLOC_SOURCE_DIR}) is not a directory"
+            )
+        endif()


The source tree is only checked to be a directory. A wrong path, or on Windows a hwloc release without contrib/windows-cmake/, fails later with an unclear error from FetchContent or autogen.sh. Cheap checks would catch this early, for example:

if(WINDOWS) set(UMF_HWLOC_REQUIRED_FILE "contrib/windows-cmake/CMakeLists.txt") else() set(UMF_HWLOC_REQUIRED_FILE "configure.ac") endif() if(NOT EXISTS "${UMF_HWLOC_SOURCE_DIR}/${UMF_HWLOC_REQUIRED_FILE}") message(FATAL_ERROR "UMF_HWLOC_SOURCE_DIR (${UMF_HWLOC_SOURCE_DIR}) does not contain ${UMF_HWLOC_REQUIRED_FILE}") endif()

The hwloc version isn't checked against the 2.3.0 minimum either.

ldorau · 2026-10-02T06:54:03Z

+        if(UMF_HWLOC_SOURCE_DIR)
+            # Drop build artifacts copied over from an already built local tree
+            set(UMF_HWLOC_PATCH_CMD
+                "(test ! -f Makefile || make distclean) && (${UMF_HWLOC_PATCH_CMD})"


make distclean on the copied tree is fragile. CMake's copy_directory probably
doesn't keep file timestamps. If so, running make on a copied, already-configured tree
may try to regenerate config.status or rerun automake before it gets to distclean,
which can fail if the autotools versions differ. Two alternatives:

say in the README that the tree must be clean, or

run git clean -xfd when the copy has a .git directory.

ldorau · 2026-10-02T06:58:21Z

+        # A directory URL is copied into the build tree, so the local clone is
+        # not patched or built in place.
+        set(UMF_HWLOC_SOURCE_ARGS URL ${UMF_HWLOC_SOURCE_DIR})


Edits to the local tree aren't picked up after the first build. The copy only runs again when the download arguments change. This matters for anyone iterating on hwloc sources and is worth a comment in README and also here.

ldorau · 2026-10-02T06:58:27Z

 | UMF_USE_VALGRIND | Enable Valgrind instrumentation | ON/OFF | OFF |
 | UMF_USE_COVERAGE | Build with coverage enabled (Linux only) | ON/OFF | OFF |
 | UMF_LINK_HWLOC_STATICALLY | Link UMF with HWLOC library statically (proxy library will be disabled on Windows+Debug build) | ON/OFF | OFF |
+| UMF_HWLOC_SOURCE_DIR | Path to local hwloc sources used instead of fetching them (with UMF_LINK_HWLOC_STATICALLY) | path | "" |


Edits to the local tree aren't picked up after the first build. The copy only runs
again when the download arguments change. This matters for anyone iterating on hwloc
sources and is worth a comment in the README. The whole .git directory is also copied,
which is wasteful for a full clone (CI uses --depth 1, so it's fine there).

github-advanced-security Bot found potential problems Sep 30, 2026

View reviewed changes

bratpiorka force-pushed the rrudnick_hwloc_source_dir branch 2 times, most recently from 26b779f to fbd7fd6 Compare October 1, 2026 07:33

bratpiorka requested a balanced review from Copilot October 1, 2026 08:16

Copilot started reviewing on behalf of bratpiorka October 1, 2026 08:16 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread CMakeLists.txt

bratpiorka force-pushed the rrudnick_hwloc_source_dir branch from fbd7fd6 to 77af354 Compare October 1, 2026 08:32

bratpiorka requested a balanced review from Copilot October 1, 2026 08:38

Copilot started reviewing on behalf of bratpiorka October 1, 2026 08:39 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread .github/workflows/reusable_basic.yml

bratpiorka added 2 commits October 1, 2026 11:16

Add CMake opt to get HWLOC sources from path instead of github

ac43d85

Use scoped hwloc location type enum in bandwidth test

274a17f

bratpiorka force-pushed the rrudnick_hwloc_source_dir branch from 77af354 to 274a17f Compare October 1, 2026 09:17

github-advanced-security Bot found potential problems Oct 1, 2026

View reviewed changes

bratpiorka requested a balanced review from Copilot October 1, 2026 09:18

Copilot started reviewing on behalf of bratpiorka October 1, 2026 09:19 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

bratpiorka marked this pull request as ready for review October 1, 2026 09:33

bratpiorka requested a review from a team as a code owner October 1, 2026 09:33

bratpiorka requested a review from lukaszstolarczuk October 1, 2026 09:33

bratpiorka assigned ldorau and unassigned ldorau Oct 1, 2026

bratpiorka requested a review from ldorau October 1, 2026 09:33

lukaszstolarczuk approved these changes Oct 1, 2026

View reviewed changes

bratpiorka marked this pull request as draft October 1, 2026 16:24

ldorau requested changes Oct 2, 2026

View reviewed changes

Navigation Menu

Add CMake opt to get HWLOC sources from path instead of github - #1610

Add CMake opt to get HWLOC sources from path instead of github#1610
bratpiorka wants to merge 2 commits into
oneapi-src:mainfrom
bratpiorka:rrudnick_hwloc_source_dir

bratpiorka commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

lukaszstolarczuk left a comment

Uh oh!

lukaszstolarczuk Oct 1, 2026

Uh oh!

bratpiorka commented Oct 1, 2026

Uh oh!

ldorau Oct 2, 2026

Uh oh!

ldorau Oct 2, 2026

Uh oh!

ldorau Oct 2, 2026

Uh oh!

ldorau Oct 2, 2026

Uh oh!

ldorau Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

bratpiorka commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

lukaszstolarczuk Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

bratpiorka commented Oct 1, 2026

Uh oh!

ldorau Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

ldorau Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

ldorau Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

ldorau Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

ldorau Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants