[qt5] Fix mingw builds #20309

dg0yt · 2021-09-23T17:52:39Z

What does your PR fix?

Fixes building qt5-base ... qt5-tools with a mingw toolchain on Windows hosts.
Which triplets are supported/not supported? Have you updated the CI baseline?

as before + mingw, no
Does your PR follow the maintainer guide?

yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

I am still working on this PR

Depends on #20253 (icu mingw fix).

TODO: Remove absolute compiler paths from tools/qt5/mkspecs/qmodule.pri.

Needed for qttools src/designer/src/uiplugin.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for qt5-declarative but no changes to version or port version.
-- Version: 5.15.2#2
-- Old SHA: 0cef09afb36f9debf22319dc4adccf478c611885
-- New SHA: 58e8a5bc626cdabcbf7be1f8367eafe2faed2cf4
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for qt5-base but no changes to version or port version.
-- Version: 5.15.2#11
-- Old SHA: 72ca286ac98e08f2fef35f85a6e393795428d033
-- New SHA: 56e1bf76bc30ba240b72ede7ef878f189893b8ce
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Neumann-A · 2021-09-23T19:25:17Z

ports/qt5-base/cmake/find_qt_mkspec.cmake

                elseif($ENV{PROCESSOR_ARCHITECTURE} MATCHES "[aA][mM][dD]64")
                    list(APPEND _test_triplets x64-windows x64-windows-static)
                    list(APPEND _test_triplets x86-windows x86-windows-static)
+                    if(VCPKG_TARGET_IS_MINGW)
+                        list(APPEND _test_triplets x64-mingw-dynamic x64-mingw-static)
+                        list(APPEND _test_triplets x86-mingw-dynamic x86-mingw-static)
+                    endif()
                elseif($ENV{PROCESSOR_ARCHITECTURE} MATCHES "x86")
                    list(APPEND _test_triplets x86-windows x86-windows-static)
+                    if(VCPKG_TARGET_IS_MINGW)
+                        list(APPEND _test_triplets x86-mingw-dynamic x86-mingw-static)
+                    endif()


you can probably change the whole code to use host triplets instead. When the code was written host triplets did not exist yet.

You are right. (I didn't want to not revise too much of the code. And I keep stumbling about the host triplet not being knowm in manifest mode. So it did even come to my mind...)

And I keep stumbling about the host triplet not being knowm in manifest mode.

that is only within your CMake code ;). Also you can just workaround that by referencing ${VCPKG_INSTALLED_DIR}/${VCPKG_HOST_TRIPLET} and explicitly setting VCPKG_HOST_TRIPLET and preprend the tool paths to CMAKE_PROGRAM_PATH. But that is a different topic

This whole section is somewhat strange: it is entered only when DEFINED VCPKG_QT_HOST_PLATFORM (a variable not used anywhere else) and NOT "${_tmp_host_out}" MATCHES "${_tmp_host_out}" (what?).

VCPKG_QT_HOST_PLATFORM is a undocumented triplet hook. Just rip the whole thing apart and use the host triplet stuff instead.

Also the check should probably check against _tmp_targ_out instead.

Neumann-A · 2021-09-23T19:28:09Z

ports/qt5-base/portfile.cmake

                            #patches/static_opengl.patch       #Use this patch if you really want to statically link angle on windows (e.g. using -opengl es2 and -static).
                                                               #Be carefull since it requires definining _GDI32_ for all dependent projects due to redefinition errors in the
                                                               #the windows supplied gl.h header and the angle gl.h otherwise.
+                            patches/fix-mingw-create_pc.patch  #Fix qmake mingw generator to write pkgconfig files also for "aux" pro files (qttools/src/designer/src/uiplugin)


why write pc files for plugins?

Because the plugin module name is refered to in other pc files. Because the Unix makefile generator writes them. In Qt, definitions (cflags) carry information about feature availability (that is, at build time).

Because the plugin module name is refered to in other pc files.

that is strange for a plugin. Seems more like a bug in the makefile generation of qmake. Is this only happening for static builds?

In Qt, definitions (cflags) carry information about feature availability (that is, at build time).

Normally this is done by the .prl files for qmake.

Because the plugin module name is refered to in other pc files.

that is strange for a plugin. Seems more like a bug in the makefile generation of qmake. Is this only happening for static builds?

Tested with native x64-mingw-dynamic. And the issue is raired by vcpkg_fixup_pkgconfig.
The point is that this is not a real run-time plugin (TEMPLATE = lib) but just some headers (TEMPLATE = aux). And the patch fixes "makefile generation of qmake", for mingw makefiles. For Unix makefiles, writing pc files for aux is the regular behaviour.

In Qt, definitions (cflags) carry information about feature availability (that is, at build time).

Normally this is done by the .prl files for qmake.

.prl is .pc is config.cmake, each in a world of its own.

Anyway, good to mention .prl here. There is a new port conflict with unnamed .prl files on windows, and I assume it is caused by the patch. I will take another look on the Unix makefile generator.

For reference:

$ cat packages/qt5-tools_x64-linux/lib/pkgconfig/Qt5UiPlugin.pc prefix=${pcfiledir}/../.. exec_prefix=${prefix} libdir=${prefix}/lib includedir=${prefix}/include/qt5 Name: Qt5 UiPlugin Description: Qt UiPlugin module Version: 5.15.2 Cflags: -DQT_UIPLUGIN_LIB -I"${includedir}/QtUiPlugin" -I"${includedir}" Requires: Qt5Core Qt5Gui Qt5Widgets

On x64-linux, there are also lib/libQt5UiPlugin.prl files for debug and release.

I don't see how this pc file is helpful to consumers? In general the header path could be useful but I doubt that anything from there is even required for direct consumers. It is probably indirectly consumed by other Qt modules.

Neumann-A · 2021-09-23T19:28:48Z

ports/qt5-base/portfile.cmake

    -system-sqlite
    -system-harfbuzz
    -icu
-    -no-vulkan


why is this removed?

As written in the commit comment: It is already under control of a feature, lines 150-154.

Neumann-A · 2021-09-23T19:41:59Z

ports/qt5-base/portfile.cmake

+    elseif(VCPKG_TARGET_IS_MINGW)
+        z_vcpkg_get_cmake_vars(cmake_vars_file)
+        debug_message("Including cmake vars from: ${cmake_vars_file}")
+        include("${cmake_vars_file}")
+        list(APPEND CORE_OPTIONS
+            QMAKE_CC="${VCPKG_DETECTED_CMAKE_C_COMPILER}"
+            QMAKE_CXX="${VCPKG_DETECTED_CMAKE_CXX_COMPILER}"
+            QMAKE_LINK="${VCPKG_DETECTED_CMAKE_CXX_COMPILER}"
+            QMAKE_LINK_C="${VCPKG_DETECTED_CMAKE_C_COMPILER}"
+        )
+        if(CMAKE_HOST_WIN32)
+            set(ENV{CC} "${VCPKG_DETECTED_CMAKE_C_COMPILER}")
+            set(ENV{CXX} "${VCPKG_DETECTED_CMAKE_CXX_COMPILER}")


please apply to all platforms not just mingw.
for compiler flags use a approach similar to
Neumann-A@d882040

also this needs to be added to vcpkg_configure_qmake

Setting CC and CXX should be avoided. As far as I understand you do it to make the host tool work. Please try to setup qt5 properly so that it has a host dependency on itself and use the artifacts from there instead.

I would like to avoid changes to platform which I don't know enough about (in particular MSVC), or I don't know well enough how vpckg's flags interfer with Qt's flags.

I agree on vcpkg_configure_qmake. This is what overrides the absolute paths in the installed qmodule.pri.

Refactoring port qt5-base for host dependencies is also out of scope for this PR. AFAIU Qt5 wasn't prepared for this. Such a change should be started and reviewed separately.

I don't know well enough how vpckg's flags interfer with Qt's flags.

It doesn't matter. Qt needs to use vcpkg flags if it wants them or not ;) (the fact that qt6 is fine seems to indicate that it will just work.)

AFAIU Qt5 wasn't prepared for this.

Qt5 itself supports cross-compiling. The scripts just needs adjustment for that. (e.g. no need to run qt5-base build via the bootstrap script). The rest ist just having the correct qt.conf for the job.

please apply to all platforms not just mingw.
for compiler flags use a approach similar to

