Merge lp:~widelands-dev/widelands/cmakepolicy into lp:widelands

[Toni Förster]

set cmake_policy version range to NEW instead of specific policy:

https://cmake.org/cmake/help/v3.12/command/cmake_policy.html

We set the policy CMP0054 to NEW since it was not set. Since I stumbled over bug #1772079 I thought it might be better to set a range instead of every single policy (which is also advised in the Cmake manual)

@kaputtnik can you compile this branch on your system, please. You shouldn't get this warning anymore and Cmake should use the GLVND now.

[kaputtnik (franku)]

Looks good to me...

The warning does not come up any more.

Thanks :-)

review: Approve

[kaputtnik (franku)]

oops, looks i have approved too early:

The warning is gone but i get a compile error later on:

[ 46%] Linking CXX executable test_economy
/usr/bin/ld: ../../graphic/libgraphic_gl_utils.a(initialize.cc.o): undefined reference to symbol 'glDrawBuffer'
/usr/bin/ld: /usr/lib/libGL.so.1: error adding symbols: DSO missing from command line
collect2: Fehler: ld gab 1 als Ende-Status zurück
make[2]: *** [src/economy/test/CMakeFiles/test_economy.dir/build.make:472: src/economy/test/test_economy] Fehler 1
make[1]: *** [CMakeFiles/Makefile2:10016: src/economy/test/CMakeFiles/test_economy.dir/all] Fehler 2
make: *** [Makefile:141: all] Fehler 2

review: Needs Fixing

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4090. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/436365665.
Appveyor build 3886. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cmakepolicy-3886.

[GunChleoc (gunchleoc)]

Trunk also has problems in Ubuntu Trusty builds:

CMake Error at CMakeLists.txt:4 (cmake_policy):
  Policy "CMP0054" is not known to this version of CMake.

https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/15512527

So, making this fix work is very much desired :)

[Toni Förster (stonerl)]

I set a fixed version now: 3.9.2. That's the one that is used to compile the macOS nightlies. The Version range does not seem to work as describe, although it should be backwards-compatible (maybe I just misread something)

This does not fix the warning kaputtnik reported. Apparently Widelands can't use the GLVND-libs ATM. That' at least what I read from the error-message kaputtnik posted. Maybe the bug-report should be updated.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4100. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/438094146.
Appveyor build 3896. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cmakepolicy-3896.

[kaputtnik (franku)]

While compiling runs through, the linked bug report isn't fixed. You mentioned CMP0054 while the warning i get mention CMP0072?

-- Boost version: 1.68.0
-- Found the following Boost libraries:
-- unit_test_framework
-- regex
-- system
CMake Warning (dev) at /usr/share/cmake-3.12/Modules/FindOpenGL.cmake:270 (message):
  Policy CMP0072 is not set: FindOpenGL prefers GLVND by default when
  available. Run "cmake --help-policy CMP0072" for policy details. Use the
  cmake_policy command to set the policy and suppress this warning.

  FindOpenGL found both a legacy GL library:

    OPENGL_gl_LIBRARY: /usr/lib/libGL.so

  and GLVND libraries for OpenGL and GLX:

    OPENGL_opengl_LIBRARY: /usr/lib/libOpenGL.so
    OPENGL_glx_LIBRARY: /usr/lib/libGLX.so

  OpenGL_GL_PREFERENCE has not been set to "GLVND" or "LEGACY", so for
  compatibility with CMake 3.10 and below the legacy GL library will be used.
Call Stack (most recent call first):
  CMakeLists.txt:61 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.

-- SDL2_INCLUDE_DIR is /usr/include/SDL2
-- SDL2MAIN_LIBRARY is /usr/lib/libSDL2main.a
-- Not using AddressSanitizer.
-- Version of Widelands Build is bzr8869[cmakepolicy](Debug)
-- Configuring done

[Toni Förster (stonerl)]

@kaputtnik I know.

The warning you get, can't be resolved be simply setting cmake_policy to 3.10 or newer (as I naively thought). Doing so would result in using the new GLVND-libs (which is desired), but apparently widelands currently can't be compiled using these libraries. See the linking errors in your second post.

Getting this fixed means. Get widelands to compile with the GLVND-libs and then set the policy to version 3.10.

The intention for this change was to get the builds for 14.04 running (the ones GunChleoc mentioned later on). By doing so I broke Travis and Appveyor builds, because most cmake-versions there don't support a version range. But just a fixed max version (which is totally fine). A by-product would have been fixing your warning as well, but for reasons I described above this is sadly not as simple as I hoped.

[GunChleoc (gunchleoc)]

How about trying to do what the original error message is suggesting?

https://cmake.org/cmake/help/latest/policy/CMP0072.html

Try this line:

OpenGL_GL_PREFERENCE=LEGACY

[Toni Förster (stonerl)]

@GunChleoc

But that does not solve the underlying issue, does it?

Setting:

cmake_policy(SET CMP0072 OLD)

is the same as not setting the policy at all. In both will result in:

OpenGL_GL_PREFERENCE=LEGACY

The first one though would silence the warning BUT we would run into issues with Cmake version older than 3.11 since they don't know this policy, as seen here:

https://launchpadlibrarian.net/392627292/buildlog_ubuntu-trusty-i386.widelands_1%3A19-ppa0-bzr8875-201810101704~ubuntu14.04.1_BUILDING.txt.gz

CMake Error at CMakeLists.txt:4 (cmake_policy):
  Policy "CMP0054" is not known to this version of CMake.

Since I don't have a Linux machine I can't real test for the linking errors kaputtnik reported.

The commit as it is now should solve the problem with the buildsystem. Should I open a new commit just for the buildsystem?

[Toni Förster (stonerl)]

@kaputtnik can you give this another try?

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4111. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/440139202.
Appveyor build 3906. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cmakepolicy-3906.

[kaputtnik (franku)]

Compiling is fine here with the latest change: No Warning and no compile errors.

I do not really understand that stuff, but can't we just add a switch like that (instead of setting the linker flags):

if(${CMAKE_VERSION} VERSION_GREATER "3.11")
    cmake_policy(SET CMP0072 OLD)
endif()

I tested this and it works for me (deleted the build directory and compiled again) ... Has to be commented well, though.

[Toni Förster (stonerl)]

@kaputtnik the changes I made set the policy to NEW. The trick that it. did were the additional linker options. But it broke travis builds and will throw errors on system that use old cmake version.

Your proposed switch sets it to the old (deprecated) behavior. Would you be so nice and test the latest change I pushed?

This change only sets the policy to NEW, if it is known to the Cmake version in use. This should work with travis, but we have to wait, though.

[Toni Förster (stonerl)]

Btw. AFAIK when using the new behavior, it seems mandatory to set the linker-flags.

See here:

https://stackoverflow.com/questions/23729213/link-error-when-trying-to-build-a-simple-opengl-program

[Toni Förster (stonerl)]

@kaputtnik you could try to remove the linker flags bit by bit and see which one is really necessary to compile without linker errors.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4114. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/440360936.
Appveyor build 3909. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cmakepolicy-3909.

[kaputtnik (franku)]

From my understanding your change will use GLVND on my system. Wouldn't this make bug hunting a bit more complicated, because some bugs may appear on systems using the legacy GL driver but not on systems using GLVND libraries?

Anyway, i have removed the linker flags bit by bit (from right to left), removed the /build directory after each remove and compiled again. The result is:

if("${OpenGL_GL_PREFERENCE}" STREQUAL "GLVND")
   set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lGL")
   message(STATUS "Adding linker flags for GLVND.")
endif()

Removing this last flag (commenting tho whole block) produces the linker errors:

[ 98%] Building CXX object src/economy/test/CMakeFiles/test_economy.dir/test_road.cc.o
/usr/bin/ld: graphic/libgraphic_gl_utils.a(initialize.cc.o): undefined reference to symbol 'glDrawBuffer'
/usr/bin/ld: /usr/lib/libGL.so.1: error adding symbols: DSO missing from command line
collect2: Fehler: ld gab 1 als Ende-Status zurück
make[2]: *** [src/CMakeFiles/widelands.dir/build.make:441: src/widelands] Fehler 1
make[1]: *** [CMakeFiles/Makefile2:1217: src/CMakeFiles/widelands.dir/all] Fehler 2
make[1]: *** Es wird auf noch nicht beendete Prozesse gewartet....
[ 98%] Building CXX object src/economy/test/CMakeFiles/test_economy.dir/test_routing.cc.o
[ 99%] Linking CXX executable test_io_filesystem
/usr/bin/ld: ../../../graphic/libgraphic_gl_utils.a(initialize.cc.o): undefined reference to symbol 'glDrawBuffer'
/usr/bin/ld: /usr/lib/libGL.so.1: error adding symbols: DSO missing from command line
collect2: Fehler: ld gab 1 als Ende-Status zurück
make[2]: *** [src/io/filesystem/test/CMakeFiles/test_io_filesystem.dir/build.make:455: src/io/filesystem/test/test_io_filesystem] Fehler 1
make[1]: *** [CMakeFiles/Makefile2:14128: src/io/filesystem/test/CMakeFiles/test_io_filesystem.dir/all] Fehler 2
[ 99%] Linking CXX executable test_economy
/usr/bin/ld: ../../graphic/libgraphic_gl_utils.a(initialize.cc.o): undefined reference to symbol 'glDrawBuffer'
/usr/bin/ld: /usr/lib/libGL.so.1: error adding symbols: DSO missing from command line
collect2: Fehler: ld gab 1 als Ende-Status zurück
make[2]: *** [src/economy/test/CMakeFiles/test_economy.dir/build.make:472: src/economy/test/test_economy] Fehler 1
make[1]: *** [CMakeFiles/Makefile2:5808: src/economy/test/CMakeFiles/test_economy.dir/all] Fehler 2
make: *** [Makefile:141: all] Fehler 2
mv: der Aufruf von stat für 'src/widelands' ist nicht möglich: Datei oder Verzeichnis nicht gefunden
mv: der Aufruf von stat für '../build/src/website/wl_map_object_info' ist nicht möglich: Datei oder Verzeichnis nicht gefunden
mv: der Aufruf von stat für '../build/src/website/wl_map_info' ist nicht möglich: Datei oder Verzeichnis nicht gefunden

[Toni Förster (stonerl)]

Thanks allot. I changed the the line for the linker flags.

> From my understanding your change will use GLVND on my system. Wouldn't this make bug hunting a
> bit more complicated, because some bugs may appear on systems using the legacy GL driver but not
> on systems using GLVND libraries?

Correct, but if only the new drivers would be installed on a system then only those would be used. And isn't it unavoidable to switch and test with the new drivers as well?

For the sake of bug hunting this could be set to OLD, but this would defeat the purpose of the policy. I think settings this to OLD manually when needed would be sensible.

As for now, this fixes the bug with the buildsystem, silences the warning and compiles when GLVND are used.

@GunChleoc what's your opinion here.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4116. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/440557293.
Appveyor build 3913. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cmakepolicy-3913.

[kaputtnik (franku)]

Can we have an output which library is used on the console in the Graphics section? On my system the output, regardless of the used library, is:

Graphics: OpenGL: Version "4.6.0 NVIDIA 410.57"
[..]
**** GRAPHICS REPORT ****
 VIDEO DRIVER x11
 pixel fmt 370546692
 size 1680 1050
**** END GRAPHICS REPORT ****

[Toni Förster (stonerl)]

That, I cannot answer.

[Toni Förster (stonerl)]

I guess I found an answer :D

@kaputtnik can you try again, please?

[kaputtnik (franku)]

Sorry, this do not work, no output here. First i thought thats because the #ifdef is inside the log(), so i pulled it out and give it an own log, like:

#ifdef OpenGL_GL_PREFERENCE
   log(" Using GLVND\n");
#endif

But this didn't work either. Looks to me like the OpenGL_GL_PREFERENCE isn't set at all, because during editing i forgot the ';' and that throughs an error on compiling but after fixing, there is no output either. No idea...

[Toni Förster (stonerl)]

Would you be so kind and test again?

[kaputtnik (franku)]

You are the master of cmake policies :-D

Original output:

**** GRAPHICS REPORT ****
 VIDEO DRIVER GLVND x11
 pixel fmt 370546692
 size 1680 1050
**** END GRAPHICS REPORT ****

After setting: cmake_policy(SET CMP0072 OLD)

The output is:

**** GRAPHICS REPORT ****
 VIDEO DRIVER x11
 pixel fmt 370546692
 size 1680 1050
**** END GRAPHICS REPORT ****

Lets wait for travis and appveyor and merge this change :-)

review: Approve

[kaputtnik (franku)]

I think bunnybot checks the latest latest changes against travis and appveyor automatically, so:

@bunnybot merge

[kaputtnik (franku)]

Hm... looks like Ubuntu needs other linkerflags?

I am running archlinux, which is much close to any changes. Each time a new release of a program/library is official it will get soonish into the archlinux repo. Ubuntu is more conservative to this.

Log for revision which has this changes already in:

https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/15541748/+files/buildlog_ubuntu-cosmic-amd64.widelands_1%3A19-ppa0-bzr8880-201810140801~ubuntu18.10.1_BUILDING.txt.gz

[Toni Förster (stonerl)]

It seems so. We would need someone with 18.10 (hasn't been released though) to test.

If no one objects I would append these flag: "-lGLU -lGLEW -lglut" and push directly to trunk.

[Toni Förster (stonerl)]

It is using the wrong libraries...

libGL.so: error adding symbols: DSO missing from command line

That's the legacy library. Maybe we need to point to he correct library like so:

-lGL /usr/lib/x86_64-linux-gnu/libOpenGL.so

[Toni Förster (stonerl)]

I was wrong about the library. Still no clue as to why it does not compile on 18.10

[Toni Förster (stonerl)]

I installed 18.10 in a virtual machine. The problem seems to be cause by GCC-8 which, is standard on 18.10. Compiling with Clang shows no problems at all.

[kaputtnik (franku)]

I have both installed, no idea which is used for compiling widelands :-D

$:> pacman -Qs gcc
local/gcc 8.2.1+20180831-1 (base-devel)
    The GNU Compiler Collection - C and C++ frontends

$:> pacman -Qs clang
local/clang 7.0.0-1
    C language family frontend for LLVM

Other packages maybe related on my system:

local/freeglut 3.0.0-2
    Provides functionality for small OpenGL programs
local/glew 2.1.0-1
    The OpenGL Extension Wrangler Library
local/glibmm 2.56.0-1
    C++ bindings for GLib
local/glu 9.0.0-5
    Mesa OpenGL Utility library
local/libglvnd 1.1.0-1
    The GL Vendor-Neutral Dispatch library
local/mesa 18.2.2-1
    An open-source implementation of the OpenGL specification

[Toni Förster (stonerl)]

When running the compile.sh script one of the very first line should tell you which compiler is used.

[kaputtnik (franku)]

I guess it's gcc 8.x:

-- The C compiler identification is GNU 8.2.1
-- The CXX compiler identification is GNU 8.2.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Boost version: 1.68.0
[...]

Why does gcc work here and not on Ubuntu?

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2018-09-26 13:41:40 +0000
+++ CMakeLists.txt	2018-10-13 18:51:07 +0000
@@ -1,7 +1,23 @@
 project (widelands)
 
 cmake_minimum_required (VERSION 2.8.7)
-cmake_policy(SET CMP0054 NEW)
+cmake_policy(VERSION 2.8.7)
+
+# This policy is not known to versions prior 3.1 and would result in errors, 
+# if set on such systems. This can be removed when cmake_minimum_required is set
+# to 3.1 or newer by using:
+# cmake_policy(VERSION 3.1)
+if(POLICY CMP0054)
+   cmake_policy(SET CMP0054 NEW)
+endif(POLICY CMP0054)
+
+# This policy is not known to versions prior 3.11 and would result in errors, 
+# if set on such systems. This can be removed when cmake_minimum_required is set
+# to 3.11 or newer  by using:
+# cmake_policy(VERSION 3.11)
+if(POLICY CMP0072)
+   cmake_policy(SET CMP0072 NEW)
+endif(POLICY CMP0072)
 
 include("${CMAKE_SOURCE_DIR}/cmake/WlFunctions.cmake")
 
@@ -140,6 +156,13 @@
   message(STATUS "Not using AddressSanitizer.")
 endif(OPTION_ASAN)
 
+# This is set to avoid linker errors when using GLVND-libs on Linux
+if("${OpenGL_GL_PREFERENCE}" STREQUAL "GLVND")
+   set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lGL")
+   add_compile_definitions(WL_USE_GLVND)
+   message(STATUS "Adding linker flags for GLVND.")
+endif()
+
 if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
   wl_add_flag(WL_COMPILE_DIAGNOSTICS "-Weverything")
 

=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc	2018-08-28 07:53:33 +0000
+++ src/graphic/graphic.cc	2018-10-13 18:51:07 +0000
@@ -105,7 +105,11 @@
 		SDL_DisplayMode disp_mode;
 		SDL_GetWindowDisplayMode(sdl_window_, &disp_mode);
 		log("**** GRAPHICS REPORT ****\n"
+		#ifdef WL_USE_GLVND
+		    " VIDEO DRIVER GLVND %s\n"
+		#else
 		    " VIDEO DRIVER %s\n"
+		#endif
 		    " pixel fmt %u\n"
 		    " size %d %d\n"
 		    "**** END GRAPHICS REPORT ****\n",