Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update qt5 to 5.12.4 #6956

Closed
wants to merge 9 commits into from
Closed

Conversation

TheScarfix
Copy link
Contributor

No description provided.

@skychef
Copy link

skychef commented Jun 20, 2019

qt5-declarative:x64-windows failing happens when compiling on 64 bit Windows here too.
vcpkg doesn't offer any information in the fail logs.

The Windows test failing is not really at all the expected behaviour of a LTS release. Lets hope it's not a hallmark of the quality of this release by Qt.

@TheScarfix
Copy link
Contributor Author

@skychef yeah but I don't think it is Qt's fault. The port in vcpkg modifies nearly everything to get it to the format vcpkg needs. And sometimes stuff randomly breaks because a script doesn't work correctly anymore. Like now the qt5-declarative doesn't seem to find the qtmaind.lib inside the manual-link folder.

@Rastaban
Copy link
Contributor

Do the other qt5-* ports need updated too?

@TheScarfix
Copy link
Contributor Author

since qt5-declarative is a dependency for most of the other ports I need to fix this one first before trying the rest

@TheScarfix
Copy link
Contributor Author

thinking about changing the behaviour of the portfile with the manual-link folder. In my opinion this is not needed because if you really want to build an activex server you need to do more work and I don't think CMake supports that (https://doc.qt.io/qt-5/activeqt-server.html#). Then again there is a CMake target property to not link qtmain.lib automatically so you maybe could do it manually in CMake but even then the manual-link folder is superfluous.

@skychef
Copy link

skychef commented Jun 23, 2019

@skychef yeah but I don't think it is Qt's fault.

If Qt have changed the location of a file in a point release (in an LTS version,) then the term "Long term support" has utterly lost it's meaning on them. The understanding in the industry is that software is built via CI systems and the point in picking an LTS release is to among other things, ensure that time isn't wasted "fixing" a build chain broken by the event that a point release of a dependent piece of software becomes available.

LTS point releases receive: security updates, bug fixes and minor optimisations. This includes backported patches (although Qt have backported very little in this point release from 5.13 - I've been watching their git repos these past few weeks.)
LTS releases are this way because in Enterprise deployments we don't have the time to waste a day "fixing" a build chain due to someone's whim to change a file location - as for instance appears to be the case here.

The definition of LTS is pretty clear and universally understood. And we pick LTS releases so that we can have increased confidence that point releases are trustworthy and that we can pull them without issue.

So, it is the guys at Qt's fault that this LTS point release does not build using the same build tool (vcpkg) as the previous point release. Because they changed something which was not at all appropriate for a Windows LTS release.
The term "LTS" doesn't need redefinition simply because some people were careless.
And if it is just the file, while on Linux you could just symlink, the option is unavailable on Windows.

Like now the qt5-declarative doesn't seem to find the qtmaind.lib inside the manual-link folder.

qt5-activeqt failed to build too.
https://dev.azure.com/vcpkg/public/_build/results?buildId=3478

If it's an includes issue, then adding the include folder to the path after the archive has been extracted may do it. Just a guess though. I noticed that vcpkg doesn't add includes to the path by default.

@Neumann-A
Copy link
Contributor

Then again there is a CMake target property to not link qtmain.lib automatically so you maybe could do it manually in CMake but even then the manual-link folder is superfluous.

manual-link folder is required because of vcpkg autolink feature in the Visual Studio integration. (Has nothing to do with CMake)

@TheScarfix
Copy link
Contributor Author

TheScarfix commented Jun 27, 2019

@Neumann-A the problem I now have is that qt5-declarative and possibly other packages don't find qtmain 🤔 wonder why this is suddenly a problem when all I changed was the version number

@Neumann-A
Copy link
Contributor

@TheScarfix

other packages

You mean other ports than Qt? Than probably something is wrong in the Qt targets and must be fixed.

@heydojo
Copy link
Contributor

heydojo commented Jun 28, 2019

You mean other ports than Qt?

I think he is referring to Qt plugins (such as QtSvg, for instance.)
Qt has a core and a set of modules which Qt calls plugins.
vcpkg has individual ports for the individual plugins. You wouldn't build world for Qt unless you had no idea how to use the toolkit or were running continuous builds on a build farm.

Various Qt plugins have dependencies upon one another and all plugins depend upon Qt's core.
Not all plugins depend upon every other plugin. This allows Qt's build size to scale to your application's needs when you only build (or install) the core and the modules which your application actually needs.
Further than that, deployment tools such as windeployqt on Windows pickup the core, the modules and other depends and allow you as a developer to package up only the bits your application needs for you in one command and then places them in one folder combined together. I will add too that windeployqt is broken in Qt built using vcpkg.

The common sense approach to this issue is to look at the git history between 5.12.3 and 5.12.4 and revert the commit(s) causing the mess using a patch, after archive extraction has been completed by the portfile.
(Followed by sending a patch upstream fixing it if need be.)

https://github.com/qt/qtbase/commits/5.12.4?after=d8efc8d718e3b3a0464f321e740541f5b221a5d6+174

the problem I now have is that qt5-declarative and possibly other packages don't find qtmain 🤔 wonder why this is suddenly a problem when all I changed was the version number

I may have found it:
qt/qtbase@c291724
That's not the kind of change I would expect in an LTS release. YMMV.

@heydojo
Copy link
Contributor

heydojo commented Jun 29, 2019

Pulling from git might be a better approach than grabbing an official archive IMO.

 ## Pull the source from github

    vcpkg_from_github(
        ## (https://github.com/qt/qtbase)
        OUT_SOURCE_PATH SOURCE_PATH
        REPO qt/qtbase
        REF d8efc8d718e3b3a0464f321e740541f5b221a5d6
        HEAD_REF 5.12.4
        SHA512 someverylongsha512hashstringinsteadhere
    )

    ## Obtain perl so that the includes folder can be created (only required if the sources have been acquired from git)

    vcpkg_find_acquire_program(PERL)
    get_filename_component(PERL_EXE_PATH ${PERL} DIRECTORY)
    set(ENV{PATH} "${PERL_EXE_PATH};$ENV{PATH}")

    ## Run the syncqt.pl script to generate the includes directory (only required if the sources have been acquired from git)

	vcpkg_execute_required_process(
		# For packages other than qtbase
		#COMMAND perl ${VCPKG_ROOT_DIR}/packages/qt5-base_x64-windows/tools/qt5/syncqt.pl -version ${FULL_VERSION}
		COMMAND perl ${SOURCE_PATH}/bin/syncqt.pl -version ${FULL_VERSION}
		WORKING_DIRECTORY ${SOURCE_PATH}
	)

set(ENV{INCLUDEPATH} "${SOURCE_PATH}/include/;$ENV{INCLUDEPATH}")

Once syncqt.pl is run on the sources pulled from github, the generated include folder is added to the PATH before a build is started.

Maybe a syncqt.pl function could be added to qt5-modularscripts?

The path:

${VCPKG_ROOT_DIR}/packages/qt5-base_x64-windows/tools/qt5/syncqt.pl

Should be changed to where syncqt.pl ends up when used in conjunction with other plugins. The variable escapes me right now.

@TheScarfix
Copy link
Contributor Author

TheScarfix commented Jul 1, 2019

problem is that the manual-link folder is created after build to prevent issues with the activeqt server. Building the base is fine, everything else after that doesn't find qtmain because it is in the manual-link folder. Would it be easier to not use the manual-link folder and fix the autolink in another way?

@JackBoosY
Copy link
Contributor

JackBoosY commented Jul 8, 2019

Try not to use manual links.
See #7108 changes.

@heydojo
Copy link
Contributor

heydojo commented Jul 18, 2019

According to appveyor/ci#2923 (comment)

Qt 5.12.4 requires OpenSSL 1.1.1

Qt 5.12.3 in vcpkg is being built against Openssl 1.0.2.

@nschimme
Copy link

@heydojo source for OpenSSL 1.1.1a requirement is https://blog.qt.io/blog/2019/06/17/qt-5-12-4-released-support-openssl-1-1-1/

@Rastaban
Copy link
Contributor

@TheScarfix getting rid of the manual-link folder and fix autolink another way makes sense to me.

@heydojo
Copy link
Contributor

heydojo commented Jul 20, 2019

@heydojo source for OpenSSL 1.1.1a requirement is https://blog.qt.io/blog/2019/06/17/qt-5-12-4-released-support-openssl-1-1-1/

Thanks. And from your link:

As an important new item it provides binaries build with OpenSSL 1.1.1, including the new TLS 1.3 functionality.

The update to OpenSSL 1.1.1 is important to note for users leveraging OpenSSL in their applications.

Unfortunately OpenSSL 1.1 is binary incompatible with 1.0, so users need to switch to the new one and repackage their applications.

The blog is talking about the pre-compiled binaries which Qt ship and not at all about the source code - as far as I can see.

So :

Please note that Qt 5.12.4 requires OpenSSL 1.1.1 as opposed to OpenSSL 1.0.2 in previous 5.12.x versions. This will most likely cause user builds to fail...

Only applies to the applications you build against pre-compiled Qt binaries you obtain from Qt via their installer and not Qt binaries you build yourself - meaning that it is still possible to compile 5.12.4 against Openssl 1.0.2 and build application against that AFAIK.

That aside, moving to OpenSSL 1.1.1 now rather than later is extremely important IMO. And if you are using cmake to build your Qt application, migrating appears to be fairly trivial.

https://gitlab.com/TW3/bhawk/blob/staging/ports/vcpkg/openssl-windows/portfile.cmake#L3

@Rastaban
Copy link
Contributor

Ug, I need to reverse my earlier recommendation. After discussing it with other members of the team I found out we have a (currently undocumented) policy to move all misbehaving libraries to a manual-links directory. A library is considered misbehaving if it defines main, defines symbols that are also defined in other libraries, or defines malloc. This is done because some build systems will attempt to link against everything in the libs directory. I am working on updating our documentation to make this policy clear.

You can also see qt5-base\fixmake.py and scripts\cmake\vcpkg_build_qmake.cmake for more manual-links handling.

@Neumann-A
Copy link
Contributor

Neumann-A commented Aug 7, 2019

so i took a look at it and would say it would be enough to patch winmain.pro
from
DESTDIR = $$QT.core.libs
to
DESTDIR = $$QT.core.libs/manual-link

and additionally change the install location somehow. (in the pro load(qt_installs) must be changed but maybe it is enough to change the generated makefiles.)

qt_installs.prf is located in mkspecs\features. So copy that an make a qt_install_manual_link.prf
and just edit the pro file. qmake should take care of the rest. This probably also allows us to get rid of some of the fixcmake stuff etc...

edit: no need to copy just insert after load(qt_installs):

!qt_no_install_library {
    win32 {
       host_build: \
           dlltarget.path = $$[QT_HOST_BINS]/manual-link
       else: \
           dlltarget.path = $$[QT_INSTALL_BINS]/manual-link
       INSTALLS += dlltarget
    }
    host_build: \
        target.path = $$[QT_HOST_LIBS]/manual-link
    else: \
        target.path = $$[QT_INSTALL_LIBS]/manual-link
}

@Neumann-A
Copy link
Contributor

Ok got qt-declaritve to compile the next trick was to change mkspecs/features/win32/windows.prf
there exists the line:
lib = $$QT.core.libs$${QMAKE_PREFIX_STATICLIB}qtmain$$QT_LIBINFIX$$qtPlatformTargetSuffix().$$QMAKE_EXTENSION_STATICLIB
which must be changed to:
lib = $$QT.core.libs/manual-link/$${QMAKE_PREFIX_STATICLIB}qtmain$$QT_LIBINFIX$$qtPlatformTargetSuffix().$$QMAKE_EXTENSION_STATICLIB

The next problem ist:
Mismatching number of debug and release binaries. Found 8 for debug but 10 for release. Apparently release builds.
G:/vcpkg_test/qt_5.12.4/packages/qt5-declarative_x86-windows/lib/Qt5QmlDevTools.lib
G:/vcpkg_test/qt_5.12.4/packages/qt5-declarative_x86-windows/lib/Qt5QmlDevToolsd.lib

so moving the latter one to debug should solve the problem.
I'll send a PR to you with th required changes for qt5-base. I'll let you solve the rest of the problems then ;)

