diff mbox

dependencies/cmake: blacklist cmake 3.7

Message ID 1488148967-8055-1-git-send-email-yann.morin.1998@free.fr
State Accepted
Commit 4422eca2d418e2b817b419ff6c4c62f7f4cb7871
Headers show

Commit Message

Yann E. MORIN Feb. 26, 2017, 10:42 p.m. UTC
cmake-3.7 has a bug in how it handles rpath, linking with libraries from
the host.

Until we completely understand the issue, just blacklist cmake-3.7.

The issue has been reported upstream:
    http://public.kitware.com/pipermail/cmake/2017-February/064970.html

Reported-by: Baruch Siach <baruch@tkos.co.il>
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: Samuel Martin <s.martin49@gmail.com>
---
 support/dependencies/check-host-cmake.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Baruch Siach Feb. 27, 2017, 4:43 a.m. UTC | #1
Hi Yann,

On Sun, Feb 26, 2017 at 11:42:47PM +0100, Yann E. MORIN wrote:
> cmake-3.7 has a bug in how it handles rpath, linking with libraries from
> the host.
> 
> Until we completely understand the issue, just blacklist cmake-3.7.
> 
> The issue has been reported upstream:
>     http://public.kitware.com/pipermail/cmake/2017-February/064970.html
> 
> Reported-by: Baruch Siach <baruch@tkos.co.il>
> 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: Samuel Martin <s.martin49@gmail.com>
> ---
>  support/dependencies/check-host-cmake.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
> index 9b63b06..84c26c2 100755
> --- a/support/dependencies/check-host-cmake.sh
> +++ b/support/dependencies/check-host-cmake.sh
> @@ -6,6 +6,9 @@ version_min="${2}"
>  major_min="${version_min%.*}"
>  minor_min="${version_min#*.}"
>  
> +# cmake-3.7 incorrectly handles rpath, linking to host libraries
> +blacklist_version="3.7"
> +
>  cmake=`which ${candidate}`
>  if [ ! -x "${cmake}" ]; then
>      # echo nothing: no suitable cmake found
> @@ -27,6 +30,11 @@ version="$(${cmake} --version \
>  major="${version%.*}"
>  minor="${version#*.}"
>  
> +if [ "${version}" = "${blacklist_version}" ]; then

Minor nit: there are no quotes around version variables in the rest of this 
file.

> +    # echo nothing: no suitable cmake found
> +    exit 1
> +fi
> +
>  if [ ${major} -gt ${major_min} ]; then

E.g., here.

>      echo "${cmake}"
>  else

baruch
Yann E. MORIN Feb. 27, 2017, 3:51 p.m. UTC | #2
Baruch, All,

On 2017-02-27 06:43 +0200, Baruch Siach spake thusly:
> Hi Yann,
> 
> On Sun, Feb 26, 2017 at 11:42:47PM +0100, Yann E. MORIN wrote:
> > cmake-3.7 has a bug in how it handles rpath, linking with libraries from
> > the host.
> > 
> > Until we completely understand the issue, just blacklist cmake-3.7.
> > 
> > The issue has been reported upstream:
> >     http://public.kitware.com/pipermail/cmake/2017-February/064970.html
> > 
> > Reported-by: Baruch Siach <baruch@tkos.co.il>
> > 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: Samuel Martin <s.martin49@gmail.com>
> > ---
> >  support/dependencies/check-host-cmake.sh | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
> > index 9b63b06..84c26c2 100755
> > --- a/support/dependencies/check-host-cmake.sh
> > +++ b/support/dependencies/check-host-cmake.sh
> > @@ -6,6 +6,9 @@ version_min="${2}"
> >  major_min="${version_min%.*}"
> >  minor_min="${version_min#*.}"
> >  
> > +# cmake-3.7 incorrectly handles rpath, linking to host libraries
> > +blacklist_version="3.7"
> > +
> >  cmake=`which ${candidate}`
> >  if [ ! -x "${cmake}" ]; then
> >      # echo nothing: no suitable cmake found
> > @@ -27,6 +30,11 @@ version="$(${cmake} --version \
> >  major="${version%.*}"
> >  minor="${version#*.}"
> >  
> > +if [ "${version}" = "${blacklist_version}" ]; then
> 
> Minor nit: there are no quotes around version variables in the rest of this 
> file.

