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

Allow to build with Qt6 #14465

Merged
merged 3 commits into from
Oct 2, 2021
Merged

Allow to build with Qt6 #14465

merged 3 commits into from
Oct 2, 2021

Conversation

glassez
Copy link
Member

@glassez glassez commented Mar 1, 2021

No description provided.

@glassez glassez added the Core label Mar 1, 2021
@glassez glassez added this to the 5.0 milestone Mar 1, 2021
@FranciscoPombal
Copy link
Member

Haven't tested this yet, but the CMake bits LGTM at first glance.

@glassez
Copy link
Member Author

glassez commented Mar 2, 2021

Haven't tested this yet

And you won't be able to do it, since it's not even going to be successfully built at this stage yet.

@xavier2k6
Copy link
Member

xavier2k6 commented Mar 2, 2021

@glassez I dunno if you have already seen this or if it will be of any help to you:

https://www.qt.io/blog/porting-from-qt-5-to-qt-6-using-clazy-checks
https://doc.qt.io/qt-6/portingguide.html

@glassez glassez mentioned this pull request Mar 8, 2021
@glassez glassez force-pushed the qt6 branch 6 times, most recently from daf4304 to 868b590 Compare March 19, 2021 08:29
@glassez
Copy link
Member Author

glassez commented Mar 19, 2021

@FranciscoPombal, can you adjust GitHub CI configuration for this PR?

@glassez glassez force-pushed the qt6 branch 3 times, most recently from b138937 to be6b94a Compare March 22, 2021 10:43
@xavier2k6
Copy link
Member

@FranciscoPombal, can you adjust GitHub CI configuration for this PR?

VCPKG still doesn't have support for Qt 6.x.x as it's still a WIP ref.: [Qt6] WIP: For people who want to try it out or help.

They will probably need some help over there...

A possible alternative could be https://github.com/martinrotter/qt-minimalistic-builds/releases for local building/testing?

Qt 6.0.3 should be out this week/latest next..

@glassez
Copy link
Member Author

glassez commented Mar 22, 2021

Well, it finally compiles successfully on Windows using official Qt 6.0.2 installation:

01

The first thing that catches your eye is the broken progress bars:

02

03

@xavier2k6
Copy link
Member

I wonder is that over the recent PR to change to ProgressBarPainter class?

@FranciscoPombal
Copy link
Member

@FranciscoPombal, can you adjust GitHub CI configuration for this PR?

Sure, I'll use the current WIP version at microsoft/vcpkg#14333. Hopefully it's usable.

@glassez
Copy link
Member Author

glassez commented Mar 22, 2021

I wonder is that over the recent PR to change to ProgressBarPainter class?

It still works fine in Qt5, doesn't it? But this is definitely due to its manual painting.

@glassez

This comment has been minimized.

@xavier2k6
Copy link
Member

It still works fine in Qt5, doesn't it?

Just checked latest GHA Master & it behaves as expected....

@jagannatharjun
Copy link
Member

maybe because someone removed this flag in the current master 816bc45#diff-1e27babe637ab6712fed4277c12010effd1f1a9187c561905709366d1187feecR56

@glassez
Copy link
Member Author

glassez commented Mar 22, 2021

maybe because someone removed this flag in the current master 816bc45#diff-1e27babe637ab6712fed4277c12010effd1f1a9187c561905709366d1187feecR56

Good catch 👍 Confirmed.
@Chocobo1

@Chocobo1

This comment has been minimized.

@glassez
Copy link
Member Author

glassez commented Mar 22, 2021

I came across a crash related to the RSS subsystem. I'll try to debug it later.
As for supporting other OSes, that's a problem for me. At least until I'm provided with a workable CI environment.

@Chocobo1
Copy link
Member

Chocobo1 commented Mar 22, 2021

@glassez
Maybe you can consider splitting some of this PR. I see some of the changes are eligible for merging by adding appropriate conditionals to Qt6. If we are lucky we might be able to support both Qt5 & Qt6 at the same time.

@xavier2k6
Copy link
Member

@glassez Apologies, I forgot that you have a separate Qt6 workflow file.

Change:
VCPKG_COMMIT: b361c2eefa3966cb7cec45275aff32e90430aaa6 to 8dddc6c899ce6fdbeab38b525a31e7f23cb2d5bb in ci_qt6_win_mac_test.yaml

@glassez glassez force-pushed the qt6 branch 2 times, most recently from b44bd35 to 95293d2 Compare July 31, 2021 11:26
@xavier2k6
Copy link
Member

xavier2k6 commented Sep 30, 2021

This is only intended as a Reference/FYI



  • Appveyor should have Image available with Qt 6.2.0 tomorrow 1st October
  • VCPKG Qt 6.2.x Port is still a WIP. Available as of VCPKG_Commit: <35bfef7> 35bfef708400cf882efb3e4df93045e0a06c9c23
    (Not yet available in default vcpkg via GHA CI OS Environment Images)

Qt 6.3 series in March 2022

@glassez glassez force-pushed the qt6 branch 5 times, most recently from 747465a to 40bdcb2 Compare October 1, 2021 13:04
@glassez glassez changed the title Add preliminary support of Qt6 Allow to build with Qt6 Oct 1, 2021
@glassez glassez marked this pull request as ready for review October 1, 2021 14:07
@glassez glassez modified the milestones: 5.0, 4.4.0 Oct 1, 2021
@glassez glassez requested a review from Chocobo1 October 1, 2021 14:07
@glassez
Copy link
Member Author

glassez commented Oct 1, 2021

Now I believe it's time to merge it into the master. I changed it by making the build with Qt6 optional (CMake-only, via QT6 option), so that we can continue to have a single codebase for both Qt5 and Qt6 branches.
There is no CI support here. I don't have time and I don't want to deal with it - someone else can do it better than me. I have saved the old developments in a separate branch, so you can use them as a basis if necessary.

@xavier2k6
Copy link
Member

xavier2k6 commented Oct 1, 2021

For CI, We may be able to use something like install-qt-action across all OS's as there's no ppa/repo with Qt6 currently or we can build from source.....

src/src.pro Outdated Show resolved Hide resolved
@glassez
Copy link
Member Author

glassez commented Oct 2, 2021

I can't figure out what the hell is wrong with the Windows CI build. I have successfully built it locally with both Qt 6.2 and Qt 5.15.2.

@xavier2k6
Copy link
Member

xavier2k6 commented Oct 2, 2021

I can't figure out what the hell is wrong with the Windows CI build.

It's because of Boost 1.77.0

Reference:
#15402

We need to probably discuss this further on what we should do going forward until Boost 1.78.0 is released in December with the included fix in asio......should we add the Qt macro temporarily etc.

@glassez
Copy link
Member Author

glassez commented Oct 2, 2021

should we add the Qt macro temporarily etc.

👎
I don't see any good reason to just mark some Boost versions as incompatible. These "keywords" (signal, slot , emit) are quite old and widely known, so that we can consider such problems as an external bug (bug of Boost in this case).

@glassez glassez merged commit a8ade3a into qbittorrent:master Oct 2, 2021
@xavier2k6
Copy link
Member

@glassez Have a fix for windows CI....

@Chocobo1 Apologies - I actually got you to remove the fix in a way from your PR #15355

vcpkgGitCommitId: 8dddc6c899ce6fdbeab38b525a31e7f23cb2d5bb # dummy value for lukka/run-vcpkg

I thought it wasn't being used at all & it wasn't really as doNotUpdateVcpkg: was set to true

If vcpkgGitCommitId is added back in to workflow with any commit e.g. commit (3768cef9c204bb168c04b3ba7cb93b10a140a91d) before Boost 1.77.0 was introduced & doNotUpdateVcpkg: is set to false this will override the pre-installed vcpkg.

This will allow windows CI to build with Boost 1.76 again.....we can turn doNotUpdateVcpkg: back to true when Boost 1.78.0 is released or if a fix / workaround is included in a newer vcpkg commit.

@glassez glassez deleted the qt6 branch October 3, 2021 12:58
@octavedo
Copy link

HTTP/2 is enabled by default, please see:
https://doc.qt.io/qt-6/network-changes-qt6.html

qBittorrent does not appear to handle HTTP/2.

I tested with the git dev branch of qt.

It will fail to download the tracker list in the "Tracker addition dialog" (please see below)
qt6 3

I propose the attached patch to solve this problem.

diff -ruN -x .DS_Store src/base/net/downloadmanager.cpp src2/base/net/downloadmanager.cpp
--- src/base/net/downloadmanager.cpp 2021-10-14 13:49:32.000000000 +0100
+++ src2/base/net/downloadmanager.cpp 2021-10-14 14:05:07.000000000 +0100
@@ -115,6 +115,7 @@
QNetworkRequest createNetworkRequest(const Net::DownloadRequest &downloadRequest)
{
QNetworkRequest request {downloadRequest.url()};

  • request.setAttribute(QNetworkRequest::Http2AllowedAttribute, false);

     if (downloadRequest.userAgent().isEmpty())
         request.setRawHeader("User-Agent", DEFAULT_USER_AGENT);
    

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants