Adding option for installing Hwloc using CMake FetchContent #6394

vrnimje · 2023-12-03T07:24:49Z

References #6392

Proposed Changes

Added HPX_WITH_FETCH_HWLOC option for fetching Hwloc using CMake FetchContent
Added CI tests for the same, on Linux, macOS and Windows platforms

Any background context you want to provide?

Checklist

Not all points below apply to all pull requests.

I have added a new feature and have added tests to go along with it.
I have fixed a bug and have added a regression test.
I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

Signed-off-by: Vedant <[email protected]>

…ows build Signed-off-by: Vedant <[email protected]>

Signed-off-by: Vedant <[email protected]>

StellarBot · 2023-12-03T07:25:02Z

Can one of the admins verify this patch?

Signed-off-by: Vedant <[email protected]>

hkaiser · 2023-12-04T21:20:51Z

@vrnimje the ci/circleci: install step fails (https://app.circleci.com/pipelines/github/STEllAR-GROUP/hpx/16294/workflows/33c0c1ec-9c58-4ec5-b900-4892d700503c/jobs/371220). Would you be able to have a look, please?

SAtacker · 2023-12-04T23:16:14Z

cmake/HPX_SetupHwloc.cmake

+    include(FetchContent)
+    fetchcontent_declare(
+      HWLoc
+      URL https://download.open-mpi.org/release/hwloc/v2.9/hwloc-2.9.3.tar.gz


We may need to keep a separate variable for HWLOC version.

Like this?

+ set(HPX_WITH_HWLOC_VERSION "2.9") + set(HPX_WITH_HWLOC_RELEASE "2.9.3") hpx_info( - "HPX_WITH_FETCH_HWLOC=${HPX_WITH_FETCH_HWLOC}, Hwloc will be fetched using CMake's FetchContent" + "HPX_WITH_FETCH_HWLOC=${HPX_WITH_FETCH_HWLOC}, Hwloc v${HPX_WITH_HWLOC_RELEASE} will be fetched using CMake's FetchContent" ) if(UNIX) include(FetchContent) fetchcontent_declare( HWLoc - URL https://download.open-mpi.org/release/hwloc/v2.9/hwloc-2.9.3.tar.gz + URL https://download.open-mpi.org/release/hwloc/v${HPX_WITH_HWLOC_VERSION}/hwloc-${HPX_WITH_HWLOC_RELEASE}.tar.gz TLS_VERIFY true )

I have added them in e67885b

SAtacker · 2023-12-04T23:17:00Z

cmake/HPX_SetupHwloc.cmake

+  elseif("${CMAKE_GENERATOR_PLATFORM}" STREQUAL "Win64")
+    fetchcontent_declare(
+      HWLoc
+      URL https://download.open-mpi.org/release/hwloc/v2.9/hwloc-win64-build-2.9.3.zip


Same as above, we may have to keep a separate variable for HWLOC version.

SAtacker · 2023-12-04T23:18:19Z

cmake/HPX_SetupHwloc.cmake

+  else()
+    fetchcontent_declare(
+      HWLoc
+      URL https://download.open-mpi.org/release/hwloc/v2.9/hwloc-win64-build-2.9.3.zip


It should be win32.

Same as above, we may have to keep a separate variable for HWLOC version.

~~Could you make this change and see if it doesn't fail? I am not sure how it's related to free(): invalid pointer, but it should be win32~~ It's unrelated.

cmake/HPX_SetupHwloc.cmake

Signed-off-by: Vedant <[email protected]>

codacy-production · 2023-12-11T22:31:07Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for `3253ee1`¹	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`3253ee1`)	Report Missing	Report Missing	Report Missing
Head commit (`d9f03d7`)	190065	161880	85.17%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6394)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation}

Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

…g .dylib file Signed-off-by: Vedant <[email protected]>

Signed-off-by: Vedant <[email protected]>

SAtacker · 2024-01-10T17:13:52Z

@hkaiser PTAL

vrnimje · 2024-01-10T17:35:40Z

I have encountered an issue with the Windows build...
The Windows workflow, with the option enabled, is unable to pass any of the tests, due to the following error:

Process 0 failed with an unexpected error code of 3221225781 (expected 0) <_io.TextIOWrapper name='<stderr>' mode='w' encoding='cp1252'>

So I tried recreating the build on my local system (using VS 2022), and I was able to reproduce the error with the tests
And after executing some examples in the repository, I encountered another error...

I am currently trying to find a solution, but any pointers are welcome..

SAtacker · 2024-01-10T17:48:44Z

cmake/HPX_SetupHwloc.cmake

+  elseif("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows" AND CMAKE_SIZEOF_VOID_P
+                                                       EQUAL 8
+  )
+    fetchcontent_declare(
+      HWLoc
+      URL https://download.open-mpi.org/release/hwloc/v${HPX_WITH_HWLOC_VERSION}/hwloc-win64-build-${HPX_WITH_HWLOC_RELEASE}.zip
+      TLS_VERIFY true
+    )
+    if(NOT HWLoc_POPULATED)
+      fetchcontent_populate(HWLoc)
+    endif()
+    set(HWLOC_ROOT
+        "${CMAKE_BINARY_DIR}/_deps/hwloc-src"
+        CACHE INTERNAL ""
+    )
+    include_directories(${HWLOC_ROOT}/include)
+    link_directories(${HWLOC_ROOT}/lib)
+    set(Hwloc_INCLUDE_DIR
+        ${HWLOC_ROOT}/include
+        CACHE INTERNAL ""
+    )
+    set(Hwloc_LIBRARY
+        ${HWLOC_ROOT}/lib/libhwloc.dll.a
+        CACHE INTERNAL ""
+    )
+  else()
+    fetchcontent_declare(
+      HWLoc
+      URL https://download.open-mpi.org/release/hwloc/v${HPX_WITH_HWLOC_VERSION}/hwloc-win32-build-${HPX_WITH_HWLOC_RELEASE}.zip


PTAL https://github.com/SAtacker/hpx-template/blob/ba47fba9bda7833fdbd9698ee5863ddd959e6513/CMakeLists.txt#L108-L119

@vrnimje I am pretty sure you need

if(WIN32 OR "${CMAKE_GENERATOR_PLATFORM}" STREQUAL "Win64" AND NOT WITH_CUSTOM_HPX) add_custom_command(TARGET opt_bench POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy_directory "${hpx_BINARY_DIR}/${CMAKE_BUILD_TYPE}/bin" $<TARGET_FILE_DIR:opt_bench> ) add_custom_command(TARGET opt_bench POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy_if_different "${HWLOC_ROOT}/bin/libhwloc-15.dll" $<TARGET_FILE_DIR:opt_bench> )

Added this command in 7ac5b33

What about 32Bit Windows? Do we support that here as well?

Yes, 32-bit Windows is also supported here:

hpx/cmake/HPX_SetupHwloc.cmake

Lines 86 to 108 in badead1

else()

fetchcontent_declare(

HWLoc

URL https://download.open-mpi.org/release/hwloc/v${HPX_WITH_HWLOC_VERSION}/hwloc-win32-build-${HPX_WITH_HWLOC_RELEASE}.zip

TLS_VERIFY true

)

if(NOT HWLoc_POPULATED)

fetchcontent_populate(HWLoc)

endif()

set(HWLOC_ROOT

"${CMAKE_BINARY_DIR}/_deps/hwloc-src"

CACHE INTERNAL ""

)

include_directories(${HWLOC_ROOT}/include)

link_directories(${HWLOC_ROOT}/lib)

set(Hwloc_INCLUDE_DIR

${HWLOC_ROOT}/include

CACHE INTERNAL ""

)

set(Hwloc_LIBRARY

${HWLOC_ROOT}/lib/libhwloc.dll.a

CACHE INTERNAL ""

)

I have also tested it locally using cmake -A Win32 ....

SAtacker · 2024-01-10T17:50:06Z

I have encountered an issue with the Windows build... The Windows workflow, with the option enabled, is unable to pass any of the tests, due to the following error:
Process 0 failed with an unexpected error code of 3221225781 (expected 0) <_io.TextIOWrapper name='<stderr>' mode='w' encoding='cp1252'>
So I tried recreating the build on my local system (using VS 2022), and I was able to reproduce the error with the tests And after executing some examples in the repository, I encountered another error...

I am currently trying to find a solution, but any pointers are welcome..

Please see https://github.com/SAtacker/hpx-template/blob/ba47fba9bda7833fdbd9698ee5863ddd959e6513/CMakeLists.txt#L108-L119

Signed-off-by: Vedant <[email protected]>

SAtacker · 2024-01-14T17:18:06Z

@hkaiser please approve the remaining workflows

SAtacker · 2024-01-16T09:36:20Z

@hkaiser PTAL

.github/workflows/linux_debug_fetch_hwloc.yml

CMakeLists.txt

Signed-off-by: Vedant <[email protected]>

codacy-production · 2024-01-17T11:47:59Z

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for `3253ee1`¹	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (`3253ee1`)	Report Missing	Report Missing	Report Missing
Head commit (`209a03b`)	190065	161900	85.18%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6394)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings Change summary preferences

_{You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation}

Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

hkaiser

LGTM, thanks!

SAtacker · 2024-01-18T19:03:46Z

@hkaiser can we please merge this?

SAtacker · 2024-01-25T15:38:56Z

CMakeLists.txt

 if(HPX_WITH_EXAMPLES)
  add_hpx_pseudo_target(examples)
+  if(HPX_WITH_FETCH_HWLOC AND "${CMAKE_SYSTEM_NAME}" STREQUAL "Windows")
+    add_custom_target(
+      HwlocDLL ALL
+      COMMAND ${CMAKE_COMMAND} -E make_directory
+              "${CMAKE_BINARY_DIR}/$<CONFIG>/bin/"
+      COMMAND
+        ${CMAKE_COMMAND} -E copy_if_different
+        "${HWLOC_ROOT}/bin/libhwloc-15.dll"
+        "${CMAKE_BINARY_DIR}/$<CONFIG>/bin/"
+    )
+    add_hpx_pseudo_dependencies(examples HwlocDLL)
+  endif()
 endif()


@vrnimje what happens when HPX_WITH_EXAMPLES is not set? Please make a new PR if this is incorrect

I was only able to find examples as a psuedo-target, to which I could add the dependency I have defined (HwlocDLL)
Since the libhwloc-15.dll needs to be copied exactly to "${CMAKE_BINARY_DIR}/$<CONFIG>/bin/", and the Configuration type $<CONFIG> is defined during the build stage, I used the above method. As far as I understand, $<CONFIG> can only be obtained using add_custom_command or add_custom_target.

~~Is there any other mechanism to achieve this?~~

I now realise that I could directly add HwlocDLL as a pseudo target using add_hpx_pseudo_target. ~~I will test it out~~. It works, will open another PR to correct it
Thanks for pointing this out

vrnimje added 8 commits December 3, 2023 12:29

HWLoc fetch option - initial commit

964641c

Signed-off-by: Vedant <[email protected]>

[CI] Adding ninja-build installation

b357783

Signed-off-by: Vedant <[email protected]>

Fixed Hwloc lib variable name in Hwloc setup

2a2a539

Signed-off-by: Vedant <[email protected]>

Adding file paths to workflow

24143b0

Signed-off-by: Vedant <[email protected]>

Fixing file name in ci

ab678cd

Signed-off-by: Vedant <[email protected]>

Linked lib directly from installed hwloc, for UNIX

1617d65

Signed-off-by: Vedant <[email protected]>

Added lib filename in Win64, and added workflows to test MacOS & Wind…

1140a90

…ows build Signed-off-by: Vedant <[email protected]>

Set new CI activation to pull_request only

803c5de

Signed-off-by: Vedant <[email protected]>

vrnimje requested a review from hkaiser as a code owner December 3, 2023 07:24

Applied cmake-format to the code

5ed80a5

Signed-off-by: Vedant <[email protected]>

hkaiser added category: CMake type: enhancement type: compatibility issue labels Dec 4, 2023

SAtacker suggested changes Dec 4, 2023

View reviewed changes

vrnimje added 2 commits December 5, 2023 14:42

Reverting cmake-format change

fdec3ed

Signed-off-by: Vedant <[email protected]>

Adding separate variables for HWLOC version and release

ee9ace6

Signed-off-by: Vedant <[email protected]>

vrnimje added 2 commits December 17, 2023 12:10

Adding better detection for Win64, and fixed linking in macOS bu usin…

ae03769

…g .dylib file Signed-off-by: Vedant <[email protected]>

Applying cmake-format to the Hwloc setup file

e67885b

Signed-off-by: Vedant <[email protected]>

SAtacker reviewed Jan 10, 2024

View reviewed changes

vrnimje added 3 commits January 12, 2024 05:14

Added custom target to copy libhwloc-15.dll in Win64

7ac5b33

Signed-off-by: Vedant <[email protected]>

Applying cmake-format

badead1

Signed-off-by: Vedant <[email protected]>

Enabling libhwloc-15.dll to be copied in 32-bit Windows as well

d9f03d7

Signed-off-by: Vedant <[email protected]>

hkaiser requested changes Jan 17, 2024

View reviewed changes

.github/workflows/linux_debug_fetch_hwloc.yml Outdated Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

Adding copyright in the new workflow files

209a03b

Signed-off-by: Vedant <[email protected]>

hkaiser approved these changes Jan 17, 2024

View reviewed changes

hkaiser added this to the 1.10.0 milestone Jan 17, 2024

hkaiser merged commit b99b590 into STEllAR-GROUP:master Jan 18, 2024
46 of 48 checks passed

SAtacker reviewed Jan 25, 2024

View reviewed changes

vrnimje mentioned this pull request Jan 26, 2024

Modifying CMakeLists to copy libhwloc-15.dll to the binary folder in Windows, independently #6427

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding option for installing Hwloc using CMake FetchContent #6394

Adding option for installing Hwloc using CMake FetchContent #6394

vrnimje commented Dec 3, 2023 •

edited

Loading

StellarBot commented Dec 3, 2023

hkaiser commented Dec 4, 2023

SAtacker Dec 4, 2023

vrnimje Dec 5, 2023 •

edited

Loading

vrnimje Dec 11, 2023 •

edited

Loading

SAtacker Dec 4, 2023

SAtacker Dec 4, 2023

SAtacker Dec 4, 2023 •

edited

Loading

codacy-production bot commented Dec 11, 2023 •

edited

Loading

SAtacker commented Jan 10, 2024

vrnimje commented Jan 10, 2024 •

edited

Loading

SAtacker Jan 10, 2024

SAtacker Jan 10, 2024

vrnimje Jan 12, 2024

hkaiser Jan 12, 2024

vrnimje Jan 13, 2024 •

edited

Loading

SAtacker commented Jan 10, 2024

SAtacker commented Jan 14, 2024

SAtacker commented Jan 16, 2024

codacy-production bot commented Jan 17, 2024

hkaiser left a comment

SAtacker commented Jan 18, 2024

SAtacker Jan 25, 2024

vrnimje Jan 25, 2024 •

edited

Loading

vrnimje Jan 25, 2024 •

edited

Loading

	else()
	fetchcontent_declare(
	HWLoc
	URL https://download.open-mpi.org/release/hwloc/v${HPX_WITH_HWLOC_VERSION}/hwloc-win32-build-${HPX_WITH_HWLOC_RELEASE}.zip
	TLS_VERIFY true
	)
	if(NOT HWLoc_POPULATED)
	fetchcontent_populate(HWLoc)
	endif()
	set(HWLOC_ROOT
	"${CMAKE_BINARY_DIR}/_deps/hwloc-src"
	CACHE INTERNAL ""
	)
	include_directories(${HWLOC_ROOT}/include)
	link_directories(${HWLOC_ROOT}/lib)
	set(Hwloc_INCLUDE_DIR
	${HWLOC_ROOT}/include
	CACHE INTERNAL ""
	)
	set(Hwloc_LIBRARY
	${HWLOC_ROOT}/lib/libhwloc.dll.a
	CACHE INTERNAL ""
	)

Adding option for installing Hwloc using CMake FetchContent #6394

Adding option for installing Hwloc using CMake FetchContent #6394

Conversation

vrnimje commented Dec 3, 2023 • edited Loading

Proposed Changes

Any background context you want to provide?

Checklist

StellarBot commented Dec 3, 2023

hkaiser commented Dec 4, 2023

Choose a reason for hiding this comment

vrnimje Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

vrnimje Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SAtacker Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

codacy-production bot commented Dec 11, 2023 • edited Loading

Coverage summary from Codacy

Footnotes

SAtacker commented Jan 10, 2024

vrnimje commented Jan 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrnimje Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

SAtacker commented Jan 10, 2024

SAtacker commented Jan 14, 2024

SAtacker commented Jan 16, 2024

codacy-production bot commented Jan 17, 2024

Coverage summary from Codacy

Footnotes

hkaiser left a comment

Choose a reason for hiding this comment

SAtacker commented Jan 18, 2024

Choose a reason for hiding this comment

vrnimje Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

vrnimje Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

vrnimje commented Dec 3, 2023 •

edited

Loading

vrnimje Dec 5, 2023 •

edited

Loading

vrnimje Dec 11, 2023 •

edited

Loading

SAtacker Dec 4, 2023 •

edited

Loading

codacy-production bot commented Dec 11, 2023 •

edited

Loading

vrnimje commented Jan 10, 2024 •

edited

Loading

vrnimje Jan 13, 2024 •

edited

Loading

vrnimje Jan 25, 2024 •

edited

Loading

vrnimje Jan 25, 2024 •

edited

Loading