I noticed "deployment target" linker issues on osx now. Probably another case for passing compiler flags to Qt.

AFAIU Qt5 wasn't prepared for this.

Qt5 itself supports cross-compiling. The scripts just needs adjustment for that. (e.g. no need to run qt5-base build via the bootstrap script). The rest ist just having the correct qt.conf for the job.

Building mingw on Linux, I can now indeed start with the host qmake, skipping configure. However, the mingw triplet will still build all tools for the host. I cannot disable it.

I also need to keep the "host data" in the target triplet. Modules out data into the the mkspecs. But I could redirect the host lib and bin paths in the mingw build so that these binaries could be removed between before the end of the portfile.

Neumann-A · 2021-09-23T19:43:57Z

ports/qt5-base/cmake/qt_port_functions.cmake

+if(CMAKE_HOST_WIN32 AND VCPKG_TARGET_IS_MINGW)
+    vcpkg_acquire_msys(QT_MSYS_ROOT PACKAGES perl)
+    vcpkg_add_to_path(PREPEND "${QT_MSYS_ROOT}/usr/bin")
+    # Override vcpkg_find_acquire_program.
+    find_program(PERL "perl" PATHS "${QT_MSYS_ROOT}/usr/bin" NO_DEFAULT_PATH REQUIRED)
+endif()


Why acquire perl? I would handle mingw just as the linux and osx triplets and assume a setup environment.

I would have to add a condition arround vcpkg_find_acquire(PERL) in another spot, because that perl breaks the build, possibly due to mixing different runtimes. (The common test -d foo || mkdir foo stopped working.) I chose to simply use the perl from the same runtime as msys.

With the current state of vcpkg, no assumptions are made about the mingw toolchain environment. It might be msys mingw64, but it might also be some other "vendor". I would love to see a switch to use the mingw environment which is already on my computer. But this a topic for another PR.

I would have to add a condition arround vcpkg_find_acquire(PERL) in another spot, because that perl breaks the build,

The first thing vcpkg_find_acquire_program does is a find_program call (first within vcpkg downloads then outside.). So if your path is correct it should find the correct version (unless you already got the vcpkg one by running a normal windows triplet using a shared download folder).

The first thing vcpkg_find_acquire_program does is test if the named variable is already set...
Anyway, there is a find_program. And for PERL, it first looks into the downloaded path. However if perl was downloaded and unpacked two months ago, it will be used now 💥

I don't even think there is something wrong with vcpkg_find_acquire_program. It is just a different use case here: For building qt5, I want to use a particular perl bnary which normally isn't found (when the msys root is not prepended to the path).

The current change applies this behaviour to ports using the special functions for building qt5 modules, and excludes normal apps. If this scope is considered too wide, it can be limited to qt5-base. (But I prefer to use the same environment for all qt5 modules.)

And for PERL, it first looks into the downloaded path. However if perl was downloaded and unpacked two months ago, it will be used now

yeah that should be fixed. Probably by introducing a host subpath or something alike.

I want to use a particular perl bnary which normally isn't found (when the msys root is not prepended to the path).

personally I think you are responsible to setup the path correctly in this case. vcpkg really doesn't want to take control of the environment of other platforms than pure windows.

personally I think you are responsible to setup the path correctly in this case. vcpkg really doesn't want to take control of the environment of other platforms than pure windows.

I do my best to control the environment. However, between msys root setup and perl acquisition, there is so much CMake code which is modified by other contributors, and often not with mingw in mind. That's why I want to set PERL early, just after acquiring msys root, before passing control to other code.
What I can do to improve consistency is to call vcpkg_find_acquire_program(PERL) instead of calling find_program(PERL).

Why do you even need perl in all Qt ports? Currently only qt5-base uses perl. Running syncqt is not done for the binary packages of the other qt modules.

What I can do to improve consistency is to call

Do that if it is actually required.

Neumann-A · 2021-09-23T19:44:34Z

scripts/cmake/vcpkg_build_qmake.cmake

