Skip to content
New issue

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

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

Already on GitHub? # to your account

4.2.0: Minimal versions and breaking stuff #10047

Closed
7 tasks done
sledgehammer999 opened this issue Dec 25, 2018 · 68 comments
Closed
7 tasks done

4.2.0: Minimal versions and breaking stuff #10047

sledgehammer999 opened this issue Dec 25, 2018 · 68 comments

Comments

@sledgehammer999
Copy link
Member

sledgehammer999 commented Dec 25, 2018

@qbittorrent/demigods
This thread will be used to discuss various things about 4.2.0 and list resolutions.

I think 2 agreed upon changes are going to c++14 and dropping Windows Vista and below support.

(can be edited by other team members)

List of resolutions:

  • Require at least c++14
  • Drop old migration/upgrade code
  • Drop Vista and XP support
  • Drop libtorrent 1.0 support
  • Raise requirement for libtorrent 1.1 to version 1.1.10
  • Support RC_1_2
  • Raise minimum Qt version to 5.9.0
@sledgehammer999
Copy link
Member Author

Qt 5.12 is LTS. We can stick to it in v4.2.x.

@glassez I don't think it is that simple. Ubuntu xenial (16.04) which is used in travis-ci uses Qt 5.5.1
IMO, we should raise the Qt requirement only if we want to make use of newer API.

A bit offtopic: On Windows, qbt with Qt 5.12 crashes sometimes during exit.
I also get 2 qt runtime warnings about requesting the same URL twice and about timers don't support stopping from different threads. Does anyone see any of that?

@sledgehammer999
Copy link
Member Author

I suppose I should add to the list:

  1. Drop RC_1_0 support
  2. Support RC_1_2 when it is released

right?

@glassez
Copy link
Member

glassez commented Dec 25, 2018

Ubuntu xenial (16.04) which is used in travis-ci uses Qt 5.5.1

Qt 5.5 is 3.5 years old. It's not supported now! Will Travis-CI tell us how to live?..
Even Qt 5.6 LTS is better option ("Since Qt 5.6" is very popular annotation in Qt documentation).

IMO, we should raise the Qt requirement only if we want to make use of newer API.

Personally, I feel uncomfortable without some of the improvements available in the new versions. I'm not calling to blindly jump to the latest Qt, but we should have reasonable progress here.

Drop RC_1_0 support

👍
But it's not enough. We should declare minimal supported version from 1.1.x branch.

Support RC_1_2 when it is released

👍
IMO, "when it is released" is redundant.

@sledgehammer999
Copy link
Member Author

Will Travis-CI tell us how to live?

So you wanna drop Continuous Integration?

We should declare minimal supported version from 1.1.x branch

Yes. We should require the latest release. IIRC 1.1.11

@glassez
Copy link
Member

glassez commented Dec 25, 2018

So you wanna drop Continuous Integration?

I can't insist on that...
I don't know much about TravisCI, but my quick search shows that other projects use newer Qt versions.

@X-Coder264
Copy link

I don't think it is that simple. Ubuntu xenial (16.04) which is used in travis-ci uses Qt 5.5.1

You switched to Ubuntu Bionic (18.04) for Travis-CI two weeks ago. 18.04 comes with Qt 5.9 -> https://packages.ubuntu.com/bionic/libs/qt5-default / https://launchpad.net/ubuntu/bionic/+package/qt5-default

Isn't that available then on Travis too?

@sledgehammer999
Copy link
Member Author

@X-Coder264 nope. Travis-ci recently(less than a month) switched to xenial. I doubt it will also make available bionic.

Question is, is there anything from >5.5.1 that we need to make use of?

@X-Coder264
Copy link

Oops, sorry, I must've seen something wrong :(

@sledgehammer999
Copy link
Member Author

If we actually need something from the newer Qt versions, then probably we can find a 3rd party PPA that provides new Qt5 for xenial/travis

@X-Coder264
Copy link

@sledgehammer999 Yes, that would definitely be the best solution so that the old Qt version on Travis doesn't hold the project back. Here is for example the PPA for Qt 5.11.2 for Xenial -> https://launchpad.net/~beineri/+archive/ubuntu/opt-qt-5.11.2-xenial

@Kolcha
Copy link
Contributor

Kolcha commented Dec 26, 2018

Why don't build new Qt on Ubuntu xenial yourself? It is pretty easy, and you will get very minimal Qt (which contains only your required stuff). .tar.xz with such "hand-made" Qt will be ~20 MB and ~100MB unpacked, so this is not problem to download it on each build run + it is possible to configure travis to cache it. I can help with it, I did this on my own projects few times. and I pretty good know what qBittorrent uses from Qt, so it will not take a lot of time.

@Chocobo1
Copy link
Member

Chocobo1 commented Dec 26, 2018

Even Qt 5.6 LTS is better option ("Since Qt 5.6" is very popular annotation in Qt documentation).

Yes, especially Qt 5.6 is LTS, we should at least try moving to it.

If we actually need something from the newer Qt versions, then probably we can find a 3rd party PPA that provides new Qt5 for xenial/travis

Agree.

Why don't build new Qt on Ubuntu xenial yourself?

I think we want as less burden on our shoulder as possible (the script is a bit messy already), I suppose if you create a separate PPA we can use it, or just packages in rpm format so we can fetch & install easily.

Another way of freeing ourselves from TravisCI OS limits is using docker images, but that would require rewriting the script & potentially slower build times, something I would not consider in near future unless necessary.

@Kolcha
Copy link
Contributor

Kolcha commented Dec 26, 2018

the script is a bit messy already

I mean don't build Qt each time on ci service, build it just once on local dev machine and just download it during build (like any other dependency, but manually, using well-known curl or wget, not through apt-get install). I don't think that these 2-3 lines in build script will make it so much complex and unreadable.

I suppose if you create a separate PPA

I'll try, I never did it, but I built deb packages. it will be interesting experience, but can't promise anything.

@Chocobo1
Copy link
Member

build it just once on local dev machine and just download it during build

Yes, that is what I meant about the rpm format (or deb), this is totally fine for me.

@sledgehammer999
Copy link
Member Author

Why into all this since there is already a 3rd party PPA for xenial with newer Qt?

PS: This will mean that we also drop ubuntu versions...

@Chocobo1
Copy link
Member

Why into all this since there is already a 3rd party PPA for xenial with newer Qt?

Is there one with Qt 5.6? We should have at least one linux CI build matching minimum requirements.

This will mean that we also drop ubuntu versions...

I'm having second thoughts... According to https://wiki.ubuntu.com/Releases , Ubuntu 16.04.5 LTS EOL is April 2021, damn long.

@Chocobo1
Copy link
Member

I also get 2 qt runtime warnings about requesting the same URL twice and about timers don't support stopping from different threads. Does anyone see any of that?

I did see the URL warning but not the other, can you file an issue so we can track it?

@thalieht
Copy link
Contributor

and about timers don't support stopping from different threads.

That may be related to #10056 (and many other duplicates). I remember in that PR there was some talk about threads. But that can't be related to 5.12 because there are reports for it from a while back, since WebUI disk space was introduced in #8217.

@glassez
Copy link
Member

glassez commented Dec 26, 2018

PS: This will mean that we also drop ubuntu versions...

IMO, users of distributives that provide outdated and unsupported versions of libraries should be prepared to use outdated applications sooner or later...

@glassez
Copy link
Member

glassez commented Jan 2, 2019

@sledgehammer999, what is your final decision on minimal supported Qt version?

@glassez glassez pinned this issue Jan 12, 2019
@glassez glassez unpinned this issue Jan 12, 2019
@glassez
Copy link
Member

glassez commented Feb 19, 2019

void Application::shutdownCleanup(QSessionManager &manager)
{
Q_UNUSED(manager);
// This is only needed for a special case on Windows XP.
// (but is called for every Windows version)
// If a process takes too much time to exit during OS
// shutdown, the OS presents a dialog to the user.
// That dialog tells the user that qbt is blocking the
// shutdown, it shows a progress bar and it offers
// a "Terminate Now" button for the user. However,
// after the progress bar has reached 100% another button
// is offered to the user reading "Cancel". With this the
// user can cancel the **OS** shutdown. If we don't do
// the cleanup by handling the commitDataRequest() signal
// and the user clicks "Cancel", it will result in qbt being
// killed and the shutdown proceeding instead. Apparently
// aboutToQuit() is emitted too late in the shutdown process.
cleanup();
// According to the qt docs we shouldn't call quit() inside a slot.
// aboutToQuit() is never emitted if the user hits "Cancel" in
// the above dialog.
QTimer::singleShot(0, qApp, &QCoreApplication::quit);
}

Is this stuff still needed after removing WinXP support?

@sledgehammer999
Copy link
Member Author

sledgehammer999 commented Mar 3, 2019

Is this stuff still needed after removing WinXP support?

Probably not.

To all: FYI, Qt version is proposed to be raised to 5.9.0 via PR #10338

@glassez
Copy link
Member

glassez commented Mar 21, 2019

I would like to be clear with libtorrent-1.2 support. Personally, I'm not suggesting having it as a v4.2.0 release goal, but to stretch its implementation and testing over the entire lifecycle of 4.2 branch (having libtorrent-1.1 as the main one).

@sledgehammer999
Copy link
Member Author

Did any of you guys test your regular qbittorrent profile with libtorrent 1.2.x?
I have build current master against 1.2.x in order to do an alpha release. I eventually run it on my normal profile. Almost everything seemed good (got crashes when quiting).
Then I opened my regular v4.1.7 install which uses libtorrent 1.1.x series. 17 completed torrents started rechecking. Not good. The log shows that the error is:

check resume(): missing or invalid 'file sizes' entry. Checking again...

I don't know if this is a fault of new torrent handling in qbt code or breaking changes in libtorrent.
Currently my torrents are still checking so I can't test further.

@sledgehammer999
Copy link
Member Author

sledgehammer999 commented Aug 15, 2019

I don't know if this is a fault of new torrent handling in qbt code or breaking changes in libtorrent.

One thing in common of those files: All of them were finished/compled but in the "Queued" status (aka not explicitly paused). I have set my max active uploads to zero so I don't upload anything. All finished torrents are Queued (to be seeded).
PS: I have others completed+paused which were not affected.

@xavier2k6
Copy link
Member

xavier2k6 commented Aug 15, 2019

@sledgehammer999 I used the 4.2.0 build daily since it's release/last compiled version in #9120
Only switched back to 4.1.7 upon its release to see if silent crash was fixed as per #10581 & remained on that since I read there would be a public beta or alpha releasing soon.
QT 5.13/OpenSSL 1.1.1.c runs without any crashes/issues BTW.

4 2 0 Alpha

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2019

@sledgehammer999 at least pieces field format changed in .fastresume, libtorrent 1.2 -> libtorrent 1.1 leads to re-hashing...
according to libtorrent docs, this is its change
new libtorrent can read old fastresume data, but saves in new format

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2019

QT 5.13/OpenSSL 1.1.1.c

👍 libtorrent 1.2, "hand-made" build for macOS, also no issues detected. but my seed box still running 4.1.7 with libtorrent 1.1

@xavier2k6
Copy link
Member

This should help performance wise if added & turned "ON"
arvidn/libtorrent#3913

@sledgehammer999
Copy link
Member Author

I just released an alpha1 build. It is based on libtorrent 1.1.x just to make sure that at least qbt code is stable. Then later releases will switch libtorrent 1.2.x series.
PS: Lib versions are the same as v4.1.7

@glassez
Copy link
Member

glassez commented Aug 24, 2019

Lib versions are the same as v4.1.7

👍
It's really good idea to never update all at once. We should release significant application changes with trusted library versions whenever possible. Otherwise it's really hard to catch the bugs.

@Chocobo1
Copy link
Member

Chocobo1 commented Aug 25, 2019

@sledgehammer999
I tried the alpha build and I'm seeing silent crash events in Windows event viewer. None of them have stacktrace and all of them are 0xc0000005 (Access Violation) errors. The silent crash occurs after I hit exit in qbt.
Also I noticed you built it with Qt 5.13. That version isn't very stable for me, see issue #11117 and I would say you should stick to Qt 5.12.x series (it is LTS!) for stable releases.

I also build qbt with Qt 5.13 myself however I have no such crash from it, not sure why there is a problem from your build. I'm seeing it now from my own build... no clue why.

@xavier2k6
Copy link
Member

xavier2k6 commented Aug 25, 2019

Qt 5.13.0 requires OpenSSL 1.1.1 version on Linux and Windows
see: https://wiki.qt.io/Qt_5.13.0_Known_Issues

crash in 4.2.0 alpha1 issue experienced & logged
#11123

NOTE:

@sledgehammer999 I used the 4.2.0 build daily since it's release/last compiled version in #9120
Only switched back to 4.1.7 upon its release to see if silent crash was fixed as per #10581 & remained on that since I read there would be a public beta or alpha releasing soon.
QT 5.13/OpenSSL 1.1.1.c runs without any crashes/issues BTW.

4 2 0 Alpha

@glassez
Copy link
Member

glassez commented Aug 25, 2019

you should stick to Qt 5.12.x series (it is LTS!) for stable releases.

👍

@sledgehammer999
Copy link
Member Author

Maybe on next release, I'll rebuild Qt using the 5.12.x series.

@Chocobo1 I too do get sometimes the crashes. I now have setup WinDbg as just-in-time debugger (or post-mortem debugger) to be used by Windows when it tells me that some program has crashed. Hopefully in the next crash, I'll be able to catch something relevant.

On a possibly related note: With master I always observe this console warning after I quit qbittorrent:

QObject::~QObject: Timers cannot be stopped from another thread

Do you guys know of a way to pinpoint from where this warning is coming? Or possibly how to a breakpoint on that?

@Chocobo1
Copy link
Member

Do you guys know of a way to pinpoint from where this warning is coming?

You could try setting QT_MESSAGE_PATTERN mentioned here: https://doc.qt.io/qt-5/debug.html#warning-and-debugging-messages

@sledgehammer999
Copy link
Member Author

@Chocobo1 I managed to catch a backtrace from the crash-on-exit. Here is the gist. (only the 1st thread seemed relevant)

@xavier2k6
Copy link
Member

xavier2k6 commented Aug 26, 2019

@sledgehammer999 mentioned this in above post...you may have overlooked it...#11123

@Chocobo1
Copy link
Member

@Chocobo1 I managed to catch a backtrace from the crash-on-exit. Here is the gist. (only the 1st thread seemed relevant)

Looks similar with xavier2k6 dump. Unfortunately I couldn't make anything useful from it.

@sledgehammer999
Copy link
Member Author

This crash (and the timer warning I mentioned earlier) seems to happen only in master. So maybe just maybe this is related on how we refactored the exit code in master after the drop of the Windows XP-specific code? This is just a hypothesis.

About the timer warning: QObject classes have an internal timer associated with them. And this warning seems to be coming from the destructor of a QObject. So probably we are destroying a QObject (maybe one of our own custom subclass?) from the wrong thread. Related forum answer: https://forum.qt.io/post/271255

@sledgehammer999
Copy link
Member Author

I did a small discovery: The crashes and the timer warning seem to start with Qt 5.12.0+. The warning and the crash doesn't happen if I use Qt 5.11.3.
This is on Windows. On the linux side, I only dabble with debian, which is still at Qt 5.11 and I can't confirm the warning/crashes there.
Unfortunately, a small Qt "hello world" program, doesn't exhibit this behavior, so I can't file a bug report to Qt yet.

@xavier2k6
Copy link
Member

xavier2k6 commented Sep 12, 2019

QObject::~QObject: Timers cannot be stopped from another thread

@sledgehammer999
Is issue still unresolved/present in Qt 5.13.1?

Use of SIGNALS & SLOT/Converting QTimer to pointer may help resolve issue/debug more.
https://stackoverflow.com/questions/26461887/qt-timers-cannot-be-stopped-from-another-thread
https://stackoverflow.com/questions/53288644/timers-cannot-be-stopped-from-another-thread-qt
therecipe/qt#341
https://www.oipapio.com/question-2554784

Or you have to run the GUI stuff in the main thread/try disabling the WITH_QT option in the make file.
https://discuss.haiku-os.org/t/qt5-error-qobject-timers-cannot-be-stopped-from-another-thread/7102

@xavier2k6
Copy link
Member

Latest libraries currently released/available.
Boost 1.71
OpenSSL 1.0.2t/OpenSSL 1.1.0l/OpenSSL 1.1.1d (security fixes)
Qt 5.13.1
Libtorrent 1.1.13/1.2.2

@xavier2k6
Copy link
Member

xavier2k6 commented Nov 19, 2019

With Vista OS & below being dropped, how long until windows 7 support will be dropped as it's Microsoft support ends January 14th 2020?
XP_Vista

Also what is the Minimum Windows 10 supported Build for Qbittorrent? - as some builds are now no longer supported by Microsoft.
OS 10 Builds Support

@sledgehammer999
Copy link
Member Author

@xavier2k6 as long as msvc compilers still support targeting Windows 7, as long as Qt supports Windows 7, as long as there is significant user base using Windows 7 we won't drop it. Look at Windows XP, we haven't drop it yet. The drop will be with v4.2.0 final.
Why? We don't care about MS EOL dates. The Windows API is pretty stable between versions and it doesn't put undue burden on us to support in the codebase. Hell, we rarely have extra code for specific Windows' versions.

@glassez
Copy link
Member

glassez commented Dec 6, 2019

Shouldn't minimal supported versions be documented somewhere in project files?

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

12 participants