@JackBoosY
Copy link
Contributor

After looking at the Makefile in BuildFolder\tools\bootstrap, I found that Qt5Bootstrapd.lib and Qt5Bootstrapd.prl should be generated in debug mode. Obviously the current debug build is wrong, so I think fixing the debug build is the best way to solve this problem.

@JackBoosY
Copy link
Contributor

@TheScarfix The last patch you commited is empty.

@Neumann-A
Copy link
Contributor

@JackBoosY:
From the look of the generated qt_lib_bootstrap_private.pri its seems like qt wants to only build and use the release version of Qt5Bootstrap.
The real problem is that the generated pri files for qt_lib_bootstrap do not contain the info for the dependent libraries making the patching of the makefiles in vcpkg_build_qmake necessary

@JackBoosY
Copy link
Contributor

@Neumann-A
Some makefiles are built by build qt5-base/qt5-declarative, and we can't patch them in the middle of builds.
I am not familiar with the path of setting up libraries/tools in pri/pro. Can you help me?

@JackBoosY
Copy link
Contributor

JackBoosY commented Aug 13, 2019

We can change the Qt5Bootstrap generation path to manual-link by modifying bootstrap.pro:152 :

target.path = $$[QT_INSTALL_LIBS]/manual-link
INSTALLS += target

Only changing DESTDIR does not work, this will only modify the build path without modifying the installation path.

Since the Winmain.pro file contains the installation method of Qt5Core and other core libraries, the installation path of Qt5Core cannot be changed to manual-link separately, otherwise we can add /manual-link to dlltarget.path/target.path in the configuration by modifying qt_installs.prf.

@Neumann-A Can you provide a way to generate Qt5Bootstrap as debug/release?

@Neumann-A
Copy link
Contributor

@JackBoosY I think mkspecs/features/qt_module.prf is the reason why it is always build as release:

host_build {
    QT -= gui   # no host module will ever use gui
    force_bootstrap {
        !build_pass:qtConfig(release_tools): CONFIG += release
        contains(QT, core(-private)?|xml) {
            QT -= core core-private xml
            QT += bootstrap-private
        }
    } else {
        !build_pass:qtConfig(debug_and_release): CONFIG += release
    }
}

But we have to be careful here. Should QtBootstrap be a host or target library? From my feeling it is used to build host libraries and tools. So it should not be in lib/manual-link. Maybe there is even a conflict in host/target libraries and tools and one of them is not getting installed?
I am still trying to figuring out how to setup the paths correctly so that qt builds without all the required patches. I already had a full qt 5.12.4 build but was not happy with the required patches.

@JackBoosY
Copy link
Contributor

@Neumann-A I agree with you. The manual-link folder just installs the library containing the function main. I think it should be installed into tools, but this folder cannot be created with qt5.

@Neumann-A
Copy link
Contributor

Neumann-A commented Aug 13, 2019

@JackBoosY:
It is possible. qt5-base just must be configured correctly. Already got:
-$(INSTALL_FILE) $(DESTDIR_TARGET) E:$(INSTALL_ROOT)\qt_5.12.4\packages\qt5-base_x86-windows\debug\share\qt5\debug\host\tools\qt5\$(TARGET) for Qt5Bootstrap.lib