That's because here we are testing strings, while...

> > +    # echo nothing: no suitable cmake found
> > +    exit 1
> > +fi
> > +
> >  if [ ${major} -gt ${major_min} ]; then
> 
> E.g., here.

... here we are testing numbers.

Regards,
Yann E. MORIN.

> >      echo "${cmake}"
> >  else
> 
> baruch
> 
> -- 
>      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. 27, 2017, 5:12 p.m. UTC | #3
Hi all,

On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> cmake-3.7 has a bug in how it handles rpath, linking with libraries
> from
> the host.
> 
> Until we completely understand the issue, just blacklist cmake-3.7.
> 
> The issue has been reported upstream:
>     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> ml

Brad King from Kitware replied today [1]. In short, Brad does not think
there anything wrong about handling the rpath and supposes to load a
custom platform cmake file instead of the Linux one.

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

Jörg
Yann E. MORIN Feb. 27, 2017, 5:25 p.m. UTC | #4
Jörg, All,

Thanks for the follow-up! :-)

On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > cmake-3.7 has a bug in how it handles rpath, linking with libraries
> > from
> > the host.
> > 
> > Until we completely understand the issue, just blacklist cmake-3.7.
> > 
> > The issue has been reported upstream:
> >     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> > ml
> 
> Brad King from Kitware replied today [1]. In short, Brad does not think
> there anything wrong about handling the rpath and supposes to load a
> custom platform cmake file instead of the Linux one.
> 
> [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html

OK, so what we would have to do (basically):

  - copy Modules/Platform/Linux.cmake to Modules/Platform/Buildroot.cmake

  - tweak that file so that the two settings (lib32 and lib64) are now
    FALSE in that file

  - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)

and we'd be all good?

Or alternatively:

  - add Modules/Platform/Buildroot.cmake, which:
    - includes Modules/Platform/Linux.cmake
    - sets the the two settings (lib32 and lib64) to FALSE

  - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)

Thoughts?

Regards,
Yann E. MORIN.
Baruch Siach Feb. 27, 2017, 5:31 p.m. UTC | #5
Hi Yann,

On Mon, Feb 27, 2017 at 06:25:40PM +0100, Yann E. MORIN wrote:
> On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > cmake-3.7 has a bug in how it handles rpath, linking with libraries
> > > from
> > > the host.
> > > 
> > > Until we completely understand the issue, just blacklist cmake-3.7.
> > > 
> > > The issue has been reported upstream:
> > >     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> > > ml
> > 
> > Brad King from Kitware replied today [1]. In short, Brad does not think
> > there anything wrong about handling the rpath and supposes to load a
> > custom platform cmake file instead of the Linux one.
> > 
> > [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html
> 
> OK, so what we would have to do (basically):
> 
>   - copy Modules/Platform/Linux.cmake to Modules/Platform/Buildroot.cmake
> 
>   - tweak that file so that the two settings (lib32 and lib64) are now
>     FALSE in that file
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> 
> and we'd be all good?
> 
> Or alternatively:
> 
>   - add Modules/Platform/Buildroot.cmake, which:
>     - includes Modules/Platform/Linux.cmake
>     - sets the the two settings (lib32 and lib64) to FALSE
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> 
> Thoughts?

Again, what about host installed cmake? Can we set lib{32,64} to FALSE 
directly in toolchain.cmake?

baruch
Yann E. MORIN Feb. 27, 2017, 5:41 p.m. UTC | #6
Baruch, All,

On 2017-02-27 19:31 +0200, Baruch Siach spake thusly:
> On Mon, Feb 27, 2017 at 06:25:40PM +0100, Yann E. MORIN wrote:
> > On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > > cmake-3.7 has a bug in how it handles rpath, linking with libraries
> > > > from
> > > > the host.
> > > > 
> > > > Until we completely understand the issue, just blacklist cmake-3.7.
> > > > 
> > > > The issue has been reported upstream:
> > > >     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> > > > ml
> > > 
> > > Brad King from Kitware replied today [1]. In short, Brad does not think
> > > there anything wrong about handling the rpath and supposes to load a
> > > custom platform cmake file instead of the Linux one.
> > > 
> > > [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html
> > 
> > OK, so what we would have to do (basically):
> > 
> >   - copy Modules/Platform/Linux.cmake to Modules/Platform/Buildroot.cmake
> > 
> >   - tweak that file so that the two settings (lib32 and lib64) are now
> >     FALSE in that file
> > 
> >   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> > 
> > and we'd be all good?
> > 
> > Or alternatively:
> > 
> >   - add Modules/Platform/Buildroot.cmake, which:
> >     - includes Modules/Platform/Linux.cmake
> >     - sets the the two settings (lib32 and lib64) to FALSE
> > 
> >   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> > 
> > Thoughts?
> 
> Again, what about host installed cmake?

And this is exactly the kind of reply I expected! ;-)

> Can we set lib{32,64} to FALSE 
> directly in toolchain.cmake?

I don't think so...

The toolchainfile.cmake sets CMAKE_SYSTEM_NAME, which is used to find
the appropriate SystemName module. It means the SystemName.cmake file
can only be parsed after tolchainfile.cmake is.

So, anything we set in toolchainfile.cmake would be overriden by the
settings in the SystemName.cmake file.

Ergo, we're screwed in this case...

Unless we can tell cmake to look for modules in alternate locations.

Regards,
Yann E. MORIN.
Yann E. MORIN Feb. 27, 2017, 6:30 p.m. UTC | #7
Jörg, All,

On 2017-02-27 18:25 +0100, Yann E. MORIN spake thusly:
> On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > cmake-3.7 has a bug in how it handles rpath, linking with libraries
> > > from
> > > the host.
> > > 
> > > Until we completely understand the issue, just blacklist cmake-3.7.
> > > 
> > > The issue has been reported upstream:
> > >     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> > > ml
> > 
> > Brad King from Kitware replied today [1]. In short, Brad does not think
> > there anything wrong about handling the rpath and supposes to load a
> > custom platform cmake file instead of the Linux one.
> > 
> > [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html
> 
> OK, so what we would have to do (basically):
> 
>   - copy Modules/Platform/Linux.cmake to Modules/Platform/Buildroot.cmake
> 
>   - tweak that file so that the two settings (lib32 and lib64) are now
>     FALSE in that file
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> 
> and we'd be all good?
> 
> Or alternatively:
> 
>   - add Modules/Platform/Buildroot.cmake, which:
>     - includes Modules/Platform/Linux.cmake
>     - sets the the two settings (lib32 and lib64) to FALSE
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)

So I tested that last solution, and it indeed fixes the build. Woot!

Of course, this is only for when we build out own cmake, not when we use
the host pre-installed one. I'll try to see tonight if we can do similar
for it, too.

Thanks for the hints! :-)

Regards,
Yann E. MORIN.
Ben Boeckel Feb. 27, 2017, 6:35 p.m. UTC | #8
On Mon, Feb 27, 2017 at 19:30:16 +0100, Yann E. MORIN wrote:
> Of course, this is only for when we build out own cmake, not when we use
> the host pre-installed one. I'll try to see tonight if we can do similar
> for it, too.

It may make sense to support this upstream. CMake already has platform
files for things such as Cray and Android (among others) which are just
special kinds of Linux, like Buildroot.

How often does that toolchain file change? Or does it depend on
Buildroot settings?

--Ben
Jörg Krause Feb. 27, 2017, 8:04 p.m. UTC | #9
On Mon, 2017-02-27 at 18:25 +0100, Yann E. MORIN wrote:
> Jörg, All,
> 
> Thanks for the follow-up! :-)
> 
> On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > cmake-3.7 has a bug in how it handles rpath, linking with
> > > libraries
> > > from
> > > the host.
> > > 
> > > Until we completely understand the issue, just blacklist cmake-
> > > 3.7.
> > > 
> > > The issue has been reported upstream:
> > >     http://public.kitware.com/pipermail/cmake/2017-February/06497
> > > 0.ht
> > > ml
> > 
> > Brad King from Kitware replied today [1]. In short, Brad does not
> > think
> > there anything wrong about handling the rpath and supposes to load
> > a
> > custom platform cmake file instead of the Linux one.
> > 
> > [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.
> > html
> 
> OK, so what we would have to do (basically):
> 
>   - copy Modules/Platform/Linux.cmake to
> Modules/Platform/Buildroot.cmake
> 
>   - tweak that file so that the two settings (lib32 and lib64) are
> now
>     FALSE in that file
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME
> Buildroot)
> 
> and we'd be all good?
> 
> Or alternatively:
> 
>   - add Modules/Platform/Buildroot.cmake, which:
>     - includes Modules/Platform/Linux.cmake
>     - sets the the two settings (lib32 and lib64) to FALSE
> 
>   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME
> Buildroot)
> 
> Thoughts?

I still think this is a bug! A host rpath should not be used when
cross-compiling whether lib32 is used or not. Somehow, it feels weird
to say that Buildroot is not a Linux platform, in the sense of CMake.

I will reply my thoughts to the CMake mailing list. Lets see what they
suppose.

Jörg
Ben Boeckel Feb. 27, 2017, 8:08 p.m. UTC | #10
On Mon, Feb 27, 2017 at 21:04:06 +0100, Jörg Krause wrote:
> I still think this is a bug! A host rpath should not be used when
> cross-compiling whether lib32 is used or not. Somehow, it feels weird
> to say that Buildroot is not a Linux platform, in the sense of CMake.

There's the chance that this was always a problem, but is now being
exposed by this. Where is the lib32 rpath coming from? CMake doesn't add
rpaths by default, so it has to be coming from somewhere. Is it added
manually somewhere? Is a library that used to be found in the sysroot
now being found on the host? If so, *that's* the bug which should be
fixed. Given how widespread it is, maybe something is set up incorrectly
in the toolchain file?

This flag is the same logic as lib64, just with a different suffix. Why
is the problem not surfacing on Red Hat platforms which use lib64?

--Ben
Jörg Krause Feb. 27, 2017, 8:24 p.m. UTC | #11
Hi Ben,

On Mon, 2017-02-27 at 15:08 -0500, Ben Boeckel wrote:
> On Mon, Feb 27, 2017 at 21:04:06 +0100, Jörg Krause wrote:
> > I still think this is a bug! A host rpath should not be used when
> > cross-compiling whether lib32 is used or not. Somehow, it feels
> > weird
> > to say that Buildroot is not a Linux platform, in the sense of
> > CMake.
> 
> There's the chance that this was always a problem, but is now being
> exposed by this. Where is the lib32 rpath coming from? CMake doesn't
> add
> rpaths by default, so it has to be coming from somewhere. Is it added
> manually somewhere? Is a library that used to be found in the sysroot
> now being found on the host? If so, *that's* the bug which should be
> fixed. Given how widespread it is, maybe something is set up
> incorrectly
> in the toolchain file?
> 
> This flag is the same logic as lib64, just with a different suffix.
> Why
> is the problem not surfacing on Red Hat platforms which use lib64?

For a detailed description, please have a look at my post [1] on the
Buildroot mailing list some weeks ago.

[1] http://lists.busybox.net/pipermail/buildroot/2017-February/183579.h
tml

Jörg
Ben Boeckel Feb. 27, 2017, 8:40 p.m. UTC | #12
On Mon, Feb 27, 2017 at 21:24:36 +0100, Jörg Krause wrote:
> For a detailed description, please have a look at my post [1] on the
> Buildroot mailing list some weeks ago.

Hmm. Is /usr/lib32 supposed to be an implicit runtime path? If so, why
is the compiler not indicating that?

--Ben
Yann E. MORIN Feb. 27, 2017, 8:53 p.m. UTC | #13
Jörg, All,

On 2017-02-27 19:30 +0100, Yann E. MORIN spake thusly:
> On 2017-02-27 18:25 +0100, Yann E. MORIN spake thusly:
> > On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > > cmake-3.7 has a bug in how it handles rpath, linking with libraries
> > > > from
> > > > the host.
> > > > 
> > > > Until we completely understand the issue, just blacklist cmake-3.7.
> > > > 
> > > > The issue has been reported upstream:
> > > >     http://public.kitware.com/pipermail/cmake/2017-February/064970.ht
> > > > ml
> > > 
> > > Brad King from Kitware replied today [1]. In short, Brad does not think
> > > there anything wrong about handling the rpath and supposes to load a
> > > custom platform cmake file instead of the Linux one.
> > > 
> > > [1] http://public.kitware.com/pipermail/cmake/2017-February/065063.html
> > 
> > OK, so what we would have to do (basically):
> > 
> >   - copy Modules/Platform/Linux.cmake to Modules/Platform/Buildroot.cmake
> > 
> >   - tweak that file so that the two settings (lib32 and lib64) are now
> >     FALSE in that file
> > 
> >   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> > 
> > and we'd be all good?
> > 
> > Or alternatively:
> > 
> >   - add Modules/Platform/Buildroot.cmake, which:
> >     - includes Modules/Platform/Linux.cmake
> >     - sets the the two settings (lib32 and lib64) to FALSE
> > 
> >   - tweak our support/misc/toolchain.cmake to set(CMAKE_SYSTEM_NAME Buildroot)
> 
> So I tested that last solution, and it indeed fixes the build. Woot!
> 
> Of course, this is only for when we build out own cmake, not when we use
> the host pre-installed one. I'll try to see tonight if we can do similar
> for it, too.

It seems that we could set CMAKE_MODULE_PATH to that effect.

More testing under way...

Regards,
Yann E. MORIN.

> Thanks for the hints! :-)
> 
> Regards,
> Yann E. MORIN.
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Jörg Krause Feb. 27, 2017, 8:56 p.m. UTC | #14
Hi Ben,

On Mon, 2017-02-27 at 15:08 -0500, Ben Boeckel wrote:
> On Mon, Feb 27, 2017 at 21:04:06 +0100, Jörg Krause wrote:
> > I still think this is a bug! A host rpath should not be used when
> > cross-compiling whether lib32 is used or not. Somehow, it feels
> > weird
> > to say that Buildroot is not a Linux platform, in the sense of
> > CMake.
> 
> There's the chance that this was always a problem, but is now being
> exposed by this. Where is the lib32 rpath coming from? CMake doesn't
> add
> rpaths by default, so it has to be coming from somewhere. Is it added
> manually somewhere? Is a library that used to be found in the sysroot
> now being found on the host? If so, *that's* the bug which should be
> fixed. Given how widespread it is, maybe something is set up
> incorrectly
> in the toolchain file?
> 
> This flag is the same logic as lib64, just with a different suffix.
> Why
> is the problem not surfacing on Red Hat platforms which use lib64?

Note, that the build errors depend on the host system. I was able to
reproduce the build errors on a Debian system, whereas on a Arch, the
build does not break. However, all system are affected by having a
rpath set to a host path. It just not break the build on all host
systems. I did not investigated why...
Yann E. MORIN Feb. 27, 2017, 8:59 p.m. UTC | #15
Ben, All,

On 2017-02-27 13:35 -0500, Ben Boeckel spake thusly:
> On Mon, Feb 27, 2017 at 19:30:16 +0100, Yann E. MORIN wrote:
> > Of course, this is only for when we build out own cmake, not when we use
> > the host pre-installed one. I'll try to see tonight if we can do similar
> > for it, too.
> 
> It may make sense to support this upstream. CMake already has platform
> files for things such as Cray and Android (among others) which are just
> special kinds of Linux, like Buildroot.
> 
> How often does that toolchain file change? Or does it depend on
> Buildroot settings?

The toolchainfile.cmake is really specific to one build: it contains the
path to the compiler, linker... It contains place-holders that get
replaced by a sed invocation:

    https://git.buildroot.org/buildroot/tree/package/pkg-cmake.mk#n236

However, the Modules/Platform/Buildroot.cmake would be just as simple
as (which is what I use currently):

    include(Platform/Linux)
    set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
    set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)

I don;t see that changing anytime soon. So, if that would be aceptable
for upstream, then we could submit it, yes.

However, I wonder if that's worth the effort...

After all, this is a setting specific to us. I would have a hard time
arguing that upstream cmake should carry all such files for all
imaginable buildsystems.

And if the solution is that simple and also works for host pre-installed
cmake, then we can carry that ourselves, I believe.

Regards,
Yann E. MORIN.
Jörg Krause Feb. 27, 2017, 9:02 p.m. UTC | #16
On Mon, 2017-02-27 at 21:53 +0100, Yann E. MORIN wrote:
> Jörg, All,
> 
> On 2017-02-27 19:30 +0100, Yann E. MORIN spake thusly:
> > On 2017-02-27 18:25 +0100, Yann E. MORIN spake thusly:
> > > On 2017-02-27 18:12 +0100, Jörg Krause spake thusly:
> > > > On Sun, 2017-02-26 at 23:42 +0100, Yann E. MORIN wrote:
> > > > > cmake-3.7 has a bug in how it handles rpath, linking with
> > > > > libraries
> > > > > from
> > > > > the host.
> > > > > 
> > > > > Until we completely understand the issue, just blacklist
> > > > > cmake-3.7.
> > > > > 
> > > > > The issue has been reported upstream:
> > > > >     http://public.kitware.com/pipermail/cmake/2017-February/0
> > > > > 64970.ht
> > > > > ml
> > > > 
> > > > Brad King from Kitware replied today [1]. In short, Brad does
> > > > not think
> > > > there anything wrong about handling the rpath and supposes to
> > > > load a
> > > > custom platform cmake file instead of the Linux one.
> > > > 
> > > > [1] http://public.kitware.com/pipermail/cmake/2017-February/065
> > > > 063.html
> > > 
> > > OK, so what we would have to do (basically):
> > > 
> > >   - copy Modules/Platform/Linux.cmake to
> > > Modules/Platform/Buildroot.cmake
> > > 
> > >   - tweak that file so that the two settings (lib32 and lib64)
> > > are now
> > >     FALSE in that file
> > > 
> > >   - tweak our support/misc/toolchain.cmake to
> > > set(CMAKE_SYSTEM_NAME Buildroot)
> > > 
> > > and we'd be all good?
> > > 
> > > Or alternatively:
> > > 
> > >   - add Modules/Platform/Buildroot.cmake, which:
> > >     - includes Modules/Platform/Linux.cmake
> > >     - sets the the two settings (lib32 and lib64) to FALSE
> > > 
> > >   - tweak our support/misc/toolchain.cmake to
> > > set(CMAKE_SYSTEM_NAME Buildroot)
> > 
> > So I tested that last solution, and it indeed fixes the build.
> > Woot!
> > 
> > Of course, this is only for when we build out own cmake, not when
> > we use
> > the host pre-installed one. I'll try to see tonight if we can do
> > similar
> > for it, too.
> 
> It seems that we could set CMAKE_MODULE_PATH to that effect.

Sounds promising!

> More testing under way...

Thanks!
Jörg Krause Feb. 27, 2017, 9:13 p.m. UTC | #17
Yann, Ben,

On Mon, 2017-02-27 at 21:59 +0100, Yann E. MORIN wrote:
> Ben, All,
> 
> On 2017-02-27 13:35 -0500, Ben Boeckel spake thusly:
> > On Mon, Feb 27, 2017 at 19:30:16 +0100, Yann E. MORIN wrote:
> > > Of course, this is only for when we build out own cmake, not when
> > > we use
> > > the host pre-installed one. I'll try to see tonight if we can do
> > > similar
> > > for it, too.
> > 
> > It may make sense to support this upstream. CMake already has
> > platform
> > files for things such as Cray and Android (among others) which are
> > just
> > special kinds of Linux, like Buildroot.
> > 
> > How often does that toolchain file change? Or does it depend on
> > Buildroot settings?
> 
> The toolchainfile.cmake is really specific to one build: it contains
> the
> path to the compiler, linker... It contains place-holders that get
> replaced by a sed invocation:
> 
>     https://git.buildroot.org/buildroot/tree/package/pkg-cmake.mk#n23
> 6
> 
> However, the Modules/Platform/Buildroot.cmake would be just as simple
> as (which is what I use currently):
> 
>     include(Platform/Linux)
>     set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS FALSE)
>     set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS FALSE)
> 
> I don;t see that changing anytime soon. So, if that would be
> aceptable
> for upstream, then we could submit it, yes.
> 
> However, I wonder if that's worth the effort...
> 
> After all, this is a setting specific to us. I would have a hard time
> arguing that upstream cmake should carry all such files for all
> imaginable buildsystems.

I replied to Bard King on the CMake mailing list [1]. From my view,
there is an issue that /sysroot/usr/lib32 is not reported by the linker
or not treated by CMake as an implicit runtime search path. However, I
am neither a CMake nor a toolchain expert...

> And if the solution is that simple and also works for host pre-
> installed
> cmake, then we can carry that ourselves, I believe.

Sounds reasonable!

[1] https://cmake.org/pipermail/cmake/2017-February/065064.html

Jörg
Peter Korsgaard Feb. 28, 2017, 8:51 a.m. UTC | #18
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > cmake-3.7 has a bug in how it handles rpath, linking with libraries from
 > the host.

 > Until we completely understand the issue, just blacklist cmake-3.7.

 > The issue has been reported upstream:
 >     http://public.kitware.com/pipermail/cmake/2017-February/064970.html

It is a bit painful that we are back to always building cmake on modern
distributions, but OK - Correctness is more important.

Committed, thanks.
Yann E. MORIN Feb. 28, 2017, 4:16 p.m. UTC | #19
Peter, All,

On 2017-02-28 09:51 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > cmake-3.7 has a bug in how it handles rpath, linking with libraries from
>  > the host.
> 
>  > Until we completely understand the issue, just blacklist cmake-3.7.
> 
>  > The issue has been reported upstream:
>  >     http://public.kitware.com/pipermail/cmake/2017-February/064970.html
> 
> It is a bit painful that we are back to always building cmake on modern
> distributions, but OK - Correctness is more important.

To be noted: I have a branch that re-introduces cmake-3.7 (and later
versions, I hope) and fixes this rpath issue.

It works for all types of cmake:

  - pre-installed host cmake, tested with 3.1 and 3.7

  - built cmake, tested with 3.6 and 3.7

I still need to do more testing, as well as proper commit logs. ;-)
Expect it Real Soon Now (TM)! :-)

Regards,
Yann E. MORIN.
Peter Korsgaard Feb. 28, 2017, 4:38 p.m. UTC | #20
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Peter, All,
 > On 2017-02-28 09:51 +0100, Peter Korsgaard spake thusly:
 >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
 >> 
 >> > cmake-3.7 has a bug in how it handles rpath, linking with libraries from
 >> > the host.
 >> 
 >> > Until we completely understand the issue, just blacklist cmake-3.7.
 >> 
 >> > The issue has been reported upstream:
 >> >     http://public.kitware.com/pipermail/cmake/2017-February/064970.html
 >> 
 >> It is a bit painful that we are back to always building cmake on modern
 >> distributions, but OK - Correctness is more important.

 > To be noted: I have a branch that re-introduces cmake-3.7 (and later
 > versions, I hope) and fixes this rpath issue.

 > It works for all types of cmake:

 >   - pre-installed host cmake, tested with 3.1 and 3.7

 >   - built cmake, tested with 3.6 and 3.7

 > I still need to do more testing, as well as proper commit logs. ;-)
 > Expect it Real Soon Now (TM)! :-)

Ok, good - Thanks!
diff mbox

Patch

diff --git a/support/dependencies/check-host-cmake.sh b/support/dependencies/check-host-cmake.sh
index 9b63b06..84c26c2 100755
--- a/support/dependencies/check-host-cmake.sh
+++ b/support/dependencies/check-host-cmake.sh
@@ -6,6 +6,9 @@  version_min="${2}"
 major_min="${version_min%.*}"
 minor_min="${version_min#*.}"
 
+# cmake-3.7 incorrectly handles rpath, linking to host libraries
+blacklist_version="3.7"
+
 cmake=`which ${candidate}`
 if [ ! -x "${cmake}" ]; then
     # echo nothing: no suitable cmake found
@@ -27,6 +30,11 @@  version="$(${cmake} --version \
 major="${version%.*}"
 minor="${version#*.}"
 
+if [ "${version}" = "${blacklist_version}" ]; then
+    # echo nothing: no suitable cmake found
+    exit 1
+fi
+
 if [ ${major} -gt ${major_min} ]; then
     echo "${cmake}"
 else