+        if(VCPKG_TARGET_IS_MINGW)
+            if(NOT DEFINED QT_MSYS_ROOT) # from qt_port_functions.cmake
+                vcpkg_acquire_msys(QT_MSYS_ROOT)
+            endif()
+            find_program(MAKE "make" PATHS "${QT_MSYS_ROOT}/usr/bin" NO_DEFAULT_PATH REQUIRED)
+            set(INVOKE "${MAKE}")
+        elseif (VCPKG_QMAKE_USE_NMAKE)


Same as perl. I would assume make being available by the system in mingw.

Same answer as for perl, minus the breakage.

dg0yt

Thanks for the quick review!

dg0yt · 2021-09-23T20:42:33Z

ports/qt5-base/cmake/find_qt_mkspec.cmake

                elseif($ENV{PROCESSOR_ARCHITECTURE} MATCHES "[aA][mM][dD]64")
                    list(APPEND _test_triplets x64-windows x64-windows-static)
                    list(APPEND _test_triplets x86-windows x86-windows-static)
+                    if(VCPKG_TARGET_IS_MINGW)
+                        list(APPEND _test_triplets x64-mingw-dynamic x64-mingw-static)
+                        list(APPEND _test_triplets x86-mingw-dynamic x86-mingw-static)
+                    endif()
                elseif($ENV{PROCESSOR_ARCHITECTURE} MATCHES "x86")
                    list(APPEND _test_triplets x86-windows x86-windows-static)
+                    if(VCPKG_TARGET_IS_MINGW)
+                        list(APPEND _test_triplets x86-mingw-dynamic x86-mingw-static)
+                    endif()


