diff mbox

[1/5] core/pkg-cmake: provide our own platform description

Message ID f25e9ff7873418c0b323528d6a9c2482c752cd31.1488305206.git.yann.morin.1998@free.fr
State Accepted
Headers show

Commit Message

Yann E. MORIN Feb. 28, 2017, 6:07 p.m. UTC
The handling of RPATH in cmake-3.7 has changed drastically, causing a
slew of build failures dues to libraries from the host being pulled in:

  - domoticz : http://autobuild.buildroot.org/results/fd0/fd0ba54c7abf973691b39a0ca1bb4e07d749593a/
  - freerdp  : http://autobuild.buildroot.org/results/5d4/5d429d0e288754a541ee5d8be515454c5fccd28b/
  - libcec   : http://autobuild.buildroot.org/results/3f3/3f3593bab7734dd274faf5b5690895e9424cbb89/
  - and so on...

The bug was reported upstream [0], which dismissed it altogether [1] as
being expected behaviour, quoting:

    I don't think there is anything wrong with that change on its own.
    It merely exposed some existing behavior in a new case.

Instead, upstream suggested in that same message that a platform
definition be used instead, quoting:

    If a toolchain file specifies CMAKE_SYSTEM_NAME such that a custom
    `Platform/MySystem.cmake` file is loaded then the latter can set
    them as needed for the target platform.

So here we are doing so:

  - we add a new platfom definitions that inherits from the Linux one,
    then overrides the problematic settings;

  - we change our toolchain file to use that platform instead;

  - we tell cmake where to find additional modules, so that it can find
    our custom platform file.

This has been tested to work in the following conditions:

 - pre-installed host cmake, versions 3.5.1 (Ubuntu 16.04) and 3.7.2
   (manually built)

  - internal cmake, versions 3.6.3 (the current version as of this
    patch) and 3.7.2 (with the followup patches).

Thanks to Jörg, Ben and Baruch for the help investigating the issue.
Special thanks to Jörg for handling the discussion with upstream and
pointing to the relevant messages! :-)

[0] http://public.kitware.com/pipermail/cmake/2017-February/064970.html
[1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html

To be noted: Thomas suggested we set these directly in the toolchain
file. Unfortunately, wherever we put those settings in the toolchain
file, this does not work.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Jörg Krause <joerg.krause@embedded.rocks>
Cc: Ben Boeckel <mathstuf@gmail.com>
Cc: Baruch Siach <baruch@tkos.co.il>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Vicente Olivert Riera <Vincent.Riera@imgtec.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 package/pkg-cmake.mk                | 4 ++++
 support/misc/Buildroot.cmake        | 3 +++
 support/misc/toolchainfile.cmake.in | 2 +-
 toolchain/toolchain/toolchain.mk    | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)
 create mode 100644 support/misc/Buildroot.cmake

Comments

Baruch Siach Feb. 28, 2017, 6:26 p.m. UTC | #1
Hi Yann,

On Tue, Feb 28, 2017 at 07:07:22PM +0100, Yann E. MORIN wrote:
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index a7c8dc3..77fa1dc 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -87,6 +87,7 @@ define $(2)_CONFIGURE_CMDS
>  	PATH=$$(BR_PATH) \
>  	$$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
>  		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
> +		-DCMAKE_MODULE_PATH="$$(HOST_DIR)/usr/share/buildroot" \
>  		-DCMAKE_INSTALL_PREFIX="/usr" \
>  		-DCMAKE_COLOR_MAKEFILE=OFF \
>  		-DBUILD_DOC=OFF \
> @@ -249,3 +250,6 @@ $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
>  		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
>  		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
>  		> $@
> +
> +$(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake:
> +	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@)
> diff --git a/support/misc/Buildroot.cmake b/support/misc/Buildroot.cmake
> new file mode 100644
> index 0000000..b0046b3
> --- /dev/null
> +++ b/support/misc/Buildroot.cmake
> @@ -0,0 +1,3 @@
> +include(Platform/Linux)

So this goes to the host directory in case of host installed cmake? What 
happens when the host installed cmake is older than 3.1? Does the Buildroot 
built module path override the host installed one?

baruch

