Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CMake support; Resolves #565 #566

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Conversation

FunMiles
Copy link

@FunMiles FunMiles commented Feb 1, 2023

The initial PR is way incomplete. It does work for MacOS :

  • It does a proper and clean install of the library with the required public include files (including generated ones)
  • It can be used a standalone or as a sub-project

However:

  • No support yet for any other OS
  • Not all tests and utilities are compiled

bgoglin and others added 30 commits June 28, 2022 11:36
Bump soname to 21:0:6 (vs 20:3:5 for 2.7.1),

Reuse AM 1.16.5 and AC 2.71, but bump LT to 2.4.7.

Signed-off-by: Brice Goglin <[email protected]>
Still too experimental, might get removed if we don't find some manpower to revive it

And also disable netloc in sonarscanner job

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 2082c24
                       and 15a927b)
(cherry picked from commit eddcbcf)
(cherry picked from commit 30075ab)
(cherry picked from commit 29af441)
(cherry picked from commit dfab43e)
(cherry picked from commit 4fcdc93)
(cherry picked from commit b2eff2b)
Signed-off-by: Brice Goglin <[email protected]>
Reuse the timeslot of v2.3 which is unlikely to ever run again.

Signed-off-by: Brice Goglin <[email protected]>
It's already defined in Cygwin's /usr/include/w32api/_mingw.h

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c91712e)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit ac18e13)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 4732ec4)
Thanks to Jonathan Peyton for the fix.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 1f0283e)
This issue seems to happen more frequently, don't report it unless
all errors are enabled.

This warning means that some PCI devices might be missing.
That's not critical enough to warn users (most of them know nothing about hwloc).
For the record, CUDA init warnings were demoted the same a couple releases ago.

Refs open-mpi#354

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 69fbc20)
…ors too

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 64e01e8)
Forgotten in commit 69fbc20

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c100a52)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 3329842)
Don't fallthough to xml filenames after setting COMPREPLY for --refname.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c57d0fe)
We were still using index instead of id.

Thanks to Guillaume Mercier.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 61083f1)
Closes open-mpi#542

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 8d81da3)
Escape double-quotes in comments (they break the parser and the remaining
enum item doc is ignored in manpages).

Hide initializer values.

Uniformize the source formatting to keep it easy to read even if the
manpage formatting isn't perfect (looks like doxygen/manpage wants
only two paragraph per enum item, but we'd have to change lots of
other places, and it would make the source harder to read).

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit d316fa7)
MinGW doesn't get uint64_t on our CI.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 601b58d)
Possible version values aren't documented in NVML API doc,
but wikipedia and other sources say 50GB/s per link
for v3 (Ampere) and v4 (Hopper).

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 352348a)
hwloc_backends.c needs some open/read/close/mktemp redirection
to _foo. Copy what we did in pci-common.c and add mktemp.

Thanks to Mario Emmenlauer for the report.

Fixes open-mpi#546

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 0de22a5)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 3b5d5d8)
128 FP32 cores again.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit a908085)
Looks like we always need free the locally allocated infos.

(cherry picked from commit b86f63f)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 4630002)
And 8.7 is officially documented now.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 604fdd5)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e84553d)
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit b5c07dd)
And use "CXLMem" as a string for PCI_CLASS_MEMORY_CXL = 0x0502

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit e0f56e2)
We'll need to double-check whether they are useful in the end.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit a172c84)
Signed-off-by: Brice Goglin <[email protected]>
bgoglin and others added 5 commits January 3, 2023 13:14
Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit c938d81)
…onditional

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit 471e43d)
HWLOC_VERSION* macros were X.Yrc1 instead of X.Y in the MSVC builds.

Windows CMAKE builds don't include suffixes such as rc1, but we still
update the hwloc version there for consistency (and in case X.Y was
changed too).

Android isn't updated since it isn't distributed in tarballs.

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit accb702)
Forgotten in f7c9aa8

Thanks to Michel Lesoinne for the report.

Closes open-mpi#564

Signed-off-by: Brice Goglin <[email protected]>
(cherry picked from commit aa0ef16)
Currently only effective for MacOS. Hoping it can be improved to become general for more systems.
@bgoglin
Copy link
Contributor

bgoglin commented Feb 1, 2023

What do you mean "standalone" or "subproject"? It's cmake-specific ways to use it?

Do you need/want to make this for more than just MacOS X? Do you need/want more than just the library? I'd like to keep things simple and easy to test and maintain. Hence supporting less things would be nice. Building/running most tests under tests/hwloc shouldn't be too hard if only supporting Mac OS X. Running other tests may require to build tools, at least lstopo. Building lstopo requires some configuration/detection, for instance because of all lstopo backends (cairo, etc). Building lstopo in minimalistic mode (no cairo, no X11, etc) is easier (and will be enough for some tests).

@FunMiles
Copy link
Author

FunMiles commented Feb 1, 2023

What do you mean "standalone" or "subproject"? It's cmake-specific ways to use it?

Standalone vs subproject is a CMake specific issue. You can put the source of a library within your own project and then bring in the CMakeLists.txt of the subproject within your compilation. If you do this, you generally may not want that all the testing code be built, nor that the subproject library be installed. As the use via FetchContent mechanism does bring the library as a subproject, I wanted to make sure the build is clean for that case.
It is nowadays good CMake practice that any library project should be build in such a way that both form of use are cleanly supported.

Do you need/want to make this for more than just MacOS X? Do you need/want more than just the library? I'd like to keep things simple and easy to test and maintain. Hence supporting less things would be nice. Building/running most tests under tests/hwloc shouldn't be too hard if only supporting Mac OS X. Running other tests may require to build tools, at least lstopo. Building lstopo requires some configuration/detection, for instance because of all lstopo backends (cairo, etc). Building lstopo in minimalistic mode (no cairo, no X11, etc) is easier (and will be enough for some tests).

I would like to make this for MacOS, Linux and Windows at least.
It is probably best to limit the tests initially. I should be able to add Cairo/X11 detection fairly easily (at least for systems to which I have access) and supporting all the standard tools should be a goal.

@bgoglin
Copy link
Contributor

bgoglin commented Feb 1, 2023

Full Linux support would duplicate looooots of configure checks, and testing them will be very difficult because the target platforms are not in our CI and/or hardly accessible for us.

@FunMiles
Copy link
Author

FunMiles commented Feb 1, 2023

Full Linux support would duplicate looooots of configure checks, and testing them will be very difficult because the target platforms are not in our CI and/or hardly accessible for us.

I am sure you are correct. Let's start small, but at least support basic Linux configs? A lot of code in Linux uses CMake. I could put some warning output messages when running CMake for Linux users stating what is not yet supported/tested.

@jsquyres
Copy link
Member

jsquyres commented Feb 1, 2023

I am sure you are correct. Let's start small, but at least support basic Linux configs?

It's not really a question of only supporting a smaller set of features. Many of the configure tests are for portability between all the different distros / versions of Linux. E.g., the same feature XYZ may have differences between different distros and versions. Linux != Linux != Linux, unfortunately.

A lot of code in Linux uses CMake.

Just to be clear: all the code in Linux that uses CMake can still use hwloc as it is. The justifications for using CMake is not about making it available to users who currently cannot use hwloc because it is built with the Autotools.

@@ -1,7 +1,7 @@
cmake_minimum_required(VERSION 3.15)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file is not needed, right?

@FunMiles
Copy link
Author

I am sure you are correct. Let's start small, but at least support basic Linux configs?

It's not really a question of only supporting a smaller set of features. Many of the configure tests are for portability between all the different distros / versions of Linux. E.g., the same feature XYZ may have differences between different distros and versions. Linux != Linux != Linux, unfortunately.

A lot of code in Linux uses CMake.

Just to be clear: all the code in Linux that uses CMake can still use hwloc as it is. The justifications for using CMake is not about making it available to users who currently cannot use hwloc because it is built with the Autotools.

I've been away for a long time, but is there an easy way to get a list for all the differences that impact hwloc?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants