[qtbase] Fix a few details #38682

Neumann-A · 2024-05-10T17:15:38Z

Fix control of cups dependency
Fix binary and directory name collision in dynamic builds by not deploy plugins into tools/Qt6/bin (wasn't necessary in the first place due to qt.conf working as expected) (closes [qttools] Build error with qml option using x64-linux-dynamic triplet #28340)
(New) Fix deploy script on windows (closes [qttools] Install error. #38936)
Fix dbus linkage as described here [qtbase] Fix a few details #38682 (comment)
Fix qtwebengine resource location to be in line what is stated in the generated qt.conf. There is probably a variable to control the installation location but moving was simpler then trying to find that variable. You will only notice it if you actually try to run a program using QtWebEngineProcess with the same qt.conf

Neumann-A · 2024-05-10T17:16:36Z

@tsondergaard: Anything more which needs fixing in qtbase?

tsondergaard · 2024-05-10T21:44:18Z

@tsondergaard: Anything more which needs fixing in qtbase?

I dunno, let me have a look around.... All I can spot are two oddities:

First issue

This first one is just a question: In qtbase/vcpkg.json I see this:

    {
      "name": "qtbase",
      "default-features": false,
      "features": [
        "doubleconversion"
      ]
    },

There is no "platform" property so if I am not mistaken this is an unconditional self-dependency with feature doubleconversion. What is the purpose of this? Is it a well-known pattern of some sort?

Second issue

The statements list(APPEND FEATURE_CORE_OPTIONS -DCMAKE_DISABLE_FIND_PACKAGE_PPS:BOOL=ON) and list(APPEND FEATURE_CORE_OPTIONS -DCMAKE_DISABLE_FIND_PACKAGE_Slog2:BOOL=ON) are duplicated. Probably not intended.

dg0yt · 2024-05-12T16:35:56Z

Anything more which needs fixing in qtbase?

Fix dbus installation order problems. Enforce "linked" on linux to match at-spi2-core (or similiar deps), and "runtime" everywhere else. Would unblock #38699.
Done for qt5-base in #38058.

Neumann-A · 2024-05-12T17:48:36Z

@data-queue: All the windows regressions here are due to adding cppwinrt to qtbase. So adding it is not really an option since it requires adding the vcpkg include folder to the default include folders (which should then be done by the cppwinrt port).

equeim · 2024-05-12T18:48:42Z

@data-queue: All the windows regressions here are due to adding cppwinrt to qtbase. So adding it is not really an option since it requires adding the vcpkg include folder to the default include folders (which should then be done by the cppwinrt port).

Another problem with cppwinrt port is that C++/WinRT has a link-time check that ensures that all object files in a binary were compiled with the same version of C++/WinRT. If some used one from Windows SDK and some from vcpkg then you will get linker error. This means that all ports that use C++/WinRT from Windows SDK internally (which is likely not easy to check) should be updated to depend on cppwinrt from vcpkg, otherwise it will create problems for users (and they will need to add dependency on cppwinrt in their apps too if they use C++/WinRT). IDK if this is a known problem in vcpkg, it just something I encountered personally.

Neumann-A · 2024-05-13T05:53:09Z

This means that all ports that use C++/WinRT from Windows SDK internally (which is likely not easy to check) should be updated to depend on cppwinrt from vcpkg, otherwise it will create problems for users (and they will need to add dependency on cppwinrt in their apps too if they use C++/WinRT)

The problem is that it is not enough to add the dependency. You actually need to setup the include and probably lib search directory ${VCPKG_INSTALLED_DIR}/(include|/(debug/)?lib) for that. Otherwise I wouldn't see the failures in cmake here since qtbase already pulls in cppwinrt here but the build itself has not setup the search directories. In the end it is a failure of the cppwinrt port not correctly overruling the VS installed version.

ports/qtbase/portfile.cmake

ports/qtbase/cmake/qt_install_submodule.cmake

Neumann-A · 2024-05-15T05:24:58Z

@BillyONeal: https://github.com/microsoft/vcpkg/runs/24854382133 is the CI run where cppwinrt is a dependency of Qt. Only static triplets fail. IT has error like:

qwindowsd.lib(qwindowsintegration.cpp.obj) : error LNK2038: mismatch detected for 'C++/WinRT version': value '2.0.220110.5' doesn't match value '2.0.240111.5' in Qt6Cored.lib(qlocale_win.cpp.obj)
qwindowsd.lib(qwindowscontext.cpp.obj) : error LNK2038: mismatch detected for 'C++/WinRT version': value '2.0.220110.5' doesn't match value '2.0.240111.5' in Qt6Cored.lib(qlocale_win.cpp.obj)
qwindowsd.lib(qwindowstheme.cpp.obj) : error LNK2038: mismatch detected for 'C++/WinRT version': value '2.0.220110.5' doesn't match value '2.0.240111.5' in Qt6Cored.lib(qlocale_win.cpp.obj)

The reason for these errors is easy to explain. Qt6Core has a lot of dependencies and one of those dependencies adds ${VCPKG_INSTALLED_DIR}/${TARGET_TRIPLET}/include to the include search paths. As soon as that include is added it will use and find the headers from the cppwinrt port. The qwindows plugin however does not depend on any other external dependencies (why should it? It assumes everything is provided by the winsdk which is correct) so it does not get any includes from vcpkg. As such it will use the VS installed headers and won't see the cppwinrt ports headers.

This is not only a problem for Qt but every downstream user which is not adding ${VCPKG_INSTALLED_DIR}/${TARGET_TRIPLET}/include as an include path.

ports/qtwebengine/portfile.cmake

tsondergaard · 2024-05-15T12:13:58Z

ports/qtbase/vcpkg.json

@@ -112,9 +117,18 @@
        }
      ]
    },
+    "cups": {
+      "description": "Provides support for the Common Unix Printing System.",


If I understand correctly this means the application will be linked with the system provided version of cups, as there is no cups port in vcpkg.git. In Qt 5 I believe cups is only linked into plugins/printsupport/libcupsprintersupport.so and it is probably the same in Qt 6. However, when libcupsprintersupport.so is loaded into the process it brings with it a bunch of shared libraries.

On RHEL 9.4 the distribution provided libcups.so pulls in an unpleasant amount of shared libraries:

# ldd /usr/lib64/libcups.so.2 linux-vdso.so.1 (0x00007fffb9be8000) libgssapi_krb5.so.2 => /lib64/libgssapi_krb5.so.2 (0x00007fdef2c71000) libavahi-common.so.3 => /lib64/libavahi-common.so.3 (0x00007fdef2c63000) libavahi-client.so.3 => /lib64/libavahi-client.so.3 (0x00007fdef2c4e000) libgnutls.so.30 => /lib64/libgnutls.so.30 (0x00007fdef2a18000) libz.so.1 => /lib64/libz.so.1 (0x00007fdef29fe000) libm.so.6 => /lib64/libm.so.6 (0x00007fdef2923000) libc.so.6 => /lib64/libc.so.6 (0x00007fdef2718000) libkrb5.so.3 => /lib64/libkrb5.so.3 (0x00007fdef263d000) libk5crypto.so.3 => /lib64/libk5crypto.so.3 (0x00007fdef2624000) libcom_err.so.2 => /lib64/libcom_err.so.2 (0x00007fdef261d000) libkrb5support.so.0 => /lib64/libkrb5support.so.0 (0x00007fdef260c000) libkeyutils.so.1 => /lib64/libkeyutils.so.1 (0x00007fdef2605000) libcrypto.so.3 => /lib64/libcrypto.so.3 (0x00007fdef21d0000) libresolv.so.2 => /lib64/libresolv.so.2 (0x00007fdef21bc000) libdbus-1.so.3 => /lib64/libdbus-1.so.3 (0x00007fdef2169000) libp11-kit.so.0 => /lib64/libp11-kit.so.0 (0x00007fdef1fd2000) libidn2.so.0 => /lib64/libidn2.so.0 (0x00007fdef1fb1000) libunistring.so.2 => /lib64/libunistring.so.2 (0x00007fdef1e2c000) libtasn1.so.6 => /lib64/libtasn1.so.6 (0x00007fdef1e12000) libnettle.so.8 => /lib64/libnettle.so.8 (0x00007fdef1dbb000) libhogweed.so.6 => /lib64/libhogweed.so.6 (0x00007fdef1d23000) /lib64/ld-linux-x86-64.so.2 (0x00007fdef2d73000) libselinux.so.1 => /lib64/libselinux.so.1 (0x00007fdef1cf6000) libsystemd.so.0 => /lib64/libsystemd.so.0 (0x00007fdef1c19000) libffi.so.8 => /lib64/libffi.so.8 (0x00007fdef1c0b000) libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x00007fdef1b6f000) libcap.so.2 => /lib64/libcap.so.2 (0x00007fdef1b65000) libgcrypt.so.20 => /lib64/libgcrypt.so.20 (0x00007fdef1a2c000) liblzma.so.5 => /lib64/liblzma.so.5 (0x00007fdef1a00000) libzstd.so.1 => /lib64/libzstd.so.1 (0x00007fdef1929000) liblz4.so.1 => /lib64/liblz4.so.1 (0x00007fdef1903000) libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fdef18e8000) libgpg-error.so.0 => /lib64/libgpg-error.so.0 (0x00007fdef18c2000)

So a Qt application that dynamically loads the libcups.so.2 plugin will have two versions loaded of several of these libraries - one version from the system and one version from vcpkg. That sounds concerning to me.

tsondergaard

Changes look good to me. The CUPS support looks questionable to me, but the PR doesn't make it worse, it improves the situation by disabling CUPS in more situations and when it is enabled it should work as well as before.

Neumann-A · 2024-05-17T07:48:55Z

@FrankXie05 waiting for review

BillyONeal · 2024-05-25T02:23:25Z

Can you explain what specifically is being fixed here?

…nto fix_qt_details

# Conflicts: # versions/q-/qtwebengine.json

Neumann-A · 2024-05-25T06:59:41Z

@BillyONeal:

Fix control of cups dependency
Fix binary and directory name collision in dynamic builds by not deploy plugins into tools/Qt6/bin (wasn't necessary in the first place due to qt.conf working as expected) (closes [qttools] Build error with qml option using x64-linux-dynamic triplet #28340)
(New) Fix deploy script on windows (closes [qttools] Install error. #38936)
Fix dbus linkage as described here [qtbase] Fix a few details #38682 (comment)
Fix qtwebengine resource location to be in line what is stated in the generated qt.conf. There is probably a variable to control the installation location but moving was simpler then trying to find that variable. You will only notice it if you actually try to run a program using QtWebEngineProcess with the same qt.conf

vicroms

Just a minor nit that I feel would make the political correctness check happier. Otherwise looks good to me. Feel free to disregard if you think that it is not worth the vcpkg x-add-version run.

vicroms · 2024-05-28T06:32:07Z

ports/qtbase/cmake/qt_install_submodule.cmake

@@ -150,7 +150,7 @@ function(qt_cmake_configure)
        ${disable_parallel}
        OPTIONS
            -DQT_NO_FORCE_SET_CMAKE_BUILD_TYPE:BOOL=ON
-            -DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to screw with users toolchain settings.
+            -DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to mess with users toolchain settings.


interfere

Suggested change

-DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to mess with users toolchain settings.

-DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to interfere with users toolchain settings.

On second thought, let's change this the next time, no point blocking over this and triggering a new run.

Neumann-A added 4 commits May 10, 2024 19:11

Remove pulgin deployment into tools. Using qt.conf will work

79d08e4

fix Cups casing

4bec3d8

bump port version

7aced28

v db

03215ea

Neumann-A added 2 commits May 10, 2024 23:32

make osx work

f54f0d3

v db

1918363

JonLiu1993 assigned FrankXie05 May 11, 2024

FrankXie05 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label May 11, 2024

Neumann-A added 3 commits May 11, 2024 17:17

remove duplicated options

2bdb1a0

add cppwinrt as a dependency

eaff4f8

v db

9547c97

dg0yt reviewed May 13, 2024

View reviewed changes

ports/qtbase/portfile.cmake Outdated Show resolved Hide resolved

Neumann-A added 4 commits May 13, 2024 08:05

remove cppwinrt dependency

2fb1516

control dbus linkage on linux

bba4ed2

remove trailing ,

ae08c1b

v db

e7f2104

equeim mentioned this pull request May 13, 2024

[qcoro] Depend on only required Qt features #38699

Merged

7 tasks

tsondergaard reviewed May 13, 2024

View reviewed changes

ports/qtbase/cmake/qt_install_submodule.cmake Show resolved Hide resolved

Neumann-A added 3 commits May 14, 2024 07:31

fix qtdeclarative now depending on qtsvg

18206b3

dbus only for linux

750199e

political correctness wording

d6f01eb

dg0yt mentioned this pull request May 14, 2024

Minor updates to comments for some verbiage that gets flagged by political correctness tools #38719

Merged

Neumann-A added 2 commits May 14, 2024 10:40

Add cups as a default feature

86334f0

v db

7d59d59

Neumann-A commented May 15, 2024

View reviewed changes

ports/qtwebengine/portfile.cmake Outdated Show resolved Hide resolved

Neumann-A added 2 commits May 15, 2024 07:27

Fix typo

210b592

v db

598db88

Neumann-A mentioned this pull request May 15, 2024

[qtdeclarative] fails post build checks if qtsvg is installed? #38720

Closed

tsondergaard reviewed May 15, 2024

View reviewed changes

tsondergaard approved these changes May 15, 2024

View reviewed changes

tsondergaard mentioned this pull request May 15, 2024

Draft: [qtwebengine] fix dyn builds #38681

Draft

MonicaLiu0311 mentioned this pull request May 16, 2024

[qtdeclarative] Fix post-build check problem #38738

Closed

7 tasks

FrankXie05 previously approved these changes May 20, 2024

View reviewed changes

FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label May 20, 2024

Merge remote-tracking branch 'upstream/master' into fix_qt_details

043da0a

This was referenced May 21, 2024

[qt] update to 6.7.1 #38843

Closed

[qt] Update qt 6.7.1 #38845

Merged

Merge branch 'microsoft:master' into fix_qt_details

826de0f

Neumann-A added 5 commits May 25, 2024 08:45

Merge branch 'fix_qt_details' of https://github.com/Neumann-A/vcpkg i…

1d94895

…nto fix_qt_details

Merge remote-tracking branch 'upstream/master' into fix_qt_details

0700012

# Conflicts: # versions/q-/qtwebengine.json

bump port version after merge

51332d3

add patch to fix deployment script on windows

74d32cf

v db

8847482

Neumann-A dismissed FrankXie05’s stale review via 8847482 May 25, 2024 06:48

Cheney-W added the category:port-bug The issue is with a library, which is something the port should already support label May 27, 2024

vicroms requested changes May 29, 2024

View reviewed changes

vicroms approved these changes May 29, 2024

View reviewed changes

vicroms merged commit 26254b9 into microsoft:master May 29, 2024
17 checks passed

Neumann-A deleted the fix_qt_details branch May 29, 2024 08:36

tsondergaard mentioned this pull request May 30, 2024

[qtbase] qdbuscpp2xml subtly broken on Windows by recent port update #39040

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[qtbase] Fix a few details #38682

[qtbase] Fix a few details #38682

Neumann-A commented May 10, 2024 •

edited

Loading

Neumann-A commented May 10, 2024

tsondergaard commented May 10, 2024

dg0yt commented May 12, 2024

Neumann-A commented May 12, 2024

equeim commented May 12, 2024

Neumann-A commented May 13, 2024

Neumann-A commented May 15, 2024

tsondergaard May 15, 2024

tsondergaard left a comment

Neumann-A commented May 17, 2024

BillyONeal commented May 25, 2024

Neumann-A commented May 25, 2024 •

edited

Loading

vicroms left a comment •

edited

Loading

vicroms May 28, 2024

vicroms May 29, 2024

	-DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to mess with users toolchain settings.
	-DQT_USE_DEFAULT_CMAKE_OPTIMIZATION_FLAGS:BOOL=ON # We don't want Qt to interfere with users toolchain settings.

[qtbase] Fix a few details #38682

[qtbase] Fix a few details #38682

Conversation

Neumann-A commented May 10, 2024 • edited Loading

Neumann-A commented May 10, 2024

tsondergaard commented May 10, 2024

First issue

Second issue

dg0yt commented May 12, 2024

Neumann-A commented May 12, 2024

equeim commented May 12, 2024

Neumann-A commented May 13, 2024

Neumann-A commented May 15, 2024

tsondergaard May 15, 2024

Choose a reason for hiding this comment

tsondergaard left a comment

Choose a reason for hiding this comment

Neumann-A commented May 17, 2024

BillyONeal commented May 25, 2024

Neumann-A commented May 25, 2024 • edited Loading

vicroms left a comment • edited Loading

Choose a reason for hiding this comment

vicroms May 28, 2024

Choose a reason for hiding this comment

vicroms May 29, 2024

Choose a reason for hiding this comment

Neumann-A commented May 10, 2024 •

edited

Loading

Neumann-A commented May 25, 2024 •

edited

Loading

vicroms left a comment •

edited

Loading