> +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
> +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)
> diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
> index d4252dd..5a42644 100644
> --- a/support/misc/toolchainfile.cmake.in
> +++ b/support/misc/toolchainfile.cmake.in
> @@ -10,7 +10,7 @@
>  # RELOCATED_HOST_DIR variable.
>  string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
>  
> -set(CMAKE_SYSTEM_NAME Linux)
> +set(CMAKE_SYSTEM_NAME Buildroot)
>  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
>  
>  # Set the {C,CXX}FLAGS appended by CMake depending on the build type
> diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
> index b16db01..e298373 100644
> --- a/toolchain/toolchain/toolchain.mk
> +++ b/toolchain/toolchain/toolchain.mk
> @@ -38,3 +38,4 @@ endif
>  $(eval $(virtual-package))
>  
>  toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
> +toolchain: $(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake
Yann E. MORIN Feb. 28, 2017, 8:03 p.m. UTC | #2
Baruch, All,

On 2017-02-28 20:26 +0200, Baruch Siach spake thusly:
> On Tue, Feb 28, 2017 at 07:07:22PM +0100, Yann E. MORIN wrote:
> > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> > index a7c8dc3..77fa1dc 100644
> > --- a/package/pkg-cmake.mk
> > +++ b/package/pkg-cmake.mk
> > @@ -87,6 +87,7 @@ define $(2)_CONFIGURE_CMDS
> >  	PATH=$$(BR_PATH) \
> >  	$$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
> >  		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
> > +		-DCMAKE_MODULE_PATH="$$(HOST_DIR)/usr/share/buildroot" \
> >  		-DCMAKE_INSTALL_PREFIX="/usr" \
> >  		-DCMAKE_COLOR_MAKEFILE=OFF \
> >  		-DBUILD_DOC=OFF \
> > @@ -249,3 +250,6 @@ $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
> >  		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
> >  		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
> >  		> $@
> > +
> > +$(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake:
> > +	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@)
> > diff --git a/support/misc/Buildroot.cmake b/support/misc/Buildroot.cmake
> > new file mode 100644
> > index 0000000..b0046b3
> > --- /dev/null
> > +++ b/support/misc/Buildroot.cmake
> > @@ -0,0 +1,3 @@
> > +include(Platform/Linux)
> 
> So this goes to the host directory in case of host installed cmake? What 
> happens when the host installed cmake is older than 3.1? Does the Buildroot 
> built module path override the host installed one?

When the host cmake is older than 3.1, we build and use our own cmake.

And the modules path we provide is looked for first, then cmake uses the
builtin one:
    https://cmake.org/cmake/help/v3.0/variable/CMAKE_MODULE_PATH.html

I should have written that in the commit log, indeed.

Regards,
Yann E. MORIN.

> baruch
> 
> > +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
> > +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)
> > diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
> > index d4252dd..5a42644 100644
> > --- a/support/misc/toolchainfile.cmake.in
> > +++ b/support/misc/toolchainfile.cmake.in
> > @@ -10,7 +10,7 @@
> >  # RELOCATED_HOST_DIR variable.
> >  string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
> >  
> > -set(CMAKE_SYSTEM_NAME Linux)
> > +set(CMAKE_SYSTEM_NAME Buildroot)
> >  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
> >  
> >  # Set the {C,CXX}FLAGS appended by CMake depending on the build type
> > diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
> > index b16db01..e298373 100644
> > --- a/toolchain/toolchain/toolchain.mk
> > +++ b/toolchain/toolchain/toolchain.mk
> > @@ -38,3 +38,4 @@ endif
> >  $(eval $(virtual-package))
> >  
> >  toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
> > +toolchain: $(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake
> 
> -- 
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Jörg Krause Feb. 28, 2017, 8:41 p.m. UTC | #3
Hi Yann,

On Tue, 2017-02-28 at 19:07 +0100, Yann E. MORIN wrote:
> The handling of RPATH in cmake-3.7 has changed drastically, causing a
> slew of build failures dues to libraries from the host being pulled
> in:
> 
>   - domoticz : http://autobuild.buildroot.org/results/fd0/fd0ba54c7ab
> f973691b39a0ca1bb4e07d749593a/
>   - freerdp  : http://autobuild.buildroot.org/results/5d4/5d429d0e288
> 754a541ee5d8be515454c5fccd28b/
>   - libcec   : http://autobuild.buildroot.org/results/3f3/3f3593bab77
> 34dd274faf5b5690895e9424cbb89/
>   - and so on...
> 
> The bug was reported upstream [0], which dismissed it altogether [1]
> as
> being expected behaviour, quoting:
> 
>     I don't think there is anything wrong with that change on its
> own.
>     It merely exposed some existing behavior in a new case.
> 
> Instead, upstream suggested in that same message that a platform
> definition be used instead, quoting:
> 
>     If a toolchain file specifies CMAKE_SYSTEM_NAME such that a
> custom
>     `Platform/MySystem.cmake` file is loaded then the latter can set
>     them as needed for the target platform.

Upstream has opened an issue [1] about this and is already working on a
fix [2] for version 3.7 and 3.8. It fixes the behavior of how
FIND_LIBRARY_USE_LIB32_PATHS handle symlinks. As 'lib32' is just just a
symlink to 'lib' in Buildroot, 'find_library()' will skip 'lib32' as a
runtime search path and as a consequence 'lib32' will not be used as
rpath anymore.

> So here we are doing so:
> 
>   - we add a new platfom definitions that inherits from the Linux
> one,
>     then overrides the problematic settings;
> 
>   - we change our toolchain file to use that platform instead;
> 
>   - we tell cmake where to find additional modules, so that it can
> find
>     our custom platform file.

As the issue might be fixed at some point, do we want to keep the
custom platform file?

> This has been tested to work in the following conditions:
> 
>  - pre-installed host cmake, versions 3.5.1 (Ubuntu 16.04) and 3.7.2
>    (manually built)
> 
>   - internal cmake, versions 3.6.3 (the current version as of this
>     patch) and 3.7.2 (with the followup patches).
> 
> Thanks to Jörg, Ben and Baruch for the help investigating the issue.
> Special thanks to Jörg for handling the discussion with upstream and
> pointing to the relevant messages! :-)

You are welcome!

[1] https://gitlab.kitware.com/cmake/cmake/issues/16682
[2] https://gitlab.kitware.com/cmake/cmake/merge_requests/534

Jörg
Yann E. MORIN Feb. 28, 2017, 8:58 p.m. UTC | #4
Jörg, All,

On 2017-02-28 21:41 +0100, Jörg Krause spake thusly:
> On Tue, 2017-02-28 at 19:07 +0100, Yann E. MORIN wrote:
> > The handling of RPATH in cmake-3.7 has changed drastically, causing a
> > slew of build failures dues to libraries from the host being pulled
> > in:
> > 
> >   - domoticz : http://autobuild.buildroot.org/results/fd0/fd0ba54c7ab
> > f973691b39a0ca1bb4e07d749593a/
> >   - freerdp  : http://autobuild.buildroot.org/results/5d4/5d429d0e288
> > 754a541ee5d8be515454c5fccd28b/
> >   - libcec   : http://autobuild.buildroot.org/results/3f3/3f3593bab77
> > 34dd274faf5b5690895e9424cbb89/
> >   - and so on...
> > 
> > The bug was reported upstream [0], which dismissed it altogether [1]
> > as
> > being expected behaviour, quoting:
> > 
> >     I don't think there is anything wrong with that change on its
> > own.
> >     It merely exposed some existing behavior in a new case.
> > 
> > Instead, upstream suggested in that same message that a platform
> > definition be used instead, quoting:
> > 
> >     If a toolchain file specifies CMAKE_SYSTEM_NAME such that a
> > custom
> >     `Platform/MySystem.cmake` file is loaded then the latter can set
> >     them as needed for the target platform.
> 
> Upstream has opened an issue [1] about this and is already working on a
> fix [2] for version 3.7 and 3.8. It fixes the behavior of how
> FIND_LIBRARY_USE_LIB32_PATHS handle symlinks. As 'lib32' is just just a
> symlink to 'lib' in Buildroot, 'find_library()' will skip 'lib32' as a
> runtime search path and as a consequence 'lib32' will not be used as
> rpath anymore.
> 
> > So here we are doing so:
> > 
> >   - we add a new platfom definitions that inherits from the Linux
> > one,
> >     then overrides the problematic settings;
> > 
> >   - we change our toolchain file to use that platform instead;
> > 
> >   - we tell cmake where to find additional modules, so that it can
> > find
> >     our custom platform file.
> 
> As the issue might be fixed at some point, do we want to keep the
> custom platform file?

Well, what bad would it do to have our own platform file? We could even
use it for yet more options if need be.

And in the meantime, there is no release of cmake with a fix. I'll
believe they've fixed it when I see it! ;-)

Regards,
Yann E. MORIN.

> > This has been tested to work in the following conditions:
> > 
> >  - pre-installed host cmake, versions 3.5.1 (Ubuntu 16.04) and 3.7.2
> >    (manually built)
> > 
> >   - internal cmake, versions 3.6.3 (the current version as of this
> >     patch) and 3.7.2 (with the followup patches).
> > 
> > Thanks to Jörg, Ben and Baruch for the help investigating the issue.
> > Special thanks to Jörg for handling the discussion with upstream and
> > pointing to the relevant messages! :-)
> 
> You are welcome!
> 
> [1] https://gitlab.kitware.com/cmake/cmake/issues/16682
> [2] https://gitlab.kitware.com/cmake/cmake/merge_requests/534
> 
> Jörg
Jörg Krause Feb. 28, 2017, 10:15 p.m. UTC | #5
Hi Yann,

On Tue, 2017-02-28 at 21:58 +0100, Yann E. MORIN wrote:
> Jörg, All,
> 
> On 2017-02-28 21:41 +0100, Jörg Krause spake thusly:
> > On Tue, 2017-02-28 at 19:07 +0100, Yann E. MORIN wrote:
> > > The handling of RPATH in cmake-3.7 has changed drastically,
> > > causing a
> > > slew of build failures dues to libraries from the host being
> > > pulled
> > > in:
> > > 
> > >   - domoticz : http://autobuild.buildroot.org/results/fd0/fd0ba54
> > > c7ab
> > > f973691b39a0ca1bb4e07d749593a/
> > >   - freerdp  : http://autobuild.buildroot.org/results/5d4/5d429d0
> > > e288
> > > 754a541ee5d8be515454c5fccd28b/
> > >   - libcec   : http://autobuild.buildroot.org/results/3f3/3f3593b
> > > ab77
> > > 34dd274faf5b5690895e9424cbb89/
> > >   - and so on...
> > > 
> > > The bug was reported upstream [0], which dismissed it altogether
> > > [1]
> > > as
> > > being expected behaviour, quoting:
> > > 
> > >     I don't think there is anything wrong with that change on its
> > > own.
> > >     It merely exposed some existing behavior in a new case.
> > > 
> > > Instead, upstream suggested in that same message that a platform
> > > definition be used instead, quoting:
> > > 
> > >     If a toolchain file specifies CMAKE_SYSTEM_NAME such that a
> > > custom
> > >     `Platform/MySystem.cmake` file is loaded then the latter can
> > > set
> > >     them as needed for the target platform.
> > 
> > Upstream has opened an issue [1] about this and is already working
> > on a
> > fix [2] for version 3.7 and 3.8. It fixes the behavior of how
> > FIND_LIBRARY_USE_LIB32_PATHS handle symlinks. As 'lib32' is just
> > just a
> > symlink to 'lib' in Buildroot, 'find_library()' will skip 'lib32'
> > as a
> > runtime search path and as a consequence 'lib32' will not be used
> > as
> > rpath anymore.
> > 
> > > So here we are doing so:
> > > 
> > >   - we add a new platfom definitions that inherits from the Linux
> > > one,
> > >     then overrides the problematic settings;
> > > 
> > >   - we change our toolchain file to use that platform instead;
> > > 
> > >   - we tell cmake where to find additional modules, so that it
> > > can
> > > find
> > >     our custom platform file.
> > 
> > As the issue might be fixed at some point, do we want to keep the
> > custom platform file?
> 
> Well, what bad would it do to have our own platform file? We could
> even
> use it for yet more options if need be.

No offense. There is nothing wrong about having a custom buildroot
platform file.

> And in the meantime, there is no release of cmake with a fix. I'll
> believe they've fixed it when I see it! ;-)

:-)

Jörg
Arnout Vandecappelle March 1, 2017, 7:58 a.m. UTC | #6
On 28-02-17 19:07, Yann E. MORIN wrote:
> The handling of RPATH in cmake-3.7 has changed drastically, causing a
> slew of build failures dues to libraries from the host being pulled in:
[snip]
>   - we add a new platfom definitions that inherits from the Linux one,
>     then overrides the problematic settings;

 Not mentioned in the commit log but important to note: this platform file is
only used for target CMake packages, not host packages.

[snip]
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index a7c8dc3..77fa1dc 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -87,6 +87,7 @@ define $(2)_CONFIGURE_CMDS
>  	PATH=$$(BR_PATH) \
>  	$$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
>  		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
> +		-DCMAKE_MODULE_PATH="$$(HOST_DIR)/usr/share/buildroot" \

 Since you don't do any processing on the file, you could just use

		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \

and avoid copying the file around. Of course, in that case, support/cmake is
probably a better place to put the file :-)

 On the other hand, in the future we may want to add other stuff to the cmake
module path, and that stuff might get processed. I don't really like adding
complexity for some unknown future use, but it does make some kind of sense.

 If you do copy the file, it should IMHO go into STAGING_DIR, not HOST_DIR. I
think the location of toolchainfile.cmake is incorrect as well. We briefly
discussed this at some BR developer meeting IIRC but nobody took action on it.

 Regards,
 Arnout

>  		-DCMAKE_INSTALL_PREFIX="/usr" \
>  		-DCMAKE_COLOR_MAKEFILE=OFF \
>  		-DBUILD_DOC=OFF \
> @@ -249,3 +250,6 @@ $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
>  		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
>  		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
>  		> $@
> +
> +$(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake:
> +	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@)
> diff --git a/support/misc/Buildroot.cmake b/support/misc/Buildroot.cmake
> new file mode 100644
> index 0000000..b0046b3
> --- /dev/null
> +++ b/support/misc/Buildroot.cmake
> @@ -0,0 +1,3 @@
> +include(Platform/Linux)
> +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
> +set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)
> diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
> index d4252dd..5a42644 100644
> --- a/support/misc/toolchainfile.cmake.in
> +++ b/support/misc/toolchainfile.cmake.in
> @@ -10,7 +10,7 @@
>  # RELOCATED_HOST_DIR variable.
>  string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
>  
> -set(CMAKE_SYSTEM_NAME Linux)
> +set(CMAKE_SYSTEM_NAME Buildroot)
>  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
>  
>  # Set the {C,CXX}FLAGS appended by CMake depending on the build type
> diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
> index b16db01..e298373 100644
> --- a/toolchain/toolchain/toolchain.mk
> +++ b/toolchain/toolchain/toolchain.mk
> @@ -38,3 +38,4 @@ endif
>  $(eval $(virtual-package))
>  
>  toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
> +toolchain: $(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake
>
Arnout Vandecappelle March 1, 2017, 8:01 a.m. UTC | #7
On 28-02-17 21:58, Yann E. MORIN wrote:
> Jörg, All,
> 
> On 2017-02-28 21:41 +0100, Jörg Krause spake thusly:
>> On Tue, 2017-02-28 at 19:07 +0100, Yann E. MORIN wrote:
>>> The handling of RPATH in cmake-3.7 has changed drastically, causing a
>>> slew of build failures dues to libraries from the host being pulled
>>> in:
>>>
>>>   - domoticz : http://autobuild.buildroot.org/results/fd0/fd0ba54c7ab
>>> f973691b39a0ca1bb4e07d749593a/
>>>   - freerdp  : http://autobuild.buildroot.org/results/5d4/5d429d0e288
>>> 754a541ee5d8be515454c5fccd28b/
>>>   - libcec   : http://autobuild.buildroot.org/results/3f3/3f3593bab77
>>> 34dd274faf5b5690895e9424cbb89/
>>>   - and so on...
>>>
>>> The bug was reported upstream [0], which dismissed it altogether [1]
>>> as
>>> being expected behaviour, quoting:
>>>
>>>     I don't think there is anything wrong with that change on its
>>> own.
>>>     It merely exposed some existing behavior in a new case.
>>>
>>> Instead, upstream suggested in that same message that a platform
>>> definition be used instead, quoting:
>>>
>>>     If a toolchain file specifies CMAKE_SYSTEM_NAME such that a
>>> custom
>>>     `Platform/MySystem.cmake` file is loaded then the latter can set
>>>     them as needed for the target platform.
>>
>> Upstream has opened an issue [1] about this and is already working on a
>> fix [2] for version 3.7 and 3.8. It fixes the behavior of how
>> FIND_LIBRARY_USE_LIB32_PATHS handle symlinks. As 'lib32' is just just a
>> symlink to 'lib' in Buildroot, 'find_library()' will skip 'lib32' as a
>> runtime search path and as a consequence 'lib32' will not be used as
>> rpath anymore.
>>
>>> So here we are doing so:
>>>
>>>   - we add a new platfom definitions that inherits from the Linux
>>> one,
>>>     then overrides the problematic settings;
>>>
>>>   - we change our toolchain file to use that platform instead;
>>>
>>>   - we tell cmake where to find additional modules, so that it can
>>> find
>>>     our custom platform file.
>>
>> As the issue might be fixed at some point, do we want to keep the
>> custom platform file?
> 
> Well, what bad would it do to have our own platform file? We could even
> use it for yet more options if need be.
> 
> And in the meantime, there is no release of cmake with a fix. I'll
> believe they've fixed it when I see it! ;-)

 Also, we want to support existing distro CMake, which doesn't have this fix.

 Which reminds me: don't forget to revert
4422eca2d4 dependencies/cmake: blacklist cmake 3.7


 Regards,
 Arnout

[snip]
Thomas Petazzoni March 1, 2017, 8:17 a.m. UTC | #8
Hello,

On Wed, 1 Mar 2017 08:58:31 +0100, Arnout Vandecappelle wrote:

>  Since you don't do any processing on the file, you could just use
> 
> 		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \

I think the idea is that since the Buildroot.cmake platform files gets
referenced by the toolchainfile.cmake, then Buildroot.cmake anyway
needs to be installed in $(HOST_DIR)/usr/share/buildroot, so we can
just as well point CMake to this place.

> and avoid copying the file around. Of course, in that case, support/cmake is
> probably a better place to put the file :-)

No: copying the file is needed, otherwise our toolchainfile.cmake file
no longer works for people using it outside of Buildroot.

Thomas
Arnout Vandecappelle March 1, 2017, 8:35 a.m. UTC | #9
On 01-03-17 09:17, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 1 Mar 2017 08:58:31 +0100, Arnout Vandecappelle wrote:
> 
>>  Since you don't do any processing on the file, you could just use
>>
>> 		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \
> 
> I think the idea is that since the Buildroot.cmake platform files gets
> referenced by the toolchainfile.cmake, then Buildroot.cmake anyway
> needs to be installed in $(HOST_DIR)/usr/share/buildroot, so we can
> just as well point CMake to this place.
> 
>> and avoid copying the file around. Of course, in that case, support/cmake is
>> probably a better place to put the file :-)
> 
> No: copying the file is needed, otherwise our toolchainfile.cmake file
> no longer works for people using it outside of Buildroot.

 Good point. I still think it should be in staging rather than host, but for
sure it's more important to keep them together.

 Therefore:

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout
Yann E. MORIN March 1, 2017, 6:43 p.m. UTC | #10
Arnout, All,

On 2017-03-01 09:35 +0100, Arnout Vandecappelle spake thusly:
> On 01-03-17 09:17, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Wed, 1 Mar 2017 08:58:31 +0100, Arnout Vandecappelle wrote:
> > 
> >>  Since you don't do any processing on the file, you could just use
> >>
> >> 		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \
> > 
> > I think the idea is that since the Buildroot.cmake platform files gets
> > referenced by the toolchainfile.cmake, then Buildroot.cmake anyway
> > needs to be installed in $(HOST_DIR)/usr/share/buildroot, so we can
> > just as well point CMake to this place.
> > 
> >> and avoid copying the file around. Of course, in that case, support/cmake is
> >> probably a better place to put the file :-)
> > 
> > No: copying the file is needed, otherwise our toolchainfile.cmake file
> > no longer works for people using it outside of Buildroot.
> 
>  Good point. I still think it should be in staging rather than host, but for
> sure it's more important to keep them together.

Why would you want it in staging? When we build our own internal cmake,
the moduels it installs are so installed in host, not staging. They are
build-description files, not target files.

Ergo, they should be in host, not staging.

Regards,
Yann E. MORIN.

>  Therefore:
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  Regards,
>  Arnout
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
Baruch Siach March 2, 2017, 12:29 p.m. UTC | #11
Hi Yann,

On Tue, Feb 28, 2017 at 07:07:22PM +0100, Yann E. MORIN wrote:
> -set(CMAKE_SYSTEM_NAME Linux)
> +set(CMAKE_SYSTEM_NAME Buildroot)
>  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)

This breaks libiio.

http://autobuild.buildroot.net/results/281/2812b008a0ab6bab5fe4d45eb9ffe4e9496a8cb4/

/home/dawncrow/buildroot-test/scripts/instance-0/output/build/libiio-0.9/local.c: In function 'local_open':
/home/dawncrow/buildroot-test/scripts/instance-0/output/build/libiio-0.9/local.c:856:33: error: 'O_CLOEXEC' undeclared (first use in this function)

That is because the libiio CMakeLists.txt has the following code:

IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")

		[...]

        set(CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE=1")
        add_definitions(-D_GNU_SOURCE=1)
endif()

Now cmake skips this code, thus omitting _GNU_SOURCE which leads to the build 
failure.

baruch
Thomas Petazzoni March 2, 2017, 12:41 p.m. UTC | #12
Hello,

On Thu, 2 Mar 2017 14:29:40 +0200, Baruch Siach wrote:

> On Tue, Feb 28, 2017 at 07:07:22PM +0100, Yann E. MORIN wrote:
> > -set(CMAKE_SYSTEM_NAME Linux)
> > +set(CMAKE_SYSTEM_NAME Buildroot)
> >  set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)  
> 
> This breaks libiio.
> 
> http://autobuild.buildroot.net/results/281/2812b008a0ab6bab5fe4d45eb9ffe4e9496a8cb4/
> 
> /home/dawncrow/buildroot-test/scripts/instance-0/output/build/libiio-0.9/local.c: In function 'local_open':
> /home/dawncrow/buildroot-test/scripts/instance-0/output/build/libiio-0.9/local.c:856:33: error: 'O_CLOEXEC' undeclared (first use in this function)
> 
> That is because the libiio CMakeLists.txt has the following code:
> 
> IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
> 
> 		[...]
> 
>         set(CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE=1")
>         add_definitions(-D_GNU_SOURCE=1)
> endif()
> 
> Now cmake skips this code, thus omitting _GNU_SOURCE which leads to the build 
> failure.

As I reported earlier today, this series breaks several other CMake
based packages. So I guess, we're good for yet another revert, back to
CMake 3.6.

Thomas
Arnout Vandecappelle March 5, 2017, 11:27 p.m. UTC | #13
On 01-03-17 19:43, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-03-01 09:35 +0100, Arnout Vandecappelle spake thusly:
>> On 01-03-17 09:17, Thomas Petazzoni wrote:
>>> Hello,
>>>
>>> On Wed, 1 Mar 2017 08:58:31 +0100, Arnout Vandecappelle wrote:
>>>
>>>>  Since you don't do any processing on the file, you could just use
>>>>
>>>> 		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \
>>>
>>> I think the idea is that since the Buildroot.cmake platform files gets
>>> referenced by the toolchainfile.cmake, then Buildroot.cmake anyway
>>> needs to be installed in $(HOST_DIR)/usr/share/buildroot, so we can
>>> just as well point CMake to this place.
>>>
>>>> and avoid copying the file around. Of course, in that case, support/cmake is
>>>> probably a better place to put the file :-)
>>>
>>> No: copying the file is needed, otherwise our toolchainfile.cmake file
>>> no longer works for people using it outside of Buildroot.
>>
>>  Good point. I still think it should be in staging rather than host, but for
>> sure it's more important to keep them together.
> 
> Why would you want it in staging? When we build our own internal cmake,
> the moduels it installs are so installed in host, not staging. They are
> build-description files, not target files.
> 
> Ergo, they should be in host, not staging.

 I was looking at it like the .pc files, which are in staging because they talk