You are right. (I didn't want to not revise too much of the code. And I keep stumbling about the host triplet not being knowm in manifest mode. So it did even come to my mind...)

dg0yt · 2021-09-23T20:46:07Z

ports/qt5-base/cmake/qt_port_functions.cmake

+if(CMAKE_HOST_WIN32 AND VCPKG_TARGET_IS_MINGW)
+    vcpkg_acquire_msys(QT_MSYS_ROOT PACKAGES perl)
+    vcpkg_add_to_path(PREPEND "${QT_MSYS_ROOT}/usr/bin")
+    # Override vcpkg_find_acquire_program.
+    find_program(PERL "perl" PATHS "${QT_MSYS_ROOT}/usr/bin" NO_DEFAULT_PATH REQUIRED)
+endif()


I would have to add a condition arround vcpkg_find_acquire(PERL) in another spot, because that perl breaks the build, possibly due to mixing different runtimes. (The common test -d foo || mkdir foo stopped working.) I chose to simply use the perl from the same runtime as msys.

With the current state of vcpkg, no assumptions are made about the mingw toolchain environment. It might be msys mingw64, but it might also be some other "vendor". I would love to see a switch to use the mingw environment which is already on my computer. But this a topic for another PR.

dg0yt · 2021-09-23T20:48:17Z

ports/qt5-base/portfile.cmake

                            #patches/static_opengl.patch       #Use this patch if you really want to statically link angle on windows (e.g. using -opengl es2 and -static).
                                                               #Be carefull since it requires definining _GDI32_ for all dependent projects due to redefinition errors in the
                                                               #the windows supplied gl.h header and the angle gl.h otherwise.
+                            patches/fix-mingw-create_pc.patch  #Fix qmake mingw generator to write pkgconfig files also for "aux" pro files (qttools/src/designer/src/uiplugin)


Because the plugin module name is refered to in other pc files. Because the Unix makefile generator writes them. In Qt, definitions (cflags) carry information about feature availability (that is, at build time).

dg0yt · 2021-09-23T20:49:32Z

ports/qt5-base/portfile.cmake

    -system-sqlite
    -system-harfbuzz
    -icu
-    -no-vulkan


As written in the commit comment: It is already under control of a feature, lines 150-154.

dg0yt · 2021-09-23T21:01:25Z

ports/qt5-base/portfile.cmake

+    elseif(VCPKG_TARGET_IS_MINGW)
+        z_vcpkg_get_cmake_vars(cmake_vars_file)
+        debug_message("Including cmake vars from: ${cmake_vars_file}")
+        include("${cmake_vars_file}")
+        list(APPEND CORE_OPTIONS
+            QMAKE_CC="${VCPKG_DETECTED_CMAKE_C_COMPILER}"
+            QMAKE_CXX="${VCPKG_DETECTED_CMAKE_CXX_COMPILER}"
+            QMAKE_LINK="${VCPKG_DETECTED_CMAKE_CXX_COMPILER}"
+            QMAKE_LINK_C="${VCPKG_DETECTED_CMAKE_C_COMPILER}"
+        )
+        if(CMAKE_HOST_WIN32)
+            set(ENV{CC} "${VCPKG_DETECTED_CMAKE_C_COMPILER}")
+            set(ENV{CXX} "${VCPKG_DETECTED_CMAKE_CXX_COMPILER}")


I would like to avoid changes to platform which I don't know enough about (in particular MSVC), or I don't know well enough how vpckg's flags interfer with Qt's flags.

I agree on vcpkg_configure_qmake. This is what overrides the absolute paths in the installed qmodule.pri.

Refactoring port qt5-base for host dependencies is also out of scope for this PR. AFAIU Qt5 wasn't prepared for this. Such a change should be started and reviewed separately.

dg0yt · 2021-09-23T21:09:47Z

scripts/cmake/vcpkg_build_qmake.cmake

+        if(VCPKG_TARGET_IS_MINGW)
+            if(NOT DEFINED QT_MSYS_ROOT) # from qt_port_functions.cmake
+                vcpkg_acquire_msys(QT_MSYS_ROOT)
+            endif()
+            find_program(MAKE "make" PATHS "${QT_MSYS_ROOT}/usr/bin" NO_DEFAULT_PATH REQUIRED)
+            set(INVOKE "${MAKE}")
+        elseif (VCPKG_QMAKE_USE_NMAKE)


Same answer as for perl, minus the breakage.

JackBoosY · 2022-01-06T16:58:01Z

Any progress here?

dg0yt · 2022-01-06T17:02:11Z

I didn't work on it recently but it is still desired. I want to continue with this PR.

JackBoosY · 2022-07-15T05:33:14Z

Please temporary close this PR if you don't have spare time to finish it.

dg0yt · 2022-07-15T05:38:06Z

It is meant to be visible as a draft until I decide to close it. Would be glad to attract other people.

LilyWangLL · 2022-10-10T07:03:59Z

Ping @dg0yt for response. Is work still being done for this PR?

dg0yt · 2022-10-10T10:15:11Z

Ping @dg0yt for response. Is work still being done for this PR?

Yes. But it is waiting for #23668.

dg0yt · 2023-03-05T15:54:09Z

Resolved by #29505, including contributions based on this PR.

dg0yt added 4 commits September 23, 2021 19:36

[qt5-base] Remove obsolete parameter (control by feature)

cf1fc95

[qt5-base] Fix native mingw build

33f262b

[qt5-declarative] Fix native mingw build

4bf439b

[qt5-base] Fix qmake mingw pc files generation

edfd1bc

Needed for qttools src/designer/src/uiplugin.

github-actions bot requested changes Sep 23, 2021

View reviewed changes

Neumann-A reviewed Sep 23, 2021

View reviewed changes

dg0yt commented Sep 23, 2021

View reviewed changes

JonLiu1993 assigned JackBoosY Sep 24, 2021

JackBoosY added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Sep 24, 2021

dg0yt mentioned this pull request Oct 21, 2021

[scripts-audit] QMake buildsystem #20322

Merged

dg0yt changed the title ~~[qt5] Fix native mingw builds~~ [qt5] Fix mingw builds Apr 18, 2022

JackBoosY assigned LilyWangLL and unassigned JackBoosY Oct 10, 2022

LilyWangLL added the depends:different-pr This PR or Issue depends on a PR which has been filed label Oct 11, 2022

dg0yt mentioned this pull request Nov 8, 2022

[Qt5Core] build failure when cross-compiling from Ubuntu22.04 to Windows using GCC 10-win32 x86_64-w64-mingw32 #27546

Closed

dg0yt mentioned this pull request Feb 15, 2023

Fix mingw32-make with sh koprok/vcpkg#2

Merged

dg0yt closed this Mar 5, 2023

dg0yt deleted the qt5-mingw branch March 5, 2023 15:54

[qt5] Fix mingw builds #20309

[qt5] Fix mingw builds #20309

Uh oh!

Conversation

dg0yt commented Sep 23, 2021

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?