with (configure_qt.cmake)

                -prefix ${CURRENT_INSTALLED_DIR}${_path_suffix}
                -extprefix ${CURRENT_PACKAGES_DIR}${_path_suffix}
                -hostprefix ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}/host
                -hostlibdir ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}/host/tools/qt5
                -hostbindir ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}/host/tools/qt5
                -archdatadir ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}
                -datadir ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}
                -plugindir ${CURRENT_INSTALLED_DIR}/${_path_suffix}/plugins
                -qmldir ${CURRENT_INSTALLED_DIR}/${_path_suffix}/qml
                -headerdir ${CURRENT_PACKAGES_DIR}${_path_suffix}/include
                -libexecdir ${CURRENT_PACKAGES_DIR}${_path_suffix}/tools/qt5
                -I ${CURRENT_INSTALLED_DIR}/include
                -L ${CURRENT_INSTALLED_DIR}${_path_suffix}/lib 
                -L ${CURRENT_INSTALLED_DIR}${_path_suffix}/lib/manual-link
                -xplatform ${_csc_PLATFORM}

@Neumann-A
Copy link
Contributor

argh qt's buildsystem is totally messed up.
So I set it up so that host libraries get installed into installed\x86-windows\share\qt5\host\bin but there are still some hosttools like uic getting installed into installed\x86-windows\tools\qt5

So if I then setup the paths correctly into qt.conf like qt-base configured them to be I get in qt-activeqt:
Error: dependent 'E:\qt_5.12.4\installed\x86-windows\share\qt5\debug\host\bin\uic.exe' does not exist. which is expected since qt-base decided to be installed into installed\x86-windows\tools\qt5
So long story short qt's buildsystem is a mess. The main difference between uic.pro and moc.pro in qt base is that CONFIG += force_bootstrap is behind a if in uic.pro which is probably generating the different target paths.

Moving those files into qt_5.12.4\installed\x86-windows\share\qt5\debug\host\bin solves all compilation issues.....

@JackBoosY is there a default folder for host tools? tools/qt5 is definitly wrong since this should be for target tools?

@JackBoosY
Copy link
Contributor

@Neumann-A Nope, our default tool folder is only tools, maybe we can create a new folder under the tools folder?

And how do you fix qt5bootstrap install path? just change the install path to $(INSTALL_ROOT)/share/qt5/tools?

@Neumann-A
Copy link
Contributor

Neumann-A commented Aug 13, 2019

@JackBoosY
-hostlibdir ${CURRENT_PACKAGES_DIR}${_path_suffix}/share/qt5${_path_suffix}/host/tools/qt5
installs it into a different path and then add to qt_<config>.conf in qt5-base port
HostPrefix=${CURRENT_INSTALLED_DIR}/share/qt5${_path_suffix}/host
with path _path_suffix being debug or empty

this basically removes 80% of the patches done in the configure and build scripts and links the external dependencies like ole32 and zlib correctly

@JackBoosY
Copy link
Contributor

@Neumann-A Thanks, my home network is terrible, I will try it tomorrow.

@Neumann-A
Copy link
Contributor

@JackBoosY details of what I did can be found in #7667. Dont know if it fully works yet due to qt's build time and i changed some paths but it is very close to working

@Neumann-A
Copy link
Contributor

@Rastaban: Can you answer me the question where we should put tools build for the host system by a port? the tools folder is probably for tools build for the target system. I remember a discussion (ninja port) where such tools should not be build by a port but instead be downloaded as binaries which is something I don't see happening in the qt case.

@Rastaban
Copy link
Contributor

Currently we don't differentiate between the compiled tools that are meant for the host build and compiled tools that are part of the port output. @cbezault is adding the ability to differentiate between those with system tool ports but I don't know when that will be complete. If you are able to download pre-built binaries for the tools then it could be handled by adding it to the vcpkg_find_aquire_program function.

@TheScarfix TheScarfix closed this Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants