[1/2] pkg-cmake: add CMAKE_SYSTEM_VERSION to toolchainfile.cmake

Message ID 20190110230529.29852-1-arnout@mind.be
State Accepted
Headers show
Series
  • [1/2] pkg-cmake: add CMAKE_SYSTEM_VERSION to toolchainfile.cmake
Related show

Commit Message

Arnout Vandecappelle Jan. 10, 2019, 11:05 p.m.
Quoting the CMake documentation:

  When the CMAKE_SYSTEM_NAME variable is set explicitly to enable cross
  compiling then the value of CMAKE_SYSTEM_VERSION must also be set
  explicitly to specify the target system version.

Thus, we should also set CMAKE_SYSTEM_VERSION in toolchainfile.cmake. It
is supposed to be set to the value of `uname -r` on the target. We don't
have that exact value available (unless we build the kernel), but the
value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
of the kernel it will run on, so it should be OK for all practical
purposes.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Samuel Martin <s.martin49@gmail.com>
---
 package/pkg-cmake.mk                | 1 +
 support/misc/toolchainfile.cmake.in | 1 +
 2 files changed, 2 insertions(+)

Comments

Yann E. MORIN Jan. 11, 2019, 9:16 p.m. | #1
Arnout, All,

On 2019-01-11 00:05 +0100, Arnout Vandecappelle (Essensium/Mind) spake thusly:
> Quoting the CMake documentation:
> 
>   When the CMAKE_SYSTEM_NAME variable is set explicitly to enable cross
>   compiling then the value of CMAKE_SYSTEM_VERSION must also be set
>   explicitly to specify the target system version.
> 
> Thus, we should also set CMAKE_SYSTEM_VERSION in toolchainfile.cmake. It
> is supposed to be set to the value of `uname -r` on the target. We don't
> have that exact value available (unless we build the kernel), but

> the
> value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
> of the kernel it will run on,

That is not true.

glibc for example will by default carry fallback mechanisms to emulate
some features when it is running on a kernel older than the headers it
was compiled against. In Buildroot we explictly disable that feature by
passing --enable-kernel=$(BR2_TOOLCHAIN_HEADERS_AT_LEAST).

But for external toolchains, we don't know (and can't know) iw they did
pass that option or not, and if they did, what version they requested,
and if they did not, what version glibc uses.

This backward compatibility of glibc can extend quite a bit in the past,
so kernels we would consider pretty old might still be usable with such
toolchains. For example, on glibc-2.24, the minimum required headers
were 3.2, but the runtime could be 2.6.32 on x86_64.

So, we could very well end up building a Buildroot system that runs on a
kernel older than the headers of the toolchain.

Regards,
Yann E. MORIN.

> so it should be OK for all practical
> purposes.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Samuel Martin <s.martin49@gmail.com>
> ---
>  package/pkg-cmake.mk                | 1 +
>  support/misc/toolchainfile.cmake.in | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 51a1929ebb..dd048b0949 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -264,6 +264,7 @@ define TOOLCHAIN_CMAKE_INSTALL_FILES
>  		-e 's#@@TARGET_CC@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CC)))#' \
>  		-e 's#@@TARGET_CXX@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CXX)))#' \
>  		-e 's#@@TARGET_FC@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_FC)))#' \
> +		-e 's#@@CMAKE_SYSTEM_VERSION@@#$(call qstrip,$(BR2_TOOLCHAIN_HEADERS_AT_LEAST))#' \
>  		-e 's#@@CMAKE_SYSTEM_PROCESSOR@@#$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR))#' \
>  		-e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \
>  		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
> diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
> index 6f3c8ca971..276b36629f 100644
> --- a/support/misc/toolchainfile.cmake.in
> +++ b/support/misc/toolchainfile.cmake.in
> @@ -15,6 +15,7 @@ string(REPLACE "/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR
>  list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR})
>  
>  set(CMAKE_SYSTEM_NAME Buildroot)
> +set(CMAKE_SYSTEM_VERSION @@CMAKE_SYSTEM_VERSION@@)
>  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
>  
>  # Set the {C,CXX}FLAGS appended by CMake depending on the build type
> -- 
> 2.20.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni Jan. 12, 2019, 3:06 p.m. | #2
Hello,

On Fri, 11 Jan 2019 22:16:26 +0100, Yann E. MORIN wrote:

> > the
> > value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
> > of the kernel it will run on,  
> 
> That is not true.
> 
> glibc for example will by default carry fallback mechanisms to emulate
> some features when it is running on a kernel older than the headers it
> was compiled against. In Buildroot we explictly disable that feature by
> passing --enable-kernel=$(BR2_TOOLCHAIN_HEADERS_AT_LEAST).
> 
> But for external toolchains, we don't know (and can't know) iw they did
> pass that option or not, and if they did, what version they requested,
> and if they did not, what version glibc uses.
> 
> This backward compatibility of glibc can extend quite a bit in the past,
> so kernels we would consider pretty old might still be usable with such
> toolchains. For example, on glibc-2.24, the minimum required headers
> were 3.2, but the runtime could be 2.6.32 on x86_64.
> 
> So, we could very well end up building a Buildroot system that runs on a
> kernel older than the headers of the toolchain.

While this is true, what can we do in practice ? We have no way to know
what will be the version of the kernel running on the target when we're
not building it, and still we have to provide this CMAKE_SYSTEM_VERSION
value. I am not really sure what packages are doing with
CMAKE_SYSTEM_VERSION. For example, azure-iot-sdk was simply testing
whether it was empty or not.

So for the time being, it seems like using the kernel headers version
is a good enough solution, even if not perfect.

Thomas
Thomas Petazzoni Jan. 12, 2019, 3:07 p.m. | #3
Hello,

On Fri, 11 Jan 2019 00:05:28 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Quoting the CMake documentation:
> 
>   When the CMAKE_SYSTEM_NAME variable is set explicitly to enable cross
>   compiling then the value of CMAKE_SYSTEM_VERSION must also be set
>   explicitly to specify the target system version.
> 
> Thus, we should also set CMAKE_SYSTEM_VERSION in toolchainfile.cmake. It
> is supposed to be set to the value of `uname -r` on the target. We don't
> have that exact value available (unless we build the kernel), but the
> value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
> of the kernel it will run on, so it should be OK for all practical
> purposes.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Samuel Martin <s.martin49@gmail.com>
> ---
>  package/pkg-cmake.mk                | 1 +
>  support/misc/toolchainfile.cmake.in | 1 +
>  2 files changed, 2 insertions(+)

Both applied, thanks!

Thomas
Yann E. MORIN Jan. 12, 2019, 3:23 p.m. | #4
Thomas, Arnout, All,

On 2019-01-12 16:06 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Jan 2019 22:16:26 +0100, Yann E. MORIN wrote:
> 
> > > the
> > > value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
> > > of the kernel it will run on,  
> > 
> > That is not true.
> > 
> > glibc for example will by default carry fallback mechanisms to emulate
> > some features when it is running on a kernel older than the headers it
> > was compiled against. In Buildroot we explictly disable that feature by
> > passing --enable-kernel=$(BR2_TOOLCHAIN_HEADERS_AT_LEAST).
> > 
> > But for external toolchains, we don't know (and can't know) iw they did
> > pass that option or not, and if they did, what version they requested,
> > and if they did not, what version glibc uses.
> > 
> > This backward compatibility of glibc can extend quite a bit in the past,
> > so kernels we would consider pretty old might still be usable with such
> > toolchains. For example, on glibc-2.24, the minimum required headers
> > were 3.2, but the runtime could be 2.6.32 on x86_64.
> > 
> > So, we could very well end up building a Buildroot system that runs on a
> > kernel older than the headers of the toolchain.
> 
> While this is true, what can we do in practice ? We have no way to know
> what will be the version of the kernel running on the target when we're
> not building it, and still we have to provide this CMAKE_SYSTEM_VERSION
> value. I am not really sure what packages are doing with
> CMAKE_SYSTEM_VERSION. For example, azure-iot-sdk was simply testing
> whether it was empty or not.
> 
> So for the time being, it seems like using the kernel headers version
> is a good enough solution, even if not perfect.

Well, random lookups on the internet for CMAKE_SYSTEM_VERSION mostly
tuen up windows- or mac-related questions, so irrelevant to us. The
only seemingly-relevant ones are dealing with android.

There are various pieces of crap here and there that just set the thing
to 1., or some variants of 1, like 1.0 or 1.0.0.

Even the official cmake wiki [0] states:

    CMAKE_SYSTEM_VERSION : optional, version of your target system, not
    used very much.

    [...]

    * CMAKE_TOOLCHAIN_FILE : absolute or relative path to a cmake script
    which sets up all the toolchain related variables mentioned above

    For instance for crosscompiling from Linux to Embedded Linux on
    PowerPC this file could look like this:

        # this one is important
        SET(CMAKE_SYSTEM_NAME Linux)
        #this one not so much
        SET(CMAKE_SYSTEM_VERSION 1)
        [...]

So my opinion would be that we do exactly what is done in the wiki: set
it to 1 (and add a pointer to the wiki) and be done with it.

Then, if/when there are issues we can re-assess the situation.

[0] https://gitlab.kitware.com/cmake/community/wikis/doc/cmake/CrossCompiling

Regards,
Yann E. MORIN.
Yann E. MORIN Jan. 12, 2019, 3:26 p.m. | #5
Thomas, All,

On 2019-01-12 16:07 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Jan 2019 00:05:28 +0100, Arnout Vandecappelle
> (Essensium/Mind) wrote:
> > Quoting the CMake documentation:
> > 
> >   When the CMAKE_SYSTEM_NAME variable is set explicitly to enable cross
> >   compiling then the value of CMAKE_SYSTEM_VERSION must also be set
> >   explicitly to specify the target system version.
> > 
> > Thus, we should also set CMAKE_SYSTEM_VERSION in toolchainfile.cmake. It
> > is supposed to be set to the value of `uname -r` on the target. We don't
> > have that exact value available (unless we build the kernel), but the
> > value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
> > of the kernel it will run on, so it should be OK for all practical
> > purposes.
> > 
> > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > Cc: Samuel Martin <s.martin49@gmail.com>
> > ---
> >  package/pkg-cmake.mk                | 1 +
> >  support/misc/toolchainfile.cmake.in | 1 +
> >  2 files changed, 2 insertions(+)
> 
> Both applied, thanks!

Well, you did not give me the opportunity to reply to your question
about the first patch...

Regards,
Yann E. MORIN.
Thomas Petazzoni Jan. 12, 2019, 4:07 p.m. | #6
Hello,

On Sat, 12 Jan 2019 16:23:43 +0100, Yann E. MORIN wrote:

> Well, random lookups on the internet for CMAKE_SYSTEM_VERSION mostly
> tuen up windows- or mac-related questions, so irrelevant to us. The
> only seemingly-relevant ones are dealing with android.
> 
> There are various pieces of crap here and there that just set the thing
> to 1., or some variants of 1, like 1.0 or 1.0.0.
> 
> Even the official cmake wiki [0] states:
> 
>     CMAKE_SYSTEM_VERSION : optional, version of your target system, not
>     used very much.
> 
>     [...]
> 
>     * CMAKE_TOOLCHAIN_FILE : absolute or relative path to a cmake script
>     which sets up all the toolchain related variables mentioned above
> 
>     For instance for crosscompiling from Linux to Embedded Linux on
>     PowerPC this file could look like this:
> 
>         # this one is important
>         SET(CMAKE_SYSTEM_NAME Linux)
>         #this one not so much
>         SET(CMAKE_SYSTEM_VERSION 1)
>         [...]
> 
> So my opinion would be that we do exactly what is done in the wiki: set
> it to 1 (and add a pointer to the wiki) and be done with it.

That would be fine for me as well. If Arnout agrees, I'll happily take
a patch changing to just "1".

Thomas
Peter Korsgaard Jan. 24, 2019, 3:50 p.m. | #7
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Hello,
 > On Fri, 11 Jan 2019 00:05:28 +0100, Arnout Vandecappelle
 > (Essensium/Mind) wrote:
 >> Quoting the CMake documentation:
 >> 
 >> When the CMAKE_SYSTEM_NAME variable is set explicitly to enable cross
 >> compiling then the value of CMAKE_SYSTEM_VERSION must also be set
 >> explicitly to specify the target system version.
 >> 
 >> Thus, we should also set CMAKE_SYSTEM_VERSION in toolchainfile.cmake. It
 >> is supposed to be set to the value of `uname -r` on the target. We don't
 >> have that exact value available (unless we build the kernel), but the
 >> value of BR2_TOOLCHAIN_HEADERS_AT_LEAST contains the (minimum) version
 >> of the kernel it will run on, so it should be OK for all practical
 >> purposes.
 >> 
 >> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 >> Cc: Samuel Martin <s.martin49@gmail.com>
 >> ---
 >> package/pkg-cmake.mk                | 1 +
 >> support/misc/toolchainfile.cmake.in | 1 +
 >> 2 files changed, 2 insertions(+)

Committed to 2018.02.x and 2018.11.x, thanks.

Patch

diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 51a1929ebb..dd048b0949 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -264,6 +264,7 @@  define TOOLCHAIN_CMAKE_INSTALL_FILES
 		-e 's#@@TARGET_CC@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CC)))#' \
 		-e 's#@@TARGET_CXX@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CXX)))#' \
 		-e 's#@@TARGET_FC@@#$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_FC)))#' \
+		-e 's#@@CMAKE_SYSTEM_VERSION@@#$(call qstrip,$(BR2_TOOLCHAIN_HEADERS_AT_LEAST))#' \
 		-e 's#@@CMAKE_SYSTEM_PROCESSOR@@#$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR))#' \
 		-e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \
 		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
index 6f3c8ca971..276b36629f 100644
--- a/support/misc/toolchainfile.cmake.in
+++ b/support/misc/toolchainfile.cmake.in
@@ -15,6 +15,7 @@  string(REPLACE "/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR
 list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR})
 
 set(CMAKE_SYSTEM_NAME Buildroot)
+set(CMAKE_SYSTEM_VERSION @@CMAKE_SYSTEM_VERSION@@)
 set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
 
 # Set the {C,CXX}FLAGS appended by CMake depending on the build type