WIndows CMake: work with non-MSVC compilers too

[scivision]

fixes #492

This was tested with GCC, Clang, and Intel oneAPI on Windows.

adds numerous tests as in Autotools-based builds.

This is also the foundation for using CMake on any platform instead of autotools.

Avoids the constant full rebuilding by using configure_file() instead of explicit file()

[scivision]

The WinCmake CI appears to be missing the files I added. Is it extracting some autotools-generated source archive I need to add the new files to a manifest of?

[bgoglin]

Your files are missing because they are not in make dist (generated by automake on one Unix CI slave before passing the resulting tarball to many different CI slaves). For contrib/windows/private_config.h.in, move it to contrib/windows-cmake/, it will be auto-added (contrib/windows/ is only for the MSVC solution). For tests/hwloc/CMakeLists.txt, add it to EXTRA_DIST at the end of tests/hwloc/Makefile.am

[scivision]

OK now it works in Jenkins too

[bgoglin]

Thanks. Can you actually run the tests from contrib/ci.inria.fr/job-1-wincmake.bat ?

/entry:mainCRTStartup is needed for lstopo-win with MSVC but we also need -mwindows instead when building with other compilers (it works with gcc, I don't know about icc on windows). The idea is that lstopo-win is a native windows program (only opens the lstopo window) while lstopo and lstopo-no-graphics use a console and then run lstopo inside it.

On the cosmetic side, can you use a real email instead of scivision@users.noreply.github.com in your signed-off-by lines? And prefix your commit first lines with "windows-cmake:" instead of "windows:" or nothing.

Also add a sentence saying that you don't use stop using windows/private_config.h anymore but your own private_config.h.in for CMake.

Put "fixes" near the __cpuidex line in the commit message too.

Finally, I am not very familiar with CMake and I'll likely forget about all the details within a month. Hence I'd like a bit more explanation in either the commit log or in the CMakeLists.txt. Maybe explain why/how you use target_include_directories, add_library, target_link_library, target_compile_definitions instead of set_target_properties.

[Neumann-A]

Suggested change

	set(HWLOC_X86_32_ARCH)
	set(HWLOC_X86_64_ARCH)
	set(HWLOC_X86_32_ARCH "")
	set(HWLOC_X86_64_ARCH "")

you want to set them empty and not unset previous values correct?

[bgoglin]

I don't know about the CMake syntax but they should be either 1 or undefined in the end, not empty.

[Neumann-A]

Why do you want to have them undefined? Be aware that user might pass -DHWLOC_X86_64_ARCH=1 via the CMake command line. If you don't want that you need to define the variable in local scope.

[bgoglin]

If we change this behavior for CMake, we have to change in configure and in the MSVC solution, and update several code paths using those macros. That's possible, but outside of this PR.

[scivision]

Setting them empty in CMake script gives a known state (undefined in header file). Setting them without any argument is akin to boolean false, which is interpreted by #cmakedefine statement in the *.h.in files as undefined header.

[Neumann-A]

Setting them without any argument is akin to boolean false, which is interpreted by #cmakedefine statement in the *.h.in files as undefined header.

Setting them to "" also achieves that. The CMake docs say:

"depending on whether VAR is set in CMake to any value not considered a false constant by the if() command."

If docs:

"False if the constant is 0, , the empty string, or ends in the suffix -NOTFOUND"

@bgoglin:

If we change this behavior for CMake, we have to change in configure and in the MSVC solution, and update several code paths using those macros. That's possible, but outside of this PR.

I don't want people to pass -DHWLOC_X86_64_ARCH=1 via the cmd line. I just mentioned it that you might need consider that and set(HWLOC_X86_64_ARCH) vs set(HWLOC_X86_32_ARCH "") have different behavior in this case.

[scivision]

It looks like the GUIs work without the -mwindows for GCC ?

[bgoglin]

It looks like the GUIs work without the -mwindows for GCC ?

Yes, the GUI works, but there are two versions: lstopo opens a console that launches the GUI. lstopo-win is a native windows program that directly launches the GUI without opening a console first. That's what -mwindows does in gcc mingw/cygwin and /entry:mainCRTStartup in MSVC. Not sure it exists for ICC. If not, there are other ways to configure this.

[scivision]

OK thanks by using Windows Start>Run menu I can confirm that -mwindows opens GUI without a console, while omitting that flag also opens a console on invocation. This is for MSYS2/MinGW GCC.

However, Clang silently accepts -mwindows but it has no effect--a console window still opens.

CMake has a target property WIN32_EXECUTABLE that automatically sets the correct flags for this instead--which works for Clang etc.

[bgoglin]

I see that you're conditionally adding some backends to the build. Note sure if you saw that it requires to update the the array of components in static-components.h. OpenCL and libXML lines just look like others. The order doesn't matter:
&hwloc_opencl_component,
&hwloc_xml_libxml_component,

[scivision]

so static-components.h should become dynamically configured and use #ifdef conditions in it?

[scivision]

yes I just rebased on master for convenience. I think there were some naming/stylistic topics noted above, and the desire to make a CMake and Pkg-config script, but they might be handled in another PR to avoid further overloading this PR?

[bgoglin]

yes I just rebased on master for convenience. I think there were some naming/stylistic topics noted above, and the desire to make a CMake and Pkg-config script, but they might be handled in another PR to avoid further overloading this PR?

Yes, feel free to keep pkg-config for another PR later.

[bgoglin]

Are we ready for merge then?

3 questions from the non-windows guy:

• I get hwloc.lib instead of libhwloc.lib in our MSVC builds, does this matter anyhow?
• we don't get any DLL because only static libraries are built by default? that's OK for external projects that will link to hwloc?
• are usual warnings enabled during the build (the equivalent of -Wall, etc)? I usually see some warnings when building on Windows, but I don't see any here.

[Neumann-A]

I get hwloc.lib instead of libhwloc.lib in our MSVC builds, does this matter anyhow?

the autotool build will also generated hwloc.lib

we don't get any DLL because only static libraries are built by default? that's OK for external projects that will link to hwloc?

just set https://cmake.org/cmake/help/latest/variable/BUILD_SHARED_LIBS.html to ON via the cmake cmd line (e.g. -DBUILD_SHARED_LIBS=ON)?

are usual warnings enabled during the build (the equivalent of -Wall, etc)?

Warning flags don't belong in build scripts. They belong in the toolchain files or should be set via the cmake cmd line.

[bgoglin]

I get hwloc.lib instead of libhwloc.lib in our MSVC builds, does this matter anyhow?

the autotool build will also generated hwloc.lib

I don't know about MSVC but current GCC builds generate libhwloc.lib. But that's not the question anyway. The question is whether it's normal.

we don't get any DLL because only static libraries are built by default? that's OK for external projects that will link to hwloc?

just set https://cmake.org/cmake/help/latest/variable/BUILD_SHARED_LIBS.html to ON via the cmake cmd line (e.g. -DBUILD_SHARED_LIBS=ON)?

Again, I want to know if that's normal and fine for most external projects.

are usual warnings enabled during the build (the equivalent of -Wall, etc)?

Warning flags don't belong in build scripts. They belong in the toolchain files or should be set via the cmake cmd line.

Default flags should be sane. Building with GCC without -Wall would be insane. What are the default flags?

[Neumann-A]

The question is whether it's normal.

depends on your view. I would say for windows not having the lib prefix is normal. (there is already a .lib suffix ...)

Again, I want to know if that's normal and fine for most external projects.

It doesn't matter. Projects which require shared libraries due to e.g. license issues can simply pass -DBUILD_SHARED_LIBS=ON. (or do you supply prebuild libraries?)

Default flags should be sane. Building with GCC without -Wall would be insane. What are the default flags?

Again depends on the view. -Wall might be useful for you as a developer of hwloc but for consumers of the library building hwloc with Wall is meaningless. If the build is successful it is assumed that the library can be used without issues. Default flags on linux for cmake are mainly defined by the environment or given via the cmd line or a toolchain.

[bgoglin]

We supplied prebuilt libraries until now because building was difficult. We may stop at some point in the future since things are much easier now.

[scivision]

target naming

We can make the library file prefix "lib" with MSVC as well, but this is unconventional for MSVC.
It's up to project preference.
It's a simple option in CMake to do this. CMake default is for all compilers except MSVC to have library filename prefix "lib".

shared/static

We could set BUILD_SHARED_LIBS, a special CMake variable, to the desired default. The default is false, which makes static hwloc.

Compiler flags

There are a few different philosophies on this. My personal practice is to have CMake script contain the per-compiler flags for Debug and Release, etc. selected by if(CMAKE_C_COMPILER_ID ...) elseif(...)

Another approach is with CMakePresets.json, to specify variables like CC and CFLAGS. This is a simple way to specify sets of configurations to use from CI.

[bgoglin]

@scivision I am going to merge this later today unless somebody complains. I am thinking of releasing 2.7 with this very soon because we have many additional changes for Windows that don't deserve to wait several months before public release.

[scivision]

OK Brice--I think this is a good milestone as we've incorporated many helpful improvements suggested by yourself and Alexander. Certainly can be more done to improve in the future.