about the target.

 About the modules: in fact, packages that install .cmake files will do that in
staging, not in host. So I do hope that cmake looks for them in staging as
well... That's what CMAKE_FIND_ROOT_PATH is for, isn't it?

 In fact, the only .cmake files I see installed in the host dir are:

- the Modules from CMake itself, but these are more related to the CMake
language, e.g. CheckIncludeFiles.cmake;

- the CMake files added by *host* packages;

- the CMake files added by buildroot which talk about *target* packages.

 So I stand by my original statement: I think the cmake files should move from
HOST/usr/share/buildroot to STAGING/usr/lib/cmake.

 Adding Samuel and Luca, our CMake "experts", to Cc.

 Regards,
 Arnout


> 
> Regards,
> Yann E. MORIN.
> 
>>  Therefore:
>>
>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>>  Regards,
>>  Arnout
>>
>> -- 
>> Arnout Vandecappelle                          arnout at mind be
>> Senior Embedded Software Architect            +32-16-286500
>> Essensium/Mind                                http://www.mind.be
>> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
>> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
>> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
>
Yann E. MORIN March 6, 2017, 5 p.m. UTC | #14
Arnout, All,

On 2017-03-06 00:27 +0100, Arnout Vandecappelle spake thusly:
> On 01-03-17 19:43, Yann E. MORIN wrote:
> > Arnout, All,
> > 
> > On 2017-03-01 09:35 +0100, Arnout Vandecappelle spake thusly:
> >> On 01-03-17 09:17, Thomas Petazzoni wrote:
> >>> Hello,
> >>>
> >>> On Wed, 1 Mar 2017 08:58:31 +0100, Arnout Vandecappelle wrote:
> >>>
> >>>>  Since you don't do any processing on the file, you could just use
> >>>>
> >>>> 		-DCMAKE_MODULE_PATH="$$(TOPDIR)/support/misc" \
> >>>
> >>> I think the idea is that since the Buildroot.cmake platform files gets
> >>> referenced by the toolchainfile.cmake, then Buildroot.cmake anyway
> >>> needs to be installed in $(HOST_DIR)/usr/share/buildroot, so we can
> >>> just as well point CMake to this place.
> >>>
> >>>> and avoid copying the file around. Of course, in that case, support/cmake is
> >>>> probably a better place to put the file :-)
> >>>
> >>> No: copying the file is needed, otherwise our toolchainfile.cmake file
> >>> no longer works for people using it outside of Buildroot.
> >>
> >>  Good point. I still think it should be in staging rather than host, but for
> >> sure it's more important to keep them together.
> > 
> > Why would you want it in staging? When we build our own internal cmake,
> > the moduels it installs are so installed in host, not staging. They are
> > build-description files, not target files.
> > 
> > Ergo, they should be in host, not staging.
> 
>  I was looking at it like the .pc files, which are in staging because they talk
> about the target.

But it is not like a .pc file.  It does not describe libs to link with
(-l), or where to lok for headers and libs (-I and -L).

It is more like the toolchain file itself; but see below...

>  About the modules: in fact, packages that install .cmake files will do that in
> staging, not in host. So I do hope that cmake looks for them in staging as
> well... That's what CMAKE_FIND_ROOT_PATH is for, isn't it?
> 
>  In fact, the only .cmake files I see installed in the host dir are:
> 
> - the Modules from CMake itself, but these are more related to the CMake
> language, e.g. CheckIncludeFiles.cmake;

And what about all the modules describing platforms:

    host/usr/share/cmake-3.7/Modules/Platform/

and the file that actually sources the platform description:

    host/usr/share/cmake-3.7/Modules/CMakeSystemSpecificInformation.cmake

> - the CMake files added by *host* packages;
> 
> - the CMake files added by buildroot which talk about *target* packages.
> 
>  So I stand by my original statement: I think the cmake files should move from
> HOST/usr/share/buildroot to STAGING/usr/lib/cmake.

Still not convinced... ;-)

Regards,
Yann E. MORIN.
Arnout Vandecappelle March 6, 2017, 10:02 p.m. UTC | #15
On 06-03-17 18:00, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2017-03-06 00:27 +0100, Arnout Vandecappelle spake thusly:
>> On 01-03-17 19:43, Yann E. MORIN wrote:
[snip]
>>> Why would you want it in staging? When we build our own internal cmake,
>>> the moduels it installs are so installed in host, not staging. They are
>>> build-description files, not target files.
>>>
>>> Ergo, they should be in host, not staging.
>>
>>  I was looking at it like the .pc files, which are in staging because they talk
>> about the target.
> 
> But it is not like a .pc file.  It does not describe libs to link with
> (-l), or where to lok for headers and libs (-I and -L).

 How is the specification of the sysroot different from the path to headers and
libs?


> It is more like the toolchain file itself; but see below...

 If with the toolchain file you mean toolchainfile.cmake, that's exactly my
point: I think toolchainfile.cmake should be in staging.


>>  About the modules: in fact, packages that install .cmake files will do that in
>> staging, not in host. So I do hope that cmake looks for them in staging as
>> well... That's what CMAKE_FIND_ROOT_PATH is for, isn't it?
>>
>>  In fact, the only .cmake files I see installed in the host dir are:
>>
>> - the Modules from CMake itself, but these are more related to the CMake
>> language, e.g. CheckIncludeFiles.cmake;
> 
> And what about all the modules describing platforms:
> 
>     host/usr/share/cmake-3.7/Modules/Platform/

 They are not directly related to the target. They contain all the possible
platforms.


> and the file that actually sources the platform description:
> 
>     host/usr/share/cmake-3.7/Modules/CMakeSystemSpecificInformation.cmake

 Note that there is nothing system specific in this file :-)

 Anyway, enough bikeshedding, let's just keep things as they are.

 Regards,
 Arnout

> 
>> - the CMake files added by *host* packages;
>>
>> - the CMake files added by buildroot which talk about *target* packages.
>>
>>  So I stand by my original statement: I think the cmake files should move from
>> HOST/usr/share/buildroot to STAGING/usr/lib/cmake.
> 
> Still not convinced... ;-)
> 
> Regards,
> Yann E. MORIN.
>
diff mbox

Patch

diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index a7c8dc3..77fa1dc 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -87,6 +87,7 @@  define $(2)_CONFIGURE_CMDS
 	PATH=$$(BR_PATH) \
 	$$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \
 		-DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
+		-DCMAKE_MODULE_PATH="$$(HOST_DIR)/usr/share/buildroot" \
 		-DCMAKE_INSTALL_PREFIX="/usr" \
 		-DCMAKE_COLOR_MAKEFILE=OFF \
 		-DBUILD_DOC=OFF \
@@ -249,3 +250,6 @@  $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
 		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
 		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
 		> $@
+
+$(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake:
+	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake $(@)
diff --git a/support/misc/Buildroot.cmake b/support/misc/Buildroot.cmake
new file mode 100644
index 0000000..b0046b3
--- /dev/null
+++ b/support/misc/Buildroot.cmake
@@ -0,0 +1,3 @@ 
+include(Platform/Linux)
+set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
+set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)
diff --git a/support/misc/toolchainfile.cmake.in b/support/misc/toolchainfile.cmake.in
index d4252dd..5a42644 100644
--- a/support/misc/toolchainfile.cmake.in
+++ b/support/misc/toolchainfile.cmake.in
@@ -10,7 +10,7 @@ 
 # RELOCATED_HOST_DIR variable.
 string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
 
-set(CMAKE_SYSTEM_NAME Linux)
+set(CMAKE_SYSTEM_NAME Buildroot)
 set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
 
 # Set the {C,CXX}FLAGS appended by CMake depending on the build type
diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
index b16db01..e298373 100644
--- a/toolchain/toolchain/toolchain.mk
+++ b/toolchain/toolchain/toolchain.mk
@@ -38,3 +38,4 @@  endif
 $(eval $(virtual-package))
 
 toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
+toolchain: $(HOST_DIR)/usr/share/buildroot/Platform/Buildroot.cmake