CMake improvements (mainly for Linux and FreeBSD) #24

arthurzam · 2016-06-17T15:42:53Z

continue of issue #22 and PR #21
As of the previous PR, I will add future commits as we resolve, and at the end squash everything and you will add it into the project (without merge commit).

fix finding GME without pkg-config

I added a new CMake module for finding GME, and it will even search the library and headers, thus increasing the chances to find it.

optional modules
fallback to Qt4 if no Qt5 or Qt5 < 5.6.1
FreeBSD man install

Now it should be installing to $PREFIX/man -> if you run the default prefix, as seen here, then is should be in correct place.

show summary
make uninstall
remove QX11Extras dependency.
FFmpeg 2.2.0 or newer is needed

I can't manage to understand how to check what version we even use. My last resort I see is to run something like ffmpeg -version and then parse the output using regex.
Another question - what is the source of the minimal version - just not working? not compiling? specific "module" version nedded?

properly install QMPlay2-opencda.desktop

I found (finally) the proper way to use those files. But some of the concerns are: how do we distinguish KDE4 and/or KF5 in case both are installed.

install the license

I think much better name for the license file would be LICENSE instead of COPYING. What are your thoughts?

add option for ProstoPleer

Another question about VDPAU: in #22 you mentioned that it also needs libX11, but in the pro file there is no mention of this check. What is the correct way?
On the same file, in FFmpeg.pro are those sources/headers optional, if yes in what way? Are they connected to libavdevice?
Also, should we add a sub-option if USE_VDPAU is on to make optional the VDPAU without own video output?

add C++11 (-std=c++11) flags for CMake < 3.1.0 and GCC and Clang (also for GCC 6, no need to detect compiler version) and Qt 5.7.0
use CMake 2.8.11 for compatibility (in progress, Qt4 problems)
detect OpenGL|ES 2.0 for Qt5 and set compiler macros for it
show warning when module will be disabled due to missing dependencies

The full and cleaned-up task list is in #22

zaps166 · 2016-06-17T16:24:04Z

I think much better name for the license file would be LICENSE instead of COPYING. What are your thoughts?

I'm not a lawer, I can change it, sounds better :) Anyway I probably should add a license-header to every cpp and hpp file... 😃

Another question about VDPAU: in #22 you mentioned that it also needs libX11, but in the pro file there is no mention of this check. What is the correct way?

I'm using XOpenDisplay() there, so it needs the libX11 (it should link to it in CMake). I don't know, but it works, maybe due to ohter libraries symbols. Windows/MinGW is more restrictive in linking.

Also, should we add a sub-option if USE_VDPAU is on to make optional the VDPAU without own video output?

I've removed macros from sources, so it needs to add the ifdefs again. They can be compiled together w/o additional option.

I found (finally) the proper way to use those files. But some of the concerns are: how do we distinguish KDE4 and/or KF5 in case both are installed.

Arch Linux has only KF5 (and some apps from KDE4). If KF5 is detected - copy to KF5 directory. Otherwise copy to KDE4 directory (path can differ between distros). No need to support both.

I can't manage to understand how to check what version we even use. My last resort I see is to run something like ffmpeg -version and then parse the output using regex.

It'll fail if ffmpeg command tool won't be available. Probably you must check every module seaprately: libavcodec, libavformat, etc (they have different versions). Just do it as the latest thing.

Another question - what is the source of the minimal version - just not working? not compiling? specific "module" version nedded?

FFmpeg less then 2.2.0 won't compile (e.g. Buffer class needs features from >= 2.2.0).

As of the previous PR, I will add future commits as we resolve, and at the end squash everything and you will add it into the project (without merge commit).

I can squash all commits before merge, so no need to change the history of PR :)

I added a new CMake module for finding GME, and it will even search the library and headers, thus increasing the chances to find it.

So we've got the new cmake dir. It will be great to have all CMake files there 😃, so only *.pro files for qmake/QtCreator will be in dirs, and entire CMake files in separate directory :)

Now it should be installing to $PREFIX/man -> if you run the default prefix, as seen here, then is should be in correct place.

I'll check this :)

zaps166 · 2016-06-17T16:58:23Z

Version merge code:

#define AV_VERSION_INT(a, b, c) (a<<16 | b<<8 | c)

so mathematically:

major * 0x10000 + minor * 0x100 + micro
major * 65536 + minor * 256 + micro

libavutil version for FFmpeg 2.2.0 is:

#define LIBAVUTIL_VERSION_MAJOR  52
#define LIBAVUTIL_VERSION_MINOR  66
#define LIBAVUTIL_VERSION_MICRO 100

libavcodec version for FFmpeg 2.2.0 is:

#define LIBAVCODEC_VERSION_MAJOR 55
#define LIBAVCODEC_VERSION_MINOR  52
#define LIBAVCODEC_VERSION_MICRO 102

libavformat version for FFmpeg 2.2.0 is:

#define LIBAVFORMAT_VERSION_MAJOR 55
#define LIBAVFORMAT_VERSION_MINOR 33
#define LIBAVFORMAT_VERSION_MICRO 100

libswscale version for FFmpeg 2.2.0 is:

#define LIBSWSCALE_VERSION_MAJOR 2
#define LIBSWSCALE_VERSION_MINOR 5
#define LIBSWSCALE_VERSION_MICRO 102

libswresample version for FFmpeg 2.2.0 is:

#define LIBSWRESAMPLE_VERSION_MAJOR 0
#define LIBSWRESAMPLE_VERSION_MINOR 18
#define LIBSWRESAMPLE_VERSION_MICRO 100

libavresample version for FFmpeg 2.2.0 is:

#define LIBAVRESAMPLE_VERSION_MAJOR  1
#define LIBAVRESAMPLE_VERSION_MINOR  2
#define LIBAVRESAMPLE_VERSION_MICRO  0

arthurzam · 2016-06-17T19:43:39Z

VDPAU x11 check was added.
Added option for Qt5X11Extras (docstring might be bad/horrible).
Added FFmpeg versions check, according to the versions stated above.

About the KF5/KDE4: I'm afraid this will bring a big dependency tree for just one little file to install. Therefore, firstly I would like to add an option for choosing the directory. This check will just find the default one: (stop if found) search for KF5 => search for KDE4 => just set the default one (please choose). If "None" is stated by the user, we won't install it. (I'm afraid for adding dependency on the KDE big world - I prefer minimalistic system, and just for building isn't a good idea).

Also about finding packages. I can continue (like now) to just use pkg-config, but I think searching (at first we will try pkg-config) is much better for porting to new OSs. The only disadvantage is some more files in the /src/cmake/modules. If won't be hard for me.

Btw: I will update the list above, and make the text crossed when it answered (I won't delete it for future generations of bored developers). It will affect only the first message.

zaps166 · 2016-06-17T21:47:03Z

Rename FindLibGME.cmake -> FindGME.cmake (looks better)
Also maybe rename cmake_uninstall.cmake.in -> Uninstall.cmake.in (it is now in cmake directory, so no need for 'cmake' in name)?

About the KF5/KDE4 (...)

I don't know the default path - it depends. Try this logic ($USER_SOLID_ACTIONS_PATH is a custom patch which can be added by -DCMAKE_SOMETHING='path'):

if [ -z $USER_SOLID_ACTIONS_PATH ]; then #If user doesn't specify the path - try detect it
    if [ -d /usr/share/solid/actions ]; then
        copy to /usr/share/solid/actions
    elif [ -d /usr/share/kde4/apps/solid/actions ]; then
        copy to /usr/share/kde4/apps/solid/actions
    elif [ -d /usr/share/apps/solid/actions ]; then
        copy to /usr/share/apps/solid/actions
    else
        "Show in summary that this file won't be copied."
    fi
else
    copy to $USER_SOLID_ACTIONS_PATH
fi

Also about finding packages. I can continue (like now) to just use pkg-config, but I think searching (at first we will try pkg-config) is much better for porting to new OSs. The only disadvantage is some more files in the /src/cmake/modules. If won't be hard for me.

It is less important. All Linux distros and FreeBSD have pkg-config (also it works on OS X and sometimes on Windows with some effort). Is it worth? Maybe later for OS X or other exotic platforms (Windows...)?

zaps166 · 2016-06-17T21:52:09Z

I don't know CMake syntax, but why it has condition in endif() ?

QMPlay2/src/cmake_uninstall.cmake.in

Line 5 in 6f407c3

endif(NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt")

Or in else():

QMPlay2/src/cmake_uninstall.cmake.in

Line 20 in 6f407c3

else(IS_SYMLINK "$ENV{DESTDIR}${file}" OR EXISTS "$ENV{DESTDIR}${file}")

? 😃

zaps166 · 2016-06-17T22:06:50Z

install the license

How other software handle this? Is it task for CMake or is it only task for packager?

zaps166 · 2016-06-17T22:33:08Z

Added another task, see #22 (about ProstoPleer, 9aac5d1).

arthurzam · 2016-06-18T07:13:40Z

fixed removal of QX11Extras
added ProstoPleer
renamed files
fixed CMake syntax (the else and endif is a legacy thing for supporting old CMake versions, not needed in our case).

About the license file - most of them use the LICENSE file naming. It is also the default one in Arch and Gentoo.

About the searching libs - I will put it on hold. CMake has a great potential for Windows, making building as effortless for you as possible. But for now we will focus on Linux and FreeBSD, and maybe OS X.

arthurzam · 2016-06-18T08:11:31Z

solid action are installing: make dependency is only extra-cmake-modules.
added summary
please say if the summary is OK?

zaps166 · 2016-06-18T08:48:54Z

I'll start testing :)

Btw. 1d7cb19#diff-ed2c06d232f33ec5338b0720dd0e61ebR23 (file condition)

zaps166 · 2016-06-18T09:19:22Z

added summary please say if the summary is OK?

OK!

solid action are installing: make dependency is only extra-cmake-modules.

ECM is optional, so w/o it it just do nothing, right? It tries to push it into /usr/local/... (also if the prefix is /usr), so it doesn't auto-detect (tried on make DESTDIR=../xxx install).

renamed files

Uninstall.cmake.in - capital letters for consistency?

About the searching libs - I will put it on hold. CMake has a great potential for Windows, making building as effortless for you as possible. But for now we will focus on Linux and FreeBSD, and maybe OS X.

OK! Btw. look here #23

According to

QMPlay2/compile_unix

Line 31 in 0577c75

if [ -z ${QT_SUFFIX+x} ]; then

: Use Qt5 if Qt4 is not found (also show warning if <= 5.6.0).

Something is wrong on FreeBSD now, cannot configure:

cmake .. -DUSE_QT5=Yes

CMake Error at src/gui/CMakeLists.txt:105 (install):
  install TARGETS given no RUNTIME DESTINATION for executable target
  "QMPlay2".

CMake Error at src/qmplay2/CMakeLists.txt:145 (install):
  install TARGETS given no LIBRARY DESTINATION for shared library target
  "qmplay2".

zaps166 · 2016-06-18T09:23:10Z

ECM is optional, so w/o it it just do nothing, right? It tries to push it into /usr/local/... (also if the prefix is /usr), so it doesn't auto-detect (tried on make DESTDIR=../xxx install).

It works, but when I use -DCMAKE_INSTALL_PREFIX=/usr. If I change the prefix later, then it doesn't work (e.g. in cmake-gui).

Caused by: 9aac5d1 This is a temporary solution, see #22 nad #24

Caused by: 9aac5d1 This is a temporary solution, see #22 and #24

arthurzam · 2016-06-18T12:52:45Z

I rebased again because of the "CMake: Enable ProstoPleer" commit (removes your change in the appropriate commit).
I think I found the problem with FreeBSD - fixed.
I rewritten the system for path of solid-action, with it's appropriate cmake module. Also it now support the value "Auto" - find the location in runtime.
Renamed the uninstall file.

ECM is optional, and if it doesn't find it, it sets the default path "/usr/share/solid/actions/" by itself. It is a recommended dependency, but not required one.

zaps166 · 2016-06-18T13:20:05Z

I think much better name for the license file would be LICENSE instead of COPYING

Done: 42c5d1c

arthurzam · 2016-06-18T13:21:05Z

Never thought I will succeed, but I managed to add uninstall empty directories...
The solution is terrible (in my opinion). It traverses every file path's dir until DESTDIR and checks if it is empty.

zaps166 · 2016-06-18T13:23:01Z

added summary please say if the summary is OK?

maybe a one empty line before it :)

arthurzam · 2016-06-18T13:24:25Z

Added LICENSE file
added one empty line before it

zaps166 · 2016-06-18T13:25:46Z

The solution is terrible (in my opinion). It traverses every file path's dir until DESTDIR and checks if it is empty.

I hope that the scrip doesn't remove everything 😃

arthurzam · 2016-06-18T13:28:37Z

There are checks to find if there is no content in the directory. And only if there is no file/dir/symlink/etc in it it deletes it. I can't say it is 100% error proof (I can't check on every CMake out there), but I believe every "make uninstall" is very risky. Using the package manager to uninstall is much better.

zaps166 · 2016-06-18T13:30:41Z

The solution is terrible (in my opinion). It traverses every file path's dir until DESTDIR and checks if it is empty.

Hmm, maybe it is better to not remove empty directories, now I've got infinite loop on FreeBSD (so I don't want to try on non-VM OS :) ) - it's too dangerous. No need to remove everything, maybe after uninstalling remove only QMPlay2 from include (and any other QMPlay2-only directory if exists).

zaps166 · 2016-06-18T13:31:51Z

Using the package manager to uninstall is much better

Yes, but if I install something via make install and it can't be uninstalled this leads to manually removing every file.

arthurzam · 2016-06-18T13:36:04Z

Ok, now it will remove only /usr/share/qmplay2, /usr/include/qmplay2, /usr/lib/qmplay2 (those types).

zaps166 · 2016-06-18T13:58:40Z

Ok, now it will remove only /usr/share/qmplay2, /usr/include/qmplay2, /usr/lib/qmplay2 (those types).

Ok :)

/usr/lib/qmplay2/ is not removed,
duplicated slash '/`'

-- Uninstalling directory //usr/share/qmplay2/lang//
-- Uninstalling directory //usr/share/qmplay2//
-- Uninstalling directory //usr/local/lib/qmplay2/modules//
-- Uninstalling directory //usr/local/include/QMPlay2/

FreeBSD output

arthurzam · 2016-06-18T14:02:28Z

About duplicate slash - this is sanity check in case DESTDIR is without slash at the end (you can guess the dangerous situation). This is a small problem that saves us errors.
It remove according to the selected PREFIX when you configured CMake. the uninstall script remove exactly what you installed, and it doesn't remove anything except it (and the directories).

zaps166 · 2016-06-19T11:45:09Z

I mean, can you check the status before the "CMake minimal version 2.8.11"? I need to check if it part of the lower CMake version or not. It it isn't the problem then the error means CMake can't find the moc executable.

If I re-run the cmake 2.8.11 it finds MOC from Qt 5.6.1 and then it can't compile.
How to check the status?

arthurzam · 2016-06-19T11:52:19Z

Ah, sorry :)
Yes it is a must one (CMP0003), because all the system of Qt5 libraries passed to the executable is using full path to lib, which this policy enables.

I ran now CMake version 3.5.2 using Qt4 (arch linux) - works properly. I even switched some times and rebuild - al working. Just to be sure - you have Qt4's moc?

zaps166 · 2016-06-19T11:52:31Z

For error messages for modules, example for XVideo:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7e9bece..31db19c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -75,7 +75,8 @@ OPTION(USE_PULSEAUDIO "Build with PulseAudio support" ${LIBPULSE_FOUND})
 add_feature_info(PulseAudio USE_PULSEAUDIO "Build with PulseAudio support")

 if(NOT DEFINED USE_XVIDEO)
-    find_package(X11 QUIET)
+    find_package(X11)
+    find_package(X11_Xv)
     if(X11_FOUND AND X11_Xv_FOUND)
         set(DEFAULT_XVIDEO ON)

But this can't find even if libxv exists 😃

I think that module detection should go to src/modules/CMakeLists.txt - if possible of course :)

zaps166 · 2016-06-19T11:56:43Z

I ran now CMake version 3.5.2 using Qt4 (arch linux) - works properly. I even switched some times and rebuild - al working. Just to be sure - you have Qt4's moc?

Yes, I've got Qt4 MOC. This is about CMake 2.8.11, on 3.5.2 Qt4 works properly. Also if I tested yesterday on 2.8.11+Qt4 it worked properly on HEAD b88ae4e

Yes it is a must one (CMP0003), because all the system of Qt5 libraries passed to the executable is using full path to lib, which this policy enables.

Ok!

zaps166 · 2016-06-19T12:05:41Z

I must check Qt4 on CMake 2.8.11 once again like yesterday.

if(COMMAND cmake_policy)
    cmake_policy(SET CMP0003 NEW)
    if(POLICY CMP0043)
        cmake_policy(SET CMP0043 NEW)
    endif()
endif()

can it be in done as include?

zaps166 · 2016-06-19T12:09:39Z

I must check Qt4 on CMake 2.8.11 once again like yesterday.

It works properly (CMake 2.8.11 + Qt4.8.7). HEAD: b88ae4e, replaced: cmake_minimum_required(VERSION 3.0.2) -> cmake_minimum_required(VERSION 2.8.11) everywhere and works without any warning (also doesn't complain about CMP0003).

Looks like the find_package(Qt4 REQUIRED) causes a problem.
find_package(Qt4 QUIET) and then find_package(Qt4 REQUIRED) causes a problem. Is it a way to get the warning in QUIET into variable and later show it when necessary?

zaps166 · 2016-06-19T12:13:57Z

I think Qt5_EXECUTABLE_COMPILE_FLAGS should include -fPIC automatically, so:

if(CMAKE_VERSION VERSION_LESS "2.8.12")
    set(CMAKE_CXX_FLAGS "${Qt5Core_EXECUTABLE_COMPILE_FLAGS} ${CMAKE_CXX_FLAGS}")
endif()

if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" or CMAKE_CXX_COMPILER_ID STREQUAL "Clang")

or -> OR ?

In C++11 flag use Qt5Core for version detection (looks better)

zaps166 · 2016-06-19T13:05:16Z

    find_package(ALSA QUIET)
    set(DEFAULT_ALSA ${ALSA_FOUND})

ALSA can be always enabled (see #22)

I think that you should check for all Qt5 modules (Core, Gui, Widgets, Network).

zaps166 · 2016-06-19T13:13:22Z

This should fix Qt4 and Cmake 2.8.11 problem:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7e9bece..54c3675 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -23,8 +23,7 @@ else()
 endif()

 find_package(Qt5Widgets QUIET)
-find_package(Qt4 QUIET)
-if(Qt5Widgets_FOUND AND (NOT Qt4_FOUND OR NOT (Qt5Widgets_VERSION VERSION_LESS 5.6.1)))
+if(Qt5Widgets_FOUND AND (Qt5Widgets_VERSION VERSION_GREATER 5.6.0))
     set(DEFAULT_QT5 ON)
 else()
     set(DEFAULT_QT5 OFF)

Incorrect

zaps166 · 2016-06-19T13:20:13Z

And for compiler flags for Qt >= 5.7.0:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7e9bece..8851486 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -116,9 +116,11 @@ string(STRIP ${QMPLAY2_VERSION} QMPLAY2_VERSION)

 if(USE_QT5)
     find_package(Qt5Widgets REQUIRED)
-    set(Qt5_EXECUTABLE_COMPILE_FLAGS "-fPIC ${Qt5_EXECUTABLE_COMPILE_FLAGS}")
-    if(CMAKE_VERSION VERSION_LESS "3.1" AND NOT (Qt5Widgets_VERSION VERSION_LESS 5.7.0)) # cmake < 3.1 && Qt5 >= 5.7.0
-        if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" or CMAKE_CXX_COMPILER_ID STREQUAL "Clang") # gcc or clang
+    if(CMAKE_VERSION VERSION_LESS 2.8.11)
+        set(CMAKE_CXX_FLAGS "${Qt5Widgets_EXECUTABLE_COMPILE_FLAGS} ${CMAKE_CXX_FLAGS}")
+    endif()
+    if(CMAKE_VERSION VERSION_LESS 3.1 AND NOT (Qt5Widgets_VERSION VERSION_LESS 5.7.0)) # CMake < 3.1 && Qt5 >= 5.7.0
+        if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") # GCC or Clang
             set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
         endif()
     endif()

BUG, should be < 2.8.12

arthurzam · 2016-06-19T14:03:19Z

Well that a lot of replies from me :)

find_package(X11_Xv) patch - not needed. This is part of what find_package(X11) does. It also defines all the libs and found variable.
module detection - this (on average) 3 lines of code before every OPTION in /CMakeLists.txt is the code that selects the default state for the OPTION. I can't (!) move it anywhere else before defining the OPTION. On the other hand I can't import /src/modules before defining the OPTION block (cause it depends on it). Possible Solutions are: move this whole block into it's own file (I think the best option), move the module's options in the corresponding CMakeLists.txt (worse because maintaining is a little more tedious to find all of them in that case).
I tried the policy code using include - unsuccessfully. (move to a new file -> include -> error as like no result from this code).
A little about the QUIET keyword in every find_package or pkg-config use: we decided the CMake system will autodetect what the user has and what it doesn't. In case it doesn't we don't need to print the user long error log that it can't find (If you are okay with this, then I will change it). The next find with REQUIRED is for when the user selected porously to enable without the libs - then show the error. The REQUIRED should not be called at all when the USE flag is disabled.
I added the -fPIC by your instructions. As you say I will do. Because I work on a rolling release systems (Arch and Gentoo) I don't have those old versions with all this "magic". So What should I do?
I never tried to call the build with Qt5 when for example Qt5Network is missing. With my experience on newer versions of CMake, including some module (maybe except only Core) includes all the others. And if the user is missing some component, The qt5_use_module should inform about it.
About the fix for Qt4 and Cmake 2.8.11 : this removes the fallback to old Qt5 when no Qt4.
About the fix for compiler flags for Qt >= 5.7.0 - will add them in next commit.

Thank you for creating order in this work by the tasks list in the issue. This is nearly a life saver for me. :)

zaps166 · 2016-06-19T14:10:43Z

About the fix for Qt4 and Cmake 2.8.11 : this removes the fallback to old Qt5 when no Qt4.

Oops 😃

About the fix for compiler flags for Qt >= 5.7.0 - will add them in next commit.

Ok, I've tested it - it adds properly the -fPIC from Qt5 flags.

find_package(X11_Xv) patch - not needed. This is part of what find_package(X11) does. It also defines all the libs and found variable.

Maybe switch to pkg-config for X11?

I tried the policy code using include - unsuccessfully. (move to a new file -> include -> error as like no result from this code).

Ok!

I can't (!) move it anywhere else before defining the OPTION

Ok (I don't know CMake) :)

A little about the QUIET keyword in every find_package or pkg-config use (...)

Just remove the QUIET and don't append REQUIRED :) My proposal (not fully tested, don't know what does it break; also uses pkg-config for XVideo):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7e9bece..d0a58f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,16 +12,6 @@ include(FeatureSummary)
 find_package(PkgConfig REQUIRED)
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/src/cmake/Modules/")

-if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
-    find_package(ALSA QUIET)
-    set(DEFAULT_ALSA ${ALSA_FOUND})
-
-    set(DEFAULT_PORTAUDIO OFF)
-else()
-    set(DEFAULT_ALSA OFF)
-    set(DEFAULT_PORTAUDIO ON)
-endif()
-
 find_package(Qt5Widgets QUIET)
 find_package(Qt4 QUIET)
 if(Qt5Widgets_FOUND AND (NOT Qt4_FOUND OR NOT (Qt5Widgets_VERSION VERSION_LESS 5.6.1)))
@@ -30,74 +20,74 @@ else()
     set(DEFAULT_QT5 OFF)
 endif()

+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+    set(DEFAULT_ALSA ON)
+    set(DEFAULT_AVDEVICE ON)
+    set(DEFAULT_PORTAUDIO OFF)
+else()
+    set(DEFAULT_ALSA OFF)
+    set(DEFAULT_AVDEVICE OFF)
+    set(DEFAULT_PORTAUDIO ON)
+endif()
+
 OPTION(USE_QT5 "Build with Qt5" ${DEFAULT_QT5})
 add_feature_info(Qt5 USE_QT5 "Build with Qt5")
 OPTION(USE_PROSTOPLEER "Build with ProstoPleer support" ON)
 add_feature_info(ProstoPleer USE_PROSTOPLEER "Build with ProstoPleer support")
 OPTION(USE_AVRESAMPLE "Build with support for libavresample" OFF)
 add_feature_info(libavresample USE_AVRESAMPLE "Build with support for libavresample")
-
 OPTION(USE_ALSA "Build with ALSA support" ${DEFAULT_ALSA})
 add_feature_info(ALSA USE_ALSA "Build with ALSA support")
 OPTION(USE_OPENGL "Build with OpenGL support" ON)
 add_feature_info(OpenGL USE_OPENGL "Build with OpenGL support")
 OPTION(USE_PORTAUDIO "Build with PortAudio support" ${DEFAULT_PORTAUDIO})
 add_feature_info(PortAudio USE_PORTAUDIO "Build with PortAudio support")
-
-if(NOT DEFINED USE_TAGLIB)
-    PKG_CHECK_MODULES(TAGLIB taglib QUIET)
-endif()
-OPTION(USE_TAGLIB "Build with tags editor" ${TAGLIB_FOUND})
+OPTION(USE_TAGLIB "Build with tags editor" ON)
 add_feature_info(taglib USE_TAGLIB "Build with tags editor")

 if(NOT DEFINED USE_AUDIOCD)
-    PKG_CHECK_MODULES(LIBCD libcdio libcddb QUIET)
+    PKG_CHECK_MODULES(LIBCD libcdio libcddb)
 endif()
 OPTION(USE_AUDIOCD "Build with AudioCD support" ${LIBCD_FOUND})
 add_feature_info(AudioCD USE_AUDIOCD "Build with AudioCD support")

 if(NOT DEFINED USE_CHIPTUNE_GME)
-    find_package(GME QUIET)
+    find_package(GME)
 endif()
 OPTION(USE_CHIPTUNE_GME "Build Chiptune with GME support" ${LIBGME_FOUND})
 add_feature_info("Chiptune GME" USE_CHIPTUNE_GME "Build Chiptune with GME support")

 if(NOT DEFINED LIBSIDPLAYFP_FOUND)
-    PKG_CHECK_MODULES(LIBSIDPLAYFP libsidplayfp QUIET)
+    PKG_CHECK_MODULES(LIBSIDPLAYFP libsidplayfp)
 endif()
 OPTION(USE_CHIPTUNE_SID "Build Chiptune with SIDPLAY support" ${LIBSIDPLAYFP_FOUND})
 add_feature_info("Chiptune SIDPLAY" USE_CHIPTUNE_SID "Build Chiptune with SIDPLAY support")

 if(NOT DEFINED USE_PULSEAUDIO)
-    PKG_CHECK_MODULES(LIBPULSE libpulse-simple QUIET)
+    PKG_CHECK_MODULES(LIBPULSE libpulse-simple)
 endif()
 OPTION(USE_PULSEAUDIO "Build with PulseAudio support" ${LIBPULSE_FOUND})
 add_feature_info(PulseAudio USE_PULSEAUDIO "Build with PulseAudio support")

 if(NOT DEFINED USE_XVIDEO)
-    find_package(X11 QUIET)
-    if(X11_FOUND AND X11_Xv_FOUND)
-        set(DEFAULT_XVIDEO ON)
-    else()
-        set(DEFAULT_XVIDEO OFF)
-    endif()
+    PKG_CHECK_MODULES(PC_XVIDEO x11 xv)
 endif()
