1.4.0: cmake fails because it requires 'adwaita-qt >= 1.4.0'

[kloczek]

Looks like adwaita-qt requires themselves :)

+ /usr/bin/cmake -B x86_64-redhat-linux-gnu -D BUILD_SHARED_LIBS=ON -D CMAKE_AR=/usr/bin/gcc-ar -D CMAKE_BUILD_TYPE=RelWithDebInfo -D CMAKE_C_FLAGS_RELEASE=-DNDEBUG -D CMAKE_CXX_FLAGS_RELEASE=-DNDEBUG -D CMAKE_Fortran_FLAGS_RELEASE=-DNDEBUG -D CMAKE_INSTALL_PREFIX=/usr -D CMAKE_NM=/usr/bin/gcc-nm -D CMAKE_RANLIB=/usr/bin/gcc-ranlib -D CMAKE_VERBOSE_MAKEFILE=ON -D DBUILD_SHARED_LIBS=ON -D INCLUDE_INSTALL_DIR=/usr/include -D LIB_INSTALL_DIR=/usr/lib64 -D LIB_SUFFIX=64 -D SHARE_INSTALL_PREFIX=/usr/share -D SYSCONF_INSTALL_DIR=/etc -S . -D BUILD_TESTING=ON
-- The C compiler identification is GNU 11.2.1
-- The CXX compiler identification is GNU 11.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.8.0")
-- Found XCB: /usr/lib64/libxcb.so (Required is at least version "1.10")
-- Checking for module 'adwaita-qt>=1.4.0'
--   Package dependency requirement 'adwaita-qt >= 1.4.0' could not be satisfied.
Package 'adwaita-qt' has version '1.2.1', required version is '>= 1.4.0'
CMake Error at CMakeLists.txt:55 (message):
  Unable to find Adwaita-qt using PkgConfig

-- Configuring incomplete, errors occurred!

[grulja]

Right, you will have to build with -DBUILD_TESTING=OFF. This has been there for a while and you don't really need to build unit tests. The reason it's done this way is that we actually test the library and whether it can be found using CMake and PkgConfig.

[grulja]

Or first with -DBUILD_TESTING=OFF and then you can enable it, but you don't really need that.

[kloczek]

But why when BUILD_TESTING=ON is checked over pkgconfig adwaita-qt if that library will be provided during the build? Here is IMO the broken logic ..

[grulja]

The idea of this test is to check whether 3rd party apps can use/search for Adwaita-qt with PkgConfig or CMake, therefore I check when I can properly find it, which means our PkgConfig file and CMake files are properly installed and have everything we need for a 3rd party app to use (like it can find headers and link against it).

[kloczek]

So I've removed that detection

--- a//CMakeLists.txt~  2021-08-24 09:41:14.000000000 +0100
+++ b//CMakeLists.txt   2021-08-24 12:02:20.204521241 +0100
@@ -48,15 +48,6 @@

     find_package(Qt5 ${QT_MIN_VERSION} NO_MODULE REQUIRED Test)

-    find_package(PkgConfig REQUIRED)
-    pkg_check_modules(ADWAITAQT adwaita-qt>=${ADWAITAQT_VERSION})
-
-    if (NOT ADWAITAQT_FOUND)
-        message(FATAL_ERROR "Unable to find Adwaita-qt using PkgConfig")
-    endif()
-
-    find_package(AdwaitaQt ${ADWAITAQT_VERSION} REQUIRED)
-
     set(test_SRCS
         tests/test.cpp
     )

to test what will happen and I see that and I see that build of the test suite has no path to in tree headed files

[ 86%] Building CXX object CMakeFiles/test-pkgconfig.dir/tests/test.cpp.o
/usr/bin/g++ -DQT_CORE_LIB -DQT_NO_DEBUG -DQT_TESTCASE_BUILDDIR=\"/home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/x86_64-redhat-linux-gnu\" -DQT_TESTLIB_LIB -I/home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/x86_64-redhat-linux-gnu/test-pkgconfig_autogen/include -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtTest -isystem /usr/include/qt5/QtCore -isystem /usr/lib64/qt5/mkspecs/linux-g++ -O2 -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fdata-sections -ffunction-sections -flto=auto -flto-partition=none -DNDEBUG   -std=c++11 -fPIC -MD -MT CMakeFiles/test-pkgconfig.dir/tests/test.cpp.o -MF CMakeFiles/test-pkgconfig.dir/tests/test.cpp.o.d -o CMakeFiles/test-pkgconfig.dir/tests/test.cpp.o -c /home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/tests/test.cpp
In file included from /home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/tests/test.cpp:24:
/usr/include/AdwaitaQt/adwaita.h:26:10: fatal error: QColor: No such file or directory
   26 | #include <QColor>
      |          ^~~~~~~~
compilation terminated.

and by this it uses syetm installed adwaita.h instead using that files from source tree

[tkloczko@barrel adwaita-qt-1.4.0]$ find . -name adwaita.h
./src/lib/adwaita.h

[kloczek]

The idea of this test is to check whether 3rd party apps can use/search for Adwaita-qt with PkgConfig or CMake, therefore I check when I can properly find it, which means our PkgConfig file and CMake files are properly installed and have everything we need for a 3rd party app to use (like it can find headers and link against it).

IMO such test does not make to much sense. In my entire +25 career I never seen such logic. Effectively you are trying to test pkgconfig :) (I can guarantee you that it works :)) .. which does not make to much sense because you suppose to build all test unit resources used by ctest and that's it .. :)

[kloczek]

IMO all what needs to be done here is just something like

target_include_directories(test PRIVATE ${YOUR_DIRECTORY})

[grulja]

The idea of this test is to check whether 3rd party apps can use/search for Adwaita-qt with PkgConfig or CMake, therefore I check when I can properly find it, which means our PkgConfig file and CMake files are properly installed and have everything we need for a 3rd party app to use (like it can find headers and link against it).

IMO such test does not make to much sense. In my entire +25 career I never seen such logic. Effectively you are trying to test pkgconfig :) (I can guarantee you that it works :)) .. which does not make to much sense because you suppose to build all test unit resources used by ctest and that's it .. :)

I used this just once when making Adwaita-qt a library, I wanted something to quickly check whether the PkgConfig file I wrote is correct, same for CMake files. Haven't used it since then. At this point when I know it works it's basically useless.

[kloczek]

OK so I've done patch:

--- a//CMakeLists.txt~  2021-08-24 09:41:14.000000000 +0100
+++ b//CMakeLists.txt   2021-08-24 12:23:10.819745605 +0100
@@ -48,21 +48,13 @@

     find_package(Qt5 ${QT_MIN_VERSION} NO_MODULE REQUIRED Test)

-    find_package(PkgConfig REQUIRED)
-    pkg_check_modules(ADWAITAQT adwaita-qt>=${ADWAITAQT_VERSION})
-
-    if (NOT ADWAITAQT_FOUND)
-        message(FATAL_ERROR "Unable to find Adwaita-qt using PkgConfig")
-    endif()
-
-    find_package(AdwaitaQt ${ADWAITAQT_VERSION} REQUIRED)
-
     set(test_SRCS
         tests/test.cpp
     )

     add_executable(test-pkgconfig ${test_SRCS})
-    target_link_libraries(test-pkgconfig Qt5::Test ${ADWAITAQT_LIBRARIES})
+    target_link_libraries(test-pkgconfig Qt5::Test AdwaitaQt)
+    target_include_directories(test-pkgconfig PRIVATE src/lib)

     add_executable(test-cmake ${test_SRCS})
     target_link_libraries(test-cmake Qt5::Test AdwaitaQt)

and here is the test suite output:

+ xvfb-run -a /usr/bin/make -O -j48 V=1 VERBOSE=1 -C x86_64-redhat-linux-gnu test ARGS=--output-on-failure
make: Entering directory '/home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/x86_64-redhat-linux-gnu'
Running tests...
/usr/bin/ctest --force-new-ctest-process --output-on-failure
Test project /home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/x86_64-redhat-linux-gnu
    Start 1: TestPkgConfig
1/2 Test #1: TestPkgConfig ....................   Passed    0.05 sec
    Start 2: TestCmake
2/2 Test #2: TestCmake ........................   Passed    0.09 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   0.14 sec
make: Leaving directory '/home/tkloczko/rpmbuild/BUILD/adwaita-qt-1.4.0/x86_64-redhat-linux-gnu'

Feel free to commit above or please let me know if you want me make PR :)

[grulja]

Feel free to commit above or please let me know if you want me make PR :)

PR will be appreciated. Thank you.

[kloczek]

used this just once when making Adwaita-qt a library, I wanted something to quickly check whether the PkgConfig file I wrote is correct, same for CMake files. Haven't used it since then. At this point when I know it works it's basically useless.

I've did not had a look on what test-pkgconfig asi doing however if intention was to test generated and installed pkgconfig and cmake module feels .. no one is doing such tests :/

Nevertheless that TestPkgConfig even if it is about just test compile and ling simpleprogram it proves that just linked DSO is OK :) If that was intention that patch should be a bit corrected probably renamed to provide only one unit like SimpleTest and remove that TestCmake as well because testing pkgconfig files and cmake modules does not make to much sense :)

[grulja]

Or we can conclude such tests are really useless and we can simply get rid of them. As I said, I really used it just once when I wanted to test my changes. Maybe I shouldn't have pushed them in the first place.

[kloczek]

Or we can conclude such tests are really useless and we can simply get rid of them. As I said, I really used it just once when I wanted to test my changes. Maybe I shouldn't have pushed them in the first place.

IMO there is no such thing like useless test :P OK here is corrected version of that patch

--- a/CMakeLists.txt~   2021-08-24 09:41:14.000000000 +0100
+++ b/CMakeLists.txt    2021-08-24 12:41:40.968719274 +0100
@@ -48,25 +48,13 @@

     find_package(Qt5 ${QT_MIN_VERSION} NO_MODULE REQUIRED Test)

-    find_package(PkgConfig REQUIRED)
-    pkg_check_modules(ADWAITAQT adwaita-qt>=${ADWAITAQT_VERSION})
-
-    if (NOT ADWAITAQT_FOUND)
-        message(FATAL_ERROR "Unable to find Adwaita-qt using PkgConfig")
-    endif()
-
-    find_package(AdwaitaQt ${ADWAITAQT_VERSION} REQUIRED)
-
     set(test_SRCS
         tests/test.cpp
     )

-    add_executable(test-pkgconfig ${test_SRCS})
-    target_link_libraries(test-pkgconfig Qt5::Test ${ADWAITAQT_LIBRARIES})
-
-    add_executable(test-cmake ${test_SRCS})
-    target_link_libraries(test-cmake Qt5::Test AdwaitaQt)
+    add_executable(adwaitqt-simple-test ${test_SRCS})
+    target_link_libraries(adwaitqt-simple-test Qt5::Test AdwaitaQt)
+    target_include_directories(adwaitqt-simple-test PRIVATE src/lib)

-    add_test(NAME TestPkgConfig COMMAND test-pkgconfig)
-    add_test(NAME TestCmake COMMAND test-cmake)
+    add_test(NAME AdwaitaQtSimpleTest COMMAND adwaitqt-simple-test)
 endif()

When you will be happy about that patch than I'll make PR :)

[grulja]

In this form it doesn't really test anything we need to test, the point was to use CMake/PkgConfig to test what we install. Testing it this way is no different from using the Adwaita library by the Adwaita style. The style also uses Adwaita headers and links against it.

[kloczek]

OK I see that you are not-so-happy about the change which I've proposed :P No problem .. I'll keep it in my rpm as better-than-nothing(tm) :)

BTW I made yet another test with my final form of the rpm package about check compile/linking warnings. Here are summary stats:

[tkloczko@barrel SPECS]$ rpmbuild -ba  adwaita-qt.spec --quiet 2>&1 | grep -- -W | sed 's/.*\[//; s/\]//' | sort | uniq -c | sort -nr
     42 -Wswitch
     37 -Wunused-function
     32 -Wunused-variable

at least those -Wunused-function and -Wunused-variable warnings should be easy to correct :P

[grulja]

OK I see that you are not-so-happy about the change which I've proposed :P No problem .. I'll keep it in my rpm as better-than-nothing(tm) :)

Don't take me wrong, those changes are completely okay, I just think with those changes we don't need such test at all. With your changes we test to link and build against Adwaita-qt library as provided by the source, which is the same thing we "test" when we link and build against the library with the style itself.

BTW I made yet another test with my final form of the rpm package about check compile/linking warnings. Here are summary stats:

[tkloczko@barrel SPECS]$ rpmbuild -ba  adwaita-qt.spec --quiet 2>&1 | grep -- -W | sed 's/.*\[//; s/\]//' | sort | uniq -c | sort -nr
     42 -Wswitch
     37 -Wunused-function
     32 -Wunused-variable

at least those -Wunused-function and -Wunused-variable warnings should be easy to correct :P

Yes, I will look into those. Thank you.

[grulja]

Done https://github.com/FedoraQt/adwaita-qt/commit/c46757375c4ff39683355f6fe9b623eb55e96eab and I also removed the test https://github.com/FedoraQt/adwaita-qt/commit/4511fef31619426cebbdc8a7595ee93ad411da27.