patch CMake's UnixPaths.cmake script if --sysroot is set #2248

boegel · 2020-11-22T19:20:24Z

Alternative to #2247, which isn't working out well...

@bartoldeman Is this more or less what you had in mind?

…LIBRARY_PATH in CMake's UnixPaths.cmake script

bartoldeman

I'll try out this block tomorrow morning and see if it's working ok.

bartoldeman · 2020-11-23T02:50:01Z

easybuild/easyblocks/c/cmake.py

@@ -95,6 +130,7 @@ def configure_step(self):
            self.log.info("Found sysroot '%s', adding it to $CMAKE_PREFIX_PATH and $CMAKE_LIBRARY_PATH", sysroot)
            cmake_prefix_path.append(sysroot)
            cmake_library_path.append(os.path.join(sysroot, 'usr', 'lib'))
+            cmake_library_path.append(os.path.join(sysroot, 'usr', 'lib64'))


I wonder if this chunk is still needed at all: if CMake is patched itself do you still need those environment variables set?

So I believe this chunk should be removed since these directories are already searched via the compiled-in CMAKE_SYSTEM_PREFIX_PATH.

They should actually never be required. CMake already reroots paths when the SYSPATH or FIND_ROOT_PATH is passed

bartoldeman · 2020-11-23T02:50:47Z

easybuild/easyblocks/c/cmake.py

+        if sysroot:
+            # prepend custom sysroot to all hardcoded paths like /lib and /usr
+            # see also patch applied by Gentoo:
+            # https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/cmake/files/cmake-3.14.0_rc3-prefix-dirs.patch


There are many small differences versus this patch. Those may not matter though but we should carefully check.

I checked and all the small differences look inconsequential. Here '-' is the Gentoo-patched cmake and '+' the easyblock patch:

list(APPEND CMAKE_SYSTEM_PREFIX_PATH # Standard - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/local /cvmfs/soft.computecanada.ca/gentoo/2020/usr /cvmfs/soft.computecanada.ca/gentoo/2020/ + /cvmfs/soft.computecanada.ca/gentoo/2020/usr/local /cvmfs/soft.computecanada.ca/gentoo/2020/usr /cvmfs/soft.computecanada.ca/gentoo/2020

this is just a slash

# Non "standard" but common install prefixes list(APPEND CMAKE_SYSTEM_PREFIX_PATH - /usr/X11R6 - /usr/pkg - /opt + /cvmfs/soft.computecanada.ca/gentoo/2020/usr/X11R6 + /cvmfs/soft.computecanada.ca/gentoo/2020/usr/pkg + /cvmfs/soft.computecanada.ca/gentoo/2020/opt

these don't exist in the prefix, and most likely /opt/bin, /opt/include aren't used either so this looks harmless.

-list(APPEND CMAKE_SYSTEM_LIBRARY_PATH - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/x86_64-pc-linux-gnu/lib//gcc - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/x86_64-pc-linux-gnu/lib/ - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib64 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/libx32 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib32 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib - /cvmfs/soft.computecanada.ca/gentoo/2020/lib - ) - -list(APPEND CMAKE_SYSTEM_PROGRAM_PATH - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/bin - /cvmfs/soft.computecanada.ca/gentoo/2020/bin +list(APPEND CMAKE_SYSTEM_LIBRARY_PATH /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib64 /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib /cvmfs/soft.computecanada.ca/gentoo/2020/lib64 /cvmfs/soft.computecanada.ca/gentoo/2020/lib + # X11 + )

The GCC paths are Gentoo GCC so not relevant to EB GCC.
The X11 path is a historical artifact, it's no longer relevant.
Appending to CMAKE_SYSTEM_PROGRAM_PATH on the Gentoo side looks silly since this defaults to CMAKE_SYSTEM_PREFIX_PATH entries suffixed with /bin already.

list(APPEND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/x86_64-pc-linux-gnu/lib//gcc - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/x86_64-pc-linux-gnu/lib/ - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib64 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/libx32 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib32 - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib - /cvmfs/soft.computecanada.ca/gentoo/2020/lib + /cvmfs/soft.computecanada.ca/gentoo/2020/lib /cvmfs/soft.computecanada.ca/gentoo/2020/lib32 /cvmfs/soft.computecanada.ca/gentoo/2020/lib64 /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib32 /cvmfs/soft.computecanada.ca/gentoo/2020/usr/lib64 )

same story as above for GCC

@@ -92,11 +77,11 @@ # parsing the implicit directory information from compiler output. set(_CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES_INIT ${CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES} - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/include + "${_cmake_sysroot_compile}/usr/include" ) set(_CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES_INIT ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES} - /cvmfs/soft.computecanada.ca/gentoo/2020/usr/include + "${_cmake_sysroot_compile}/usr/include" ) set(_CMAKE_CUDA_IMPLICIT_INCLUDE_DIRECTORIES_INIT ${CMAKE_CUDA_IMPLICIT_INCLUDE_DIRECTORIES}

not sure why Gentoo patches this. The implicit include directories are ok when they are initialized from the compiler output at CMake-compile-time, so we should not worry about this bit.

Flamefire · 2020-11-23T13:28:15Z

I don't think this is a good idea. Using sysroot from a toolchain file sounds much much better.
Counter example: Some software does find_library(FOO foo.so PATHS /usr/foo)
This won't be caught be the patch approach but by the sysroot approach
Better without a toolchain file (if --sysroot is not really required):
CMAKE_FIND_ROOT_PATH with CMAKE_FIND_ROOT_PATH_MODE_LIBRARY and the other 2

bartoldeman · 2020-11-23T16:41:35Z

I may have misunderstood your point here @Flamefire but the question is here if find_library(FOO foo.so PATHS /usr/foo) should look in $EPREFIX/usr/foo or in /usr/foo.

In the EESSI (or CC CVMFS) Gentoo Prefix context the paths are absolute and not auto-prepended with any prefix, i.e. the output of pkg-config and friends includes the prefix, and if you then prepend the prefix to that you get a double prefix.

Flamefire · 2020-11-23T17:11:04Z

@bartoldeman IIUC the whole point of sysroot is that you use $EPREFIX and not /usr/foo, is this correct?

The CMake find_* commands have logic which will prepend the CMAKE_FIND_ROOT_PATH (or CMAKE_SYSROOT) to ALL paths passed to the find_* command (explicit and implicit via e.g. CMAKE_PREFIX_PATH) And via some variables you can control if the non-prefixed paths should be searched too. This is useful so that find_library(FOO foo.so PATHS /usr/foo) searches $EPREFIX/usr/foo.

I can see that failing though as e.g. modules set CMAKE_PREFIX_PATH and you don't want to search prepended paths here but you want (only) them for the paths passed to PATHS (above) and for CMAKE_SYSTEM_PREFIX_PATH (autogenerated)

This is tough... I guess then this is the right approach to get it right most of the time. Make sure to test this thoroughly with --debug-find

bartoldeman · 2020-11-23T19:08:33Z

I think we are conflating two uses of sysroot here, one as used in cross-compilation strategies and one used as a prefix in a Gentoo Prefix installation.

I'll do some more research and get back on this.

boegel · 2023-05-31T06:16:02Z

@bartoldeman I would like to get back to this, and get it merged ASAP.

Maybe we should set up a quick call to discuss what needs to change here?

We've been using this easyblock in EESSI for a while now, and haven't seen any problems with CMake itself, or stuff that involves CMake in the installation procedure...

bartoldeman

I think rereading my comments it's ok, but I would still remove the whole chunk from line 129 to line 136 since it looks superfluous.

sysroot = ...
if sysroot:
    ...

…d $CMAKE_LIBRARY_PATH in configure step of CMake easyblock - no longer needed due to sysroot-aware patching of Modules/Platform/UnixPaths.cmake

boegel · 2023-07-06T17:25:23Z

Test report by @boegel

Overview of tested easyconfigs (in order)

SUCCESS CMake-3.20.1-GCCcore-10.3.0.eb
SUCCESS CMake-3.22.1-GCCcore-11.2.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
node3129.skitty.os - Linux Debian GNU/Linux 11 (bullseye), x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.11.4
See https://gist.github.com/boegel/4af87dbe3320d0100362d626e95f6d65 for a full test report.

edit: tested in EESSI build environment, using --sysroot

boegel · 2023-07-06T17:43:06Z

Test report by @boegel

Overview of tested easyconfigs (in order)

SUCCESS CMake-3.20.1-GCCcore-10.3.0.eb
SUCCESS CMake-3.22.1-GCCcore-11.2.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
node3129.skitty.os - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/boegel/23839eedcef7b0a650c5664fa8ae7134 for a full test report.

bartoldeman

LGTM

patch CMake's UnixPaths.cmake script if --sysroot is set

60b8522

boegel added bug fix EESSI Related to EESSI project labels Nov 22, 2020

boegel added this to the 4.3.2 (next release) milestone Nov 22, 2020

This was referenced Nov 22, 2020

cmakemake easyblock add -DCMAKE_ EESSI/software-layer#36

Open

fix how --sysroot is taken into account in CMakeMake easyblock #2247

Closed

boegel added 2 commits November 22, 2020 20:35

also replace hardcoded / with sysroot in CMake's UnixPaths.cmake

908baec

also add <sysroot>/usr/lib* and <sysroot>/lib* paths to CMAKE_SYSTEM_…

6bf4d1f

…LIBRARY_PATH in CMake's UnixPaths.cmake script

bartoldeman requested changes Nov 23, 2020

View reviewed changes

boegel modified the milestones: 4.3.2 (next release), 4.4.0 Dec 1, 2020

boegel mentioned this pull request Dec 5, 2020

lib vs lib64 in Prefix installation differs between architectures EESSI/compatibility-layer#63

Closed

boegel modified the milestones: 4.3.3 (next release), release after 4.3.3 Feb 19, 2021

boegel modified the milestones: next release (4.3.4), release after 4.3.4 Apr 6, 2021

boegel modified the milestones: 4.4.0, release after 4.4.0 May 26, 2021

boegel modified the milestones: next release (4.4.1), release after 4.4.1 Jul 2, 2021

boegel modified the milestones: 4.4.2, release after 4.4.2 Sep 1, 2021

boegel modified the milestones: 4.5.0 (next release), release after 4.5.0 Oct 27, 2021

boegel modified the milestones: 4.5.1, release after 4.5.1 Dec 7, 2021

boegel added this to the release after 4.5.3 milestone Feb 9, 2022

boegel modified the milestones: next release (4.5.4), release after 4.5.4 Mar 25, 2022

boegel modified the milestones: 4.5.5, release after 4.5.5 Jun 4, 2022

boegel modified the milestones: next release (4.6.0), release after 4.6.0 Jul 6, 2022

boegel modified the milestones: next release (4.6.1), release after 4.6.1 Sep 9, 2022

boegel modified the milestones: next release (4.6.2?), release after 4.6.2 Oct 18, 2022

boegel modified the milestones: next release (4.7.0), release after 4.7.0 Dec 20, 2022

boegel modified the milestones: next release (4.7.1), release after 4.7.1 Mar 1, 2023

boegel modified the milestones: next release (4.7.2), release after 4.7.2 May 10, 2023

Merge branch 'develop' into patch_cmake_sysroot

3f52ad2

boegel mentioned this pull request Jun 4, 2023

{lang}[GCCcore/12.3.0] Rust v1.70.0 easybuilders/easybuild-easyconfigs#18016

Merged

boegel mentioned this pull request Jun 23, 2023

{2023.06}[2021b] CMake v3.21.1 + CMake v3.22.1 EESSI/software-layer#273

Merged

boegel mentioned this pull request Jul 5, 2023

document contribution policy for adding software to EESSI EESSI/docs#108

Merged

Merge branch 'develop' into patch_cmake_sysroot

76561b0

bartoldeman requested changes Jul 6, 2023

View reviewed changes

strip out adding a sysroot lib/include paths to $CMAKE_PREFIX_PATH an…

5edf172

…d $CMAKE_LIBRARY_PATH in configure step of CMake easyblock - no longer needed due to sysroot-aware patching of Modules/Platform/UnixPaths.cmake

bartoldeman approved these changes Jul 6, 2023

View reviewed changes

bartoldeman merged commit fddeab7 into easybuilders:develop Jul 6, 2023

boegel deleted the patch_cmake_sysroot branch July 6, 2023 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch CMake's UnixPaths.cmake script if --sysroot is set #2248

patch CMake's UnixPaths.cmake script if --sysroot is set #2248

boegel commented Nov 22, 2020 •

edited

Loading

bartoldeman left a comment

bartoldeman Nov 23, 2020

bartoldeman Nov 23, 2020

Flamefire Nov 23, 2020

bartoldeman Nov 23, 2020

bartoldeman Nov 23, 2020

Flamefire commented Nov 23, 2020

bartoldeman commented Nov 23, 2020

Flamefire commented Nov 23, 2020

bartoldeman commented Nov 23, 2020

boegel commented May 31, 2023

bartoldeman left a comment

boegel commented Jul 6, 2023 •

edited

Loading

boegel commented Jul 6, 2023

bartoldeman left a comment

patch CMake's UnixPaths.cmake script if --sysroot is set #2248

patch CMake's UnixPaths.cmake script if --sysroot is set #2248

Conversation

boegel commented Nov 22, 2020 • edited Loading

bartoldeman left a comment

Choose a reason for hiding this comment

bartoldeman Nov 23, 2020

Choose a reason for hiding this comment

bartoldeman Nov 23, 2020

Choose a reason for hiding this comment

Flamefire Nov 23, 2020

Choose a reason for hiding this comment

bartoldeman Nov 23, 2020

Choose a reason for hiding this comment

bartoldeman Nov 23, 2020

Choose a reason for hiding this comment

Flamefire commented Nov 23, 2020

bartoldeman commented Nov 23, 2020

Flamefire commented Nov 23, 2020

bartoldeman commented Nov 23, 2020

boegel commented May 31, 2023

bartoldeman left a comment

Choose a reason for hiding this comment

boegel commented Jul 6, 2023 • edited Loading

Overview of tested easyconfigs (in order)

boegel commented Jul 6, 2023

Overview of tested easyconfigs (in order)

bartoldeman left a comment

Choose a reason for hiding this comment

boegel commented Nov 22, 2020 •

edited

Loading

boegel commented Jul 6, 2023 •

edited

Loading