-OPTION(USE_XVIDEO "Build with XVideo support" ${DEFAULT_XVIDEO})
+OPTION(USE_XVIDEO "Build with XVideo support" ${PC_XVIDEO_FOUND})
 add_feature_info(XVideo USE_XVIDEO "Build with XVideo support")

 if(NOT DEFINED USE_FFMPEG_VAAPI)
-    PKG_CHECK_MODULES(LIBS_VAAPI libva libva-x11 QUIET)
+    PKG_CHECK_MODULES(LIBS_VAAPI libva libva-x11)
 endif()
 OPTION(USE_FFMPEG_VAAPI "Build VAAPI acceleration into FFmpeg" ${LIBS_VAAPI_FOUND})
 add_feature_info(VAAPI USE_FFMPEG_VAAPI "Build VAAPI acceleration into FFmpeg")

 if(NOT DEFINED USE_FFMPEG_VDPAU)
-    PKG_CHECK_MODULES(LIBS_VDPAU vdpau x11 QUIET)
+    PKG_CHECK_MODULES(LIBS_VDPAU vdpau x11)
 endif()
 OPTION(USE_FFMPEG_VDPAU "Build VDPAU acceleration into FFmpeg" ${LIBS_VDPAU_FOUND})
 add_feature_info(VDPAU USE_FFMPEG_VDPAU "Build VDPAU acceleration into FFmpeg")

-OPTION(USE_FFMPEG_AVDEVICE "Build FFmpeg with libavdevice suport" ON)
+OPTION(USE_FFMPEG_AVDEVICE "Build FFmpeg with libavdevice suport" ${DEFAULT_AVDEVICE})
 add_feature_info(libavdevice USE_FFMPEG_AVDEVICE "Build FFmpeg with libavdevice suport")

 set(LANGUAGES "All" CACHE STRING "A space-seperated list of translations to compile in to QMPlay2, \"None\", or \"All\".")
@@ -122,6 +112,9 @@ if(USE_QT5)
             set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
         endif()
     endif()
+    if(USE_QT5 AND Qt5Widgets_VERSION VERSION_LESS 5.6.1)
+        message(AUTHOR_WARNING "Qt5 >= 5.6.1 is recommended for stable usage")
+    endif()
 else()
     find_package(Qt4 REQUIRED)
     include("${QT_USE_FILE}")
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 05c3c3a..1513f60 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -3,10 +3,6 @@ cmake_minimum_required(VERSION 2.8.11)
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 set(CMAKE_AUTOMOC ON)

-if(USE_QT5 AND Qt5Widgets_VERSION LESS 5.6.1)
-    message(AUTHOR_WARNING "Qt5 >= 5.6.1 is recommended for stable usage")
-endif()
-
 add_subdirectory(gui)
 add_subdirectory(qmplay2)
 add_subdirectory(modules)
diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt
index 58c6535..8b73d07 100644
--- a/src/gui/CMakeLists.txt
+++ b/src/gui/CMakeLists.txt
@@ -130,7 +130,7 @@ if(INSTALL_PATH_SOLID_ACTION AND NOT INSTALL_PATH_SOLID_ACTION STREQUAL "None")
     if(LOCAL_INSTALL_PATH_SOLID_ACTION)
         install(FILES Unix/solid-actions/QMPlay2-opencda.desktop DESTINATION "${LOCAL_INSTALL_PATH_SOLID_ACTION}")
     else()
-        message(AUTHOR_WARNING "No path found for solid actions - won't be installed")
+        message(WARNING "No path found for solid actions - won't be installed")
     endif()
 endif()

zaps166 · 2016-06-19T14:33:28Z

What about this (fixes CMake 2.8.11 and Qt4):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7e9bece..330dfa6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -23,11 +23,15 @@ else()
 endif()

 find_package(Qt5Widgets QUIET)
-find_package(Qt4 QUIET)
-if(Qt5Widgets_FOUND AND (NOT Qt4_FOUND OR NOT (Qt5Widgets_VERSION VERSION_LESS 5.6.1)))
+if(Qt5Widgets_FOUND AND (Qt5Widgets_VERSION VERSION_GREATER 5.6.0))
     set(DEFAULT_QT5 ON)
 else()
-    set(DEFAULT_QT5 OFF)
+    find_package(Qt4 QUIET)
+    if(NOT Qt4_FOUND AND Qt5Widgets_FOUND)
+        set(DEFAULT_QT5 ON)
+    else()
+        set(DEFAULT_QT5 OFF)
+    endif()
 endif()

 OPTION(USE_QT5 "Build with Qt5" ${DEFAULT_QT5})
@@ -115,7 +119,9 @@ execute_process(
 string(STRIP ${QMPLAY2_VERSION} QMPLAY2_VERSION)

 if(USE_QT5)
-    find_package(Qt5Widgets REQUIRED)
+    if(NOT Qt5Widgets_FOUND)
+        find_package(Qt5Widgets REQUIRED)
+    endif()
     set(Qt5_EXECUTABLE_COMPILE_FLAGS "-fPIC ${Qt5_EXECUTABLE_COMPILE_FLAGS}")
     if(CMAKE_VERSION VERSION_LESS "3.1" AND NOT (Qt5Widgets_VERSION VERSION_LESS 5.7.0)) # cmake < 3.1 && Qt5 >= 5.7.0
         if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" or CMAKE_CXX_COMPILER_ID STREQUAL "Clang") # gcc or clang
@@ -123,7 +129,9 @@ if(USE_QT5)
         endif()
     endif()
 else()
-    find_package(Qt4 REQUIRED)
+    if(NOT Qt4_FOUND)
+        find_package(Qt4 REQUIRED)
+    endif()
     include("${QT_USE_FILE}")
     add_definitions(${QT_DEFINITIONS})
 endif()

?
Checks for Qt5, if can't be used, checks for Qt4. Later checks for REQUIRED only when didn't find it before.

zaps166 · 2016-06-19T14:39:46Z

I never tried to call the build with Qt5 when for example Qt5Network is missing. With my experience on newer versions of CMake, including some module (maybe except only Core) includes all the others. And if the user is missing some component, The qt5_use_module should inform about it.

Ok!

arthurzam · 2016-06-19T15:16:37Z

applied your patches.

zaps166 · 2016-06-19T16:12:35Z

applied your patches.

Ok!

"taglib" -> "TagLib" in message.
~~Maybe use also pkg-config in XVideo module?~~
qmplay2 directory from doc should be removed.
~~GLESv2 for Qt4 and fail with GLESv1.~~

I think that's all for this PR :)

zaps166 · 2016-06-19T17:01:01Z

I don't know about GLES in Qt4, let's handle it later. Maybe I can handle it in C++?

zaps166 · 2016-06-19T17:22:27Z

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 21e35bf..e50d2f1 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,16 +12,18 @@ include(FeatureSummary)
 find_package(PkgConfig REQUIRED)
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/src/cmake/Modules/")

-find_package(Qt5Widgets QUIET)
-if(Qt5Widgets_FOUND AND (Qt5Widgets_VERSION VERSION_GREATER 5.6.0))
-    set(DEFAULT_QT5 ON)
-else()
-    find_package(Qt4 QUIET)
-    if(NOT Qt4_FOUND AND Qt5Widgets_FOUND)
-        set(DEFAULT_QT5 ON)
-    else()
-        set(DEFAULT_QT5 OFF)
-    endif()
+if(NOT DEFINED USE_QT5 OR USE_QT5)
+       find_package(Qt5Widgets QUIET)
+       if(Qt5Widgets_FOUND AND (Qt5Widgets_VERSION VERSION_GREATER 5.6.0))
+               set(DEFAULT_QT5 ON)
+       else()
+               find_package(Qt4 QUIET)
+               if(NOT Qt4_FOUND AND Qt5Widgets_FOUND)
+                       set(DEFAULT_QT5 ON)
+               else()
+                       set(DEFAULT_QT5 OFF)
+               endif()
+       endif()
 endif()

 if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
@@ -47,7 +49,7 @@ add_feature_info(OpenGL USE_OPENGL "Build with OpenGL support")
 OPTION(USE_PORTAUDIO "Build with PortAudio support" ${DEFAULT_PORTAUDIO})
 add_feature_info(PortAudio USE_PORTAUDIO "Build with PortAudio support")
 OPTION(USE_TAGLIB "Build with tags editor" ON)
-add_feature_info(taglib USE_TAGLIB "Build with tags editor")
+add_feature_info(TagLib USE_TAGLIB "Build with tags editor")

 if(NOT DEFINED USE_AUDIOCD)
     PKG_CHECK_MODULES(LIBCD libcdio libcddb)
@@ -112,7 +114,7 @@ if(USE_QT5)
     if(NOT Qt5Widgets_FOUND)
         find_package(Qt5Widgets REQUIRED)
     endif()
-    if(CMAKE_VERSION VERSION_LESS 2.8.11)
+    if(CMAKE_VERSION VERSION_LESS 2.8.12)
         set(CMAKE_CXX_FLAGS "${Qt5Widgets_EXECUTABLE_COMPILE_FLAGS} ${CMAKE_CXX_FLAGS}")
     endif()
     if(CMAKE_VERSION VERSION_LESS 3.1 AND NOT (Qt5Widgets_VERSION VERSION_LESS 5.7.0)) # CMake < 3.1 && Qt5 >= 5.7.0
@@ -120,7 +122,7 @@ if(USE_QT5)
             set(CMAKE_CXX_FLAGS "-std=c++11 ${CMAKE_CXX_FLAGS}")
         endif()
     endif()
-    if(USE_QT5 AND Qt5Widgets_VERSION VERSION_LESS 5.6.1)
+    if(Qt5Widgets_VERSION VERSION_LESS 5.6.1)
         message(AUTHOR_WARNING "Qt5 >= 5.6.1 is recommended for stable usage")
     endif()
 else()
diff --git a/src/cmake/Uninstall.cmake.in b/src/cmake/Uninstall.cmake.in
index 6a21782..3b5357e 100644
--- a/src/cmake/Uninstall.cmake.in
+++ b/src/cmake/Uninstall.cmake.in
@@ -8,6 +8,7 @@ set(DIRS_CLEAN
     "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_INCLUDEDIR@/QMPlay2/"
     "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATAROOTDIR@/qmplay2/lang/"
     "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DATAROOTDIR@/qmplay2/"
+    "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_DOCDIR@"
     "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/qmplay2/modules/"
     "@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@/qmplay2/"
 )
@@ -30,11 +31,6 @@ foreach(file ${files})
     endif()
 endforeach()

-message("")
-
-list(SORT DIRS_CLEAN)
-list(REVERSE DIRS_CLEAN)
-
 foreach(dir ${DIRS_CLEAN})
     file(GLOB DIR_LIST "$ENV{DESTDIR}${dir}/*")
     list(LENGTH DIR_LIST DIR_LIST_LEN)
diff --git a/src/modules/OpenGL2/CMakeLists.txt b/src/modules/OpenGL2/CMakeLists.txt
index 1a84d14..da0c342 100644
--- a/src/modules/OpenGL2/CMakeLists.txt
+++ b/src/modules/OpenGL2/CMakeLists.txt
@@ -32,7 +32,7 @@ if(NOT USE_QT5 OR Qt5Widgets_VERSION LESS 5.6.0)
     list(APPEND OpenGL2_HDR OpenGL2OldWidget.hpp)
     list(APPEND OpenGL2_SRC OpenGL2OldWidget.cpp)

-    if(NOT Qt5Gui_OPENGL_IMPLEMENTATION STREQUAL GLESv2)
+    if(NOT Qt5Gui_OPENGL_IMPLEMENTATION STREQUAL GLESv2) #FIXME (see OpenGL2.pro)
         add_definitions(-DVSYNC_SETTINGS)
     endif()
 else()
@@ -41,7 +41,7 @@ else()
     list(APPEND OpenGL2_SRC OpenGL2Window.cpp OpenGL2Widget.cpp OpenGL2CommonQt5.cpp)
 endif()

-if((USE_QT5 AND Qt5Gui_OPENGL_IMPLEMENTATION STREQUAL GLESv2))
+if(USE_QT5 AND (Qt5Gui_OPENGL_IMPLEMENTATION STREQUAL GLESv2))
     add_definitions(-DOPENGL_ES2)
 endif()

This should be enough, please review, fix (if necessary) and apply. Then I'll merge it :)

zaps166 · 2016-06-19T17:32:03Z

Hmm, what will happen if I push a commit here...?

zaps166 · 2016-06-19T17:32:56Z

Oops... revert...

arthurzam · 2016-06-19T17:33:19Z

Maybe revert history???
git reset

zaps166 · 2016-06-19T17:34:30Z

It is merged, maybe just squash them and force push with a nice commit message?
I should learn more git 😃

arthurzam · 2016-06-19T17:34:43Z

OK!
Will take five minutes.

zaps166 · 2016-06-19T17:35:34Z

I though than I can push a next commit to this PR... 😢 Is it possible?

arthurzam · 2016-06-19T17:37:14Z

The problem is that git supports it. Not github, I think.
It's ok. The amount of code I nearly deleted by mistaken empty push force :)

* fallback to Qt4 if no Qt5 or Qt5 < 5.6.1 * fix finding libgme without pkg-config * detect missing libs and disable options * fix FreeBSD man install location * set FFmpeg minimal version as 2.2.0 * remove Qt5X11Extras dependency * link with x11 when using VDPAU * added ProstoPleer option * install solid actions * print summary of options * support remove qmplay2's unique directories on uninstall * install LICENSE, README.md, TODO and ChangeLog * support CMake >= 2.8.11 PR #24 #27 Task #22

arthurzam force-pushed the master branch from 8bea44a to 902923f Compare June 17, 2016 19:31

arthurzam force-pushed the master branch from 902923f to 2cf8d13 Compare June 18, 2016 05:14

zaps166 added a commit that referenced this pull request Jun 18, 2016

CMake: Enable ProstoPleer

525460c

Caused by: 9aac5d1 This is a temporary solution, see #22 nad #24

zaps166 added a commit that referenced this pull request Jun 18, 2016

CMake: Enable ProstoPleer

2633d78

Caused by: 9aac5d1 This is a temporary solution, see #22 and #24

arthurzam force-pushed the master branch from 837059d to d1df977 Compare June 18, 2016 12:53

arthurzam force-pushed the master branch from 905407a to 87e1876 Compare June 18, 2016 13:24

Fixed options finders

c90f6d6

zaps166 merged commit c90f6d6 into zaps166:master Jun 19, 2016

arthurzam mentioned this pull request Jun 19, 2016

CMake improvements (mainly for Linux and FreeBSD) - ver 2 #27

Merged

zaps166 added the task label Jun 20, 2016

CMake improvements (mainly for Linux and FreeBSD) #24

CMake improvements (mainly for Linux and FreeBSD) #24

Conversation

arthurzam commented Jun 17, 2016 • edited Loading

zaps166 commented Jun 17, 2016

zaps166 commented Jun 17, 2016

arthurzam commented Jun 17, 2016

zaps166 commented Jun 17, 2016

zaps166 commented Jun 17, 2016

zaps166 commented Jun 17, 2016

zaps166 commented Jun 17, 2016

arthurzam commented Jun 18, 2016

arthurzam commented Jun 18, 2016

zaps166 commented Jun 18, 2016

zaps166 commented Jun 18, 2016

zaps166 commented Jun 18, 2016

arthurzam commented Jun 18, 2016

zaps166 commented Jun 18, 2016

arthurzam commented Jun 18, 2016

zaps166 commented Jun 18, 2016

arthurzam commented Jun 18, 2016

zaps166 commented Jun 18, 2016

arthurzam commented Jun 18, 2016 • edited Loading

zaps166 commented Jun 18, 2016

zaps166 commented Jun 18, 2016

arthurzam commented Jun 18, 2016

zaps166 commented Jun 18, 2016 • edited Loading

arthurzam commented Jun 18, 2016

zaps166 commented Jun 19, 2016 • edited Loading

arthurzam commented Jun 19, 2016

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016 • edited Loading

arthurzam commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016

arthurzam commented Jun 19, 2016

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016

zaps166 commented Jun 19, 2016

zaps166 commented Jun 19, 2016

arthurzam commented Jun 19, 2016

zaps166 commented Jun 19, 2016

arthurzam commented Jun 19, 2016 • edited Loading

zaps166 commented Jun 19, 2016 • edited Loading

arthurzam commented Jun 19, 2016 • edited Loading

arthurzam commented Jun 17, 2016 •

edited

Loading

arthurzam commented Jun 18, 2016 •

edited

Loading

zaps166 commented Jun 18, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

arthurzam commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

arthurzam commented Jun 19, 2016 •

edited

Loading

zaps166 commented Jun 19, 2016 •

edited

Loading

arthurzam commented Jun 19, 2016 •

edited

Loading