mirror of
https://github.com/qbittorrent/qBittorrent.git
synced 2026-03-02 22:57:32 -05:00
Improving application architecture (Redesign). #2058
Labels
No labels
Accessibility
AppImage
Bounty
Build system
CI
Can't reproduce
Code cleanup
Confirmed bug
Confirmed bug
Core
Crash
Data loss
Discussion
Docker
Documentation
Duplicate
Feature
Feature request
Feature request
Feature request
Filters
Flatpak
GUI
Has workaround
I2P
Invalid
Libtorrent
Look and feel
Meta
NSIS
Network
Not an issue
OS: *BSD
OS: Linux
OS: Windows
OS: macOS
PPA
Performance
Project management
Proxy/VPN
Qt bugs
Qt6 compat
RSS
Search engine
Security
Temp folder
Themes
Translations
Triggers
Waiting diagnosis
Waiting info
Waiting upstream
Waiting web implementation
Watched folders
WebAPI
WebUI
autoCloseOldIssue
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/qBittorrent#2058
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @glassez on GitHub (Jan 17, 2015).
Lately I've been increasingly involved in the development of qBittorrent. In this regard, I dive into the source code more and more. And the more I do it, the more I cultivate the desire to restore order there.
qBittorrent needs overhaul - that is my conclusion.
I will not give here the details, so as not to waste my time. Let me just say that there are a lot of problems here, starting with excessively long functions, various inconsistencies and cross-dependencies of different classes and subsystems, and to the lack of a clear architecture.
One of the important problems is the widespread dependence of our code from libtorrent code.
(Those who now consider me a fool and say "qBittorrent is based on libtorrent", I will answer immediately - no comments about this, if you do not understand what I mean - you can continue to regard me as a fool.)
To @sledgehammer999 mostly:
The circumstances are so that I can over the next few months to do this job.
I would like to discuss issues of cooperation.
This work is long and extensive code changes will be made. It is clear that I'm going to target it at a later release (after 3.2). As soon the lack of support for Qt4, I would prefer not to support Qt4. And still it is necessary to strictly set the minimum supported version of Qt5.
For the same reasons (major changes) some time after the beginning of my work (a very short time, I think) I will not be able more to rebase my code. And this is a problem because there will be concurrent commits.
Any ideas or comments?
Smaller issues.
I also have the idea to use our modification of QtSingleApplication in all qBittorrent builds. I want to take as a basis QtSingleApplication from QtCreator, which is more powerful than the one that is now represented in QtSolutions (and which we use).
Using libtorrent 0.16.x also questionable, but in any case, I initially will support both branches in order to better design the abstraction layer.
Next, I'll publish materials related to this (plans, ideas, models, and so on).
@sledgehammer999 commented on GitHub (Jan 17, 2015):
I kinda agree on most of that.
I don't understand what you mean here. Can you give an example or explain more?
In the past few toys I am toying with the idea of dropping qt4 and libtorrent 0.16.x after v3.2.0. And I think I'll definitely do it.
Why different QtSingleApplication? We maintain only a Windows-specific modification and nothing else.
What more features do we need from the one QtCreator uses?
As for how to continue work on this without stalling the rest of the development for you or having you constantly rebasing well I can think only one way. You should declare/acquire a "mutex" on files that you intend to work on for you next PR. You'll do that by opening a new issue stating what you are doing and which files you don't want touched and a rough ETA.
Of course you should try to lock as few files as possible on each iteration. Or at least permit for some rebasing on your part too.
@glassez commented on GitHub (Jan 17, 2015):
We widely use the data types of libtorrent, call its functions, etc. It makes excessive dependence on it. Another problem is the set of States of the torrent, which gives us libtorrent, does not correspond to the set that we use on the user side. Hence, multiple transformations, check the version of libtorrent and so on. For example, in another version, the developers change something, and we have to edit a lot of code to insert there #ifdef's and so on. I want to isolate all interaction with libtorrent in some abstraction layer. I understand that to get rid of all problems completely will not work even with this approach, but will get rid of some.
@sledgehammer999 commented on GitHub (Jan 17, 2015):
Yes! I wanted to change that too.
Let's say that I agree with you on libtorrent but I am not too fond of over-abstracting stuff. I think libtorrent dependence should be the last thing to "fix".
The effort should concentrate on the other stuff you mentioned first.
@glassez commented on GitHub (Jan 17, 2015):
Actually, I don't have to do it too Intrusive. I also don't like that. Much of what I mean by this abstraction will be made as a by-product of other work.
For specifics. What do I expect? I, in principle, may not remove support for Qt4. But it will increase my work, and I will be a pity if this support will be removed when I finish.
@sledgehammer999 commented on GitHub (Jan 17, 2015):
One question. Do you use KDE at all? If yes, can you tell me what is the penetration of qt5 in relation to qt4? Are most apps using qt5 now?
@glassez commented on GitHub (Jan 17, 2015):
I thought to implement this functionality in our code more deeply, if possible. I don't like, for example, the current situation with daemon.
@glassez commented on GitHub (Jan 17, 2015):
I am currently not related with Linux and KDE. I use them only for testing qBittorrent builds and even sometimes.
As for Qt, then I guess it won't be me caring at first. So you have time to think it over.
@pmzqla commented on GitHub (Jan 17, 2015):
If I'm not wrong the developement of KDE works as follows now. There are three different main components: Framework, Plasma and Applications. Currently KDE Frameworks 5 requires Qt5(.3?), Plasma 5 too if I'm not wrong, while I think most of the applications haven't drop Qt4 yet.
KDE SC 4.x is no longer developed (each 4.x release included all the components that are now released separately).
Here the schedule.
That saod, I'd expect a better Qt5 support by most of the distros in the near future.
@sledgehammer999 commented on GitHub (Jan 18, 2015):
Dropping 0.16.x is almost certain for the 3.3.x.
Dropping qt4 is still considered.
@glassez commented on GitHub (Jan 19, 2015):
OK. I will support yet.
@Gelmir commented on GitHub (Jan 19, 2015):
Hello, just wanted to ask if anything will be done about
downloadUrlAndSkipDialogandaddMagnetSkipAddDlginQBtSession. After implementing possibility of overriding global 'add paused' option for RSS feeds on per-rule basis we have quite an ugly set of default args, not counting headless ifdefs.addTorrentandaddMagnetUrimight also need a revamp.I propose using
TorrentTempDataor similar mechanics (maybe some kind of structure) for adding torrents and magnets, because with growing code complexity (and by complexity I mean different input conditionals for torrent/magnets addition) the number of arguments will grow by margin.@glassez commented on GitHub (Jan 20, 2015):
@Gelmir OK. I will bear in mind it.
@ximply commented on GitHub (Jan 21, 2015):
That's a great job!
Can some important data in ini file now turn to database, just like sqlite, because sometimes I found data lost when qBittorent crashed or exit with exceptions.
@chrishirst commented on GitHub (Jan 21, 2015):
So it's not just me that finds it a disorganised mish-mash of almost incomprehensible 'bolt-ons' then.
@ngosang commented on GitHub (Jan 31, 2015):
I would love to see some of these changes as soon as possible. I'm working on a new tab with upload and download speed charts.
Let's make qBittorrent something bigger!
@sledgehammer999 commented on GitHub (Feb 12, 2015):
@glassez you mentioned that you are refactoring QBtSession and QTorrentHandle. You also mentioned that you won't have anything to show for about a month.
However, I would be interested if you have time to write quickly what you are trying to achieve/do. Not very detailed or with code. I don't want an implementation of course. Just things like "I noticed that qtorrenthandle doesn't hold a state of the torrent and I refactoring it to always keep a cached copy of the last torrent_status".
I want an idea where you are going because:
@glassez commented on GitHub (Feb 12, 2015):
@sledgehammer999 You read my mind! 👍
Discrete BitTorrent session state model is one of the first innovations. This is in part already exists in some places. Need to do this at a basic level.
Well. I just didn't want to spend time on it. But you're right. Later I will provide a list of potential design problem (some even will have a way to correct them).
Will be very sorry if we will have conflicts on some issues. I'm always open to criticism and dialogue, if it is objectively.
@glassez commented on GitHub (Feb 12, 2015):
I may from time to time publish individual files (or fragments) until mergable/buildable PR is ready.
@sledgehammer999 commented on GitHub (Feb 12, 2015):
@sorokin since you did a lot of refactoring regarding cpu efficiency, your input here would be greatly appreciated. -after of course @glassez writes what he wants to do-
@glassez if new PRs arrive with trivial changes in QBtSession/QTorrentHandle I'll tag you to say if I can merge it.
@sledgehammer999 commented on GitHub (Feb 12, 2015):
@glassez One last thing: Try to keep it as simple as possible. No need for complex data structures or exotic design. -unless the situation can't be solved otherwise-.
Are you also trying to tackle the issue of syncronously adding torrents? If yes, I and sorokin had an offtopic discussion about this with some pretty interesting ideas. I'll tell them to you if you are working on it too.
@glassez commented on GitHub (Feb 12, 2015):
Yes. It is in my plans too. Point me to this discussion. And get some summary if possible.
@sledgehammer999 commented on GitHub (Feb 12, 2015):
I'll find it later. It was in a commit comment.
The gist of it is:
The big problem with the current loading of torrents on startup is that it slows down the qbt startup and especially the GUI. This is more prevalent when the user has many many torrents in his list.
The problem arises because we synchronously load the torrent into the libtorrent session.
This can be avoided if we use async_add_torrent() and listen for the appropriate alert.
The only issue is that I want to finally implement #583. This is already implement in part by the custom values we load/save in the .fastresume files. However, we still need that ini file to determine the order of the torrents in the queue. Otherwise we would need to first read all the .fastresume fields, parse them, sort them and then load them. This isn't very efficient.
The clever idea that @sorokin mentioned is: "We already code the hash in the filename. Why don't we code the queue number too. We'll only need to get a list of filenames and sort them.".
Of course we didn't talk about magnets. But this could be solved by having a bencoded file written in BT_backup called magnets. It would hold info about ALL the magnets that don't have metadata and of course their queue number. -even today the paused state of magnets isn't remembered-
So what were your plans on this?
@glassez commented on GitHub (Feb 12, 2015):
Not so fast. I haven't delved into this issue. I just saw async_add_torrent() function and thought, "Why don't we use it? We must consider this option." So it is in my plans, but not in priority. First I would like to finish with a redesign.
@sledgehammer999 commented on GitHub (Feb 12, 2015):
No problem.
@glassez commented on GitHub (Feb 12, 2015):
I don't agree with some of his cases. Now I'm looking at QAlertDispatcher and it seems too far-fetched (I would even say - monstrous). No, all done well, if we consider it in itself, but I think I can make it much easier.
@glassez commented on GitHub (Feb 12, 2015):
What were the reasons to refuse the use of pop_alerts() in favor of set_alert_dispatch()?
@sledgehammer999 commented on GitHub (Feb 12, 2015):
I think it showed as slow. More info in the #1668 megathread and in its PR #1703
Now I remembered the most important reason. We did away with the fixed timer. The dispatcher reacts in realtime. With pop_alerts() you react in fixed time. If you have many torrents in the queue, each time you pop_alerts() a lot of alerts will have accumulated and it will result in small gui freezes because you'll have to process them all at that time.
I am not so sure we should change this. If the dispatcher can be simplified I am game. But IMO we should continue processing in realtime.
@sledgehammer999 commented on GitHub (Feb 12, 2015):
And the discussion about async_add_torrent() at the end of this commit
github.com/qbittorrent/qBittorrent@a1a5fb065e@glassez commented on GitHub (Feb 13, 2015):
Realtime is a Chimera.
I have studied in detail the algorithm of the current code and came to the conclusion that we can get rid of QAlertDispatcher without any harm.
Description:
Now when the first alert appears (dispath() called) QAlertDispatcher adds an event to the Event Loop of the application and collects further alerts as long as the application does not handle this event (and process current alerts). And so on. While the app is idle, it processes the alerts one by one. When it is busy, the alerts have time to accumulate, and the app handles multiple.
When using an idle timer (QTimer with interval = 0) so it turns out: QTimer adds an event to App Event Loop. When application can handle this event (his turn came), we call pop_alerts() and process current alerts. The rest is the same. While the app is idle, it processes the alerts one by one. When it is busy, the alerts have time to accumulate, and the app handles multiple. But only qBittorrent not doing it now, and libtorrent itself. I.e., we simplify the code by removing unnecessary duplication of existing functionality.
The only thing here that may be like a slower work, this is the time when the application is idle and libtorrent is also idle. In this case, we still will occasionally call pop_alerts(), which will return no alerts.
But the main thing here is that the app is IDLE, and we will not reduce its performance. With a strong desire we can make adaptive adjustment of the timer, so that to set some interval is greater than zero during libtorrent session idle. But it seems to me that the situation, when libtorrent session is idle, very rare, and it is possible not to worry about it.
@glassez commented on GitHub (Feb 14, 2015):
I've tested it. Adaptive timer shows good results (without over-complicated code). I set the interval to 500 ms. after pop_alerts() returns no alert, and reset it to 0 when alerts appear.
My initial assumption that it is possible to do without interval correction, was not correct. If you do not do this, it is high CPU usage when idle libtorrent session.
@sledgehammer999 commented on GitHub (Feb 14, 2015):
I suspected such a thing when I read your post late at night yesterday.
Anyway, the decision should be based on objective data. What I intend to do when/if you do PR changing to a timer.
I'll make 3 builds. One pre-QAlertDispatcher, one QAlertDispatcher and one with your timer. Test all 3 with 1000 torrents and run them under a performance analysis tool. Then compare the data. If the timer approach seems slightly worse than QAlertDispatcher, it will still be preferred due to its simplicity.
If it shows too slow it will be dropped.
@sorokin your expertise with vtune would be greatly appreciated. If you could do benchmarks that would be really great.
Sidenote: Today I'll be mostly unavailable. Why? My new pc parts arrive and I'll be building my new rig and reinstalling Windows(drivers, programs, etc) and transferring data from the old HDDs. Plus rebuilding my toolchain.
Interesting specs for compiling: amd fx8320 (8 cores) and 8GB RAM.
@glassez commented on GitHub (Feb 14, 2015):
Ok. I'll send PR soon.
I have no way to test it with big data. So, if my assumptions regarding the performance will turn out to be wrong, you can throw it without regret. Besides, I have another idea about this. Some intermediate option.
@sledgehammer999 commented on GitHub (Feb 15, 2015):
It took me way longer than I thought to have things working. I haven't managed yet to completely transfer old settings/files/data over to the new system. The good news now are these
From tomorrow I'll slowly begin the reviewing all the PRs that have accumulated.
@glassez commented on GitHub (Feb 16, 2015):
I love it! But if you really want to do this, you must formally specify the minimum version of the compilers that qBittorrent supports. Otherwise we will get a new headache, and not improving.
@glassez commented on GitHub (Feb 17, 2015):
qBittorrent top-level design (To-Be Model):
Here is the design that I want to embody.
Its key components:
The most important thing here is that we need to get rid of cross-dependencies wherever it is not required (and such cases are very rare). Core should not in any way depend on UI components. UI components use Core, and not Vice versa. It's not supposed to know about that and how its uses, but only to provide an interface in terms of their subject area. This mainly applies to BitTorrent session, but other Core code should not have external dependencies. Ideally, the Core should compile no matter what other top-level components used in this build, while not having any
#ifndef DISABLE_GUIetc.One more thing. I want the application structure was reflected everywhere - in the directory structure of the project, and used the names (namespaces, where necessary), and the method of inclusion of headers. Opening any project file, we need to understand that it uses, what classes and types belong to one or another part of the application structure.
@Gelmir commented on GitHub (Feb 17, 2015):
@glassez, where would RSS/Search code go in this model?
@glassez commented on GitHub (Feb 17, 2015):
I haven't come to RSS... as for the search, its basic components must belong to the Core that you can use them not only from the GUI. Already have a request to access them from WebUI.
@glassez commented on GitHub (Feb 17, 2015):
@sledgehammer999 There is really a lot of work, and I can't do everything at once. So we'll have to do it by parts.
The first part is torrents data caching. Also I can implement asynchronous torrents addition, if we make a decision regarding the queue storage.
I have read your discussion. The option of encoding priority in the name of .fastresume file is simple only at first glance. It's just when loading, but can be difficult when you save, because you need to worry about deleting old files, which names contains outdated priority. Although, we can keep the current name of .fastresume file, and if the new will not match with it, delete the old file.
Other options here are either to store an ordered list of torrents in a separate file (you can use a simple text file for quick parsing), or to use a database (we could store queue and other data there, including fastresume and persistent data).
@sledgehammer999 commented on GitHub (Feb 17, 2015):
@glassez I generally agree with the picture.
about the async loading: Isn't it simpler to manage the deleting of old files this way. Fist of all, the queue number should be coded in the fastresume filename in this way
<hash>.<queue number>.fastresume. Then before saving the new fastresume, we query the file list in BT_backup, filter by .fastresume and delete all those that begin with our hash. No need to remember the old queue number.@glassez commented on GitHub (Feb 17, 2015):
Storing the queue number (and not the full file name, as I suggested earlier), we will win in performance, since get rid of unnecessary disk operations and sorts.
@glassez commented on GitHub (Feb 17, 2015):
Sorry, not sorts but filtering.
@sledgehammer999 commented on GitHub (Feb 17, 2015):
Oh you were talking only about the storing of the queue number in a txt/db
On second thought, do we really need to store it on disk?
Why not have a QHash that will store the previous file name for each hash? We look it up before saving the new fastresume, we save it, we delete the old one, we update the QHash.
I don't think this needs to survive between sessions.
@glassez commented on GitHub (Feb 17, 2015):
Blame the language barrier!
I do not propose to keep the queue in the database. I just suggested several cases:
@sledgehammer999 commented on GitHub (Feb 17, 2015):
Number 2 is the better of the 3.
@glassez commented on GitHub (Feb 17, 2015):
I already understood that you dislike the database. But now it does not matter. I'm going to implement the second option. Perhaps some other time I implement this using a database as an alternative (possibly specified in the configuration). But not now. Now there are more important things to do.
Last clarification. Now all persistent data will be stored in .fastresume file? Queue number was the last that was preventing the rejection from the storing this data in a separate file?
@sledgehammer999 commented on GitHub (Feb 17, 2015):
IIRC, yes. Unless we added some extra field in the meantime. Anyway, if there is some obstacle you will discover it during the refactoring. But I am confident that there isn't any.
@sledgehammer999 commented on GitHub (Feb 23, 2015):
@glassez I think all your next changes will be for v3.3.0. I also am considering dropping qt4 entirely after v3.2.0. Do you have know any reason why I shouldn't do this?
@pmzqla IIRC you use linux right? qt5 support is ok there, right? We don't have any problems?
@Noctem what is qt5 support in OSX?
@glassez if qt4 is dropped we can choose to use QSaveFile and be sure no data is lost. (and of course it will be used for the .inis too)
@pmzqla commented on GitHub (Feb 23, 2015):
I use Debian (sid). I've just built qBittorrent with Qt5 with no problems. This means that it will be possible to build qBittorrent with Qt5 on the next Debian stable.
EDIT: and it seems lot of stuff had been backported, so it's maybe even possible to build it with Qt5 on the current stable.
@Noctem commented on GitHub (Feb 23, 2015):
@sledgehammer999 Qt5 is working well in OS X. I use 3.2.0 with Qt5 everyday, and it looks better than Qt4. Do you plan on distributing the 3.2 release with Qt5?
@glassez commented on GitHub (Feb 24, 2015):
The only reason that may prevent migration to Qt5 is possible lack of support for it in linux distros. You need to check what version of Qt are available in the main distros.
@sledgehammer999 commented on GitHub (Feb 24, 2015):
Debian testing(which will become the next stable) has qt5 support. The same goes for Ubuntu.
I suspect Mint will not have a problem since it derives from Ubuntu/Debian. Fedora also has qt5.
And Arch is a rolling distro so it already has it.
I think those are the major distros.
@sledgehammer999 commented on GitHub (Feb 24, 2015):
As for v3.2.0, most of the issues I left in the corresponding milestone are related to the build system.
I intend to make the usage of qt4 an explicit option. The script will look for qt5 and fail if it doesn't exist and the invoker has used the "--use-qt4" option. I'll do the same for libtorrent 0.16.x. Default will be libtorrent 1.0.x.
Of course the Windows binaries will be build using qt5 and libtorrent 1.0.x.
@ngosang commented on GitHub (Feb 24, 2015):
The QT5 applications work well. The problem is that QT5 applications do not use the default theme and look very ugly.
Qt4 left (the KDE4 theme), Qt5 right.
@sledgehammer999 commented on GitHub (Feb 24, 2015):
Is that fixable? For example under GTK DEs you can use the
qt4-qtconfigprogram. Is there a similar solution for qt5 apps?@ngosang commented on GitHub (Feb 24, 2015):
For now you can not do anything. Qt4 themes are not compatible with Qt5. The Qt guys are working to make a Qt5 theme that mimics the Oxygen theme used in Qt4.
@ngosang commented on GitHub (Feb 24, 2015):
There is an ugly hack that partially works. You can run a Qt5 app with this parameter "-style=gtk+" and the app uses the Qt5 theme that it's used to run GTK apps in KDE5 desktop. Works only in the apps that doesn't enforce one specific theme. 😖 😖 😖
https://bbs.archlinux.org/viewtopic.php?id=160206
@pmzqla commented on GitHub (Feb 24, 2015):
Oxygen had been already ported to Qt5. The problem is that it depends on KF5. I don't think it will be possible to have oxygen for Qt5 without it. Funnily, qBittorrent blends in perfectly if the DE is not depending on Qt, Gnome for instance.
@ngosang
-style gtkshould tell qBittorrent to use the style used for GTK application, which can also be qtcurve. The integration is not perfect though. The default theme should be fusion, which is not the one of the second window in the screenshot you posted.This is how qBittorrent looks with fusion (yes, it's not perfect):
This is how it looks in Gnome (I don't know if it's normal the white line on top of the left column, right above "TORRENTS". I never use Gnome, this screen was taken from inside a virtual machine):
@pmzqla commented on GitHub (Feb 24, 2015):
I don't know if I should create a separate issue for this, but I think that
QBtSession::handleTorrentFinishedAlert()has to be reworked.Currently it's called when a download is completed and it does the followings:
The main problem is that the first two operations are asynchronous and the others depend on them. This causes some problems, such as #2107 #2561 and I've seen other problems reported with the extension appended.
@ngosang commented on GitHub (Feb 24, 2015):
@pmzqla Thanks for the explanation.
@andoruB commented on GitHub (Mar 1, 2015):
It would be really awesome if some performance improvements can be achieved. Currently I have around 250 torrents loaded in the client, and I have to wait ~3 minutes when I start qBitorrent and it constantly takes about 10% CPU when running (not sure if this is expected behaviour). In other words, same thing @sledgehammer999 mentioned in an earlier comment.
@glassez commented on GitHub (Mar 1, 2015):
@andoruB Since I don't have much of torrents, I need your help (and other users with a large number of torrents) to test the new behavior, when I do it.
@andoruB commented on GitHub (Mar 1, 2015):
I forgot to mention that I'm using an older version of qB, one that's made available in the Debian unstable repos (3.1.11) so this version doesn't have any of your modifications included. But I'm building qB source cloned from this git and will let you know how it works.
I would be glad to help with my limited expertise :)
@sledgehammer999 commented on GitHub (Mar 1, 2015):
@andoruB CPU during runtime should be minimal with current git master.
However, currently there isn't any optimization about the startup.
@andoruB commented on GitHub (Mar 1, 2015):
Weird, the client startup is now a lot snappier, but the CPU usage during runtime is the same.
I built it without the webui, could that contribute to anything?
@glassez commented on GitHub (Apr 6, 2015):
Yay! My branch is finally released from non-buildable state.
This does not mean that I completely finished the redesign - there are still a lot of work. But I almost finished to make the changes that affect a lot of files. Soon I will make some more edits and make preliminary tests and then publish this BIG PR.
@sledgehammer999 In this regard, I ask not to merge any significant changes during this time. It's damn hard to support it!
@sledgehammer999 commented on GitHub (Apr 6, 2015):
Good to know.
I'll try to comply. However, v3.2.0 takes precedents. I suspect only minor mods are necessary in the files under "qtlibtorrent" folder.
I'll make them as PR first and give you a heads up.
@glassez commented on GitHub (Apr 18, 2015):
You're going to leave support for libtorrent 0.16.x in v3.3?
I already finished preparing PR. But it remains to fix support for libtorrent 0.16.x. Or should I drop it?
@sledgehammer999 commented on GitHub (Apr 18, 2015):
I am in a huge dilemma here. I want to drop 0.16 for v3.3.0.
I was also considering backporting your restructuring back into v3.2.x. Why? Because there is a ton of PRs waiting to be merged. If I don't backport your changes then the PR authors should need to do double work. One PR for the v3.2.x branch and one for the master. I don't know if they would like to do that.
So, the 0.16 dropping, it depends on if I should backport your changes or not.
Any suggestions on how to proceed?
@glassez commented on GitHub (Apr 19, 2015):
Okay, then I'll just finish support for libtorrent 0.16.x. Besides, it will show how decreased dependent code, but also show how to add support for other (newer) versions.
P.S. About backporting... Not sure you will be able to achieve great success in this matter. I am afraid when I see how you (and others) will review my changes - they are huge. As I expected, I won't be able to break it into small independent commits. Although I was on the way and came across some minor problems, I had to correct them on the go, not to spend time on creating and maintaining separate commits. Sorry...
@duytrung commented on GitHub (Apr 19, 2015):
Using QT, I found that it is not necessary because I found that the user interface of QBittorrent is simple. There is no many things to display except a few buttons, some directory display content of torrent. Focusing on the practical functions such as download algorithms, swarm merging, etc.. probably the best thing :-)
@Chocobo1 commented on GitHub (Apr 22, 2015):
This is what I have in mind (in regard to the big change, not taking future milestones into consideration):
v3.2.x should only accept maintaining fixes, cherry pick important PR from v3.3.0 to backport
v3.3.x will be the new developing branch, also try to give out early alpha builds for users to test and report.
And hope future refactoring could be done more easily and in smaller steps.
@glassez commented on GitHub (Apr 25, 2015):
PR #2892 published.
@sledgehammer999 commented on GitHub (Jun 11, 2015):
@glassez now that #2892 is merged I want to mention something relevant.
We should continue using the
BT_backupfolder. Obviously you used another name because now we use a different naming scheme for the fastresumes.My proposal is to have a transitioning code that will test for the existence of files ending in .fastresume. If it finds them, it will also mark the corresponding .torrent files, otherwise they will be added as not having a fastresume by the fallback mechanism.
When the UI is up, inform the user that "You updated from an older version that saved things differently. You must migrate to the new saving system. If you click yes, you will not be able to go back and use an older version than v3.3.0." If user clicks "Abort", qbt should exit.
Of course in the meantime no torrents should be loaded, if it is detected that the user is coming from and old installation and before he chooses to migrate.
If he chooses to migrate, we read the fastresumes and rename them according to the saved queue number. Then call the default load function.
What do you say?
@glassez commented on GitHub (Jun 12, 2015):
@sledgehammer999
Undoubtedly it should be.
But I want something else to change before that.
I really don't like that we have to use two files to resume the torrent. I want to do only one .fastresume. libtorrent allows you to save metadata in it. If this works correctly, then we will be able to do it without storing separate .torrent file. This should simplify the resume logic.
Once this is done (or rejected) we can work to create a transitional mechanism.
Here I have so far only one comment (offhand):
We also need to consider options for NoGUI builds.
@Chocobo1 commented on GitHub (Jun 12, 2015):
Just asking, in the foreseeable future, is there going to be another large refactor?
@glassez commented on GitHub (Jun 12, 2015):
Redesign is not over yet. But the rest (I think) can be done gradually in small (relatively small) commits.
@ngosang commented on GitHub (Jun 12, 2015):
@glassez This bug is still there in master https://github.com/qbittorrent/qBittorrent/pull/2892#issuecomment-98361957 Can you take a look again?
@glassez commented on GitHub (Jun 12, 2015):
@ngosang Can you test #3194? This should fix it.
@sledgehammer999 commented on GitHub (Jun 13, 2015):
@glassez I don't think it is a good idea to merge metadata+resumedata into one file. We already have cases where torrent progress is lost due to various reasons (crashes, OS killing the program, etc). Separate .torrent files, since they written only once, ensure that the user at least has torrent added and it doesn't just vanish if crash happens.
@glassez commented on GitHub (Jun 13, 2015):
In General, the presence of only the torrent file doesn't give us anything.
When we find such a file, it raises some questions without having the answers, we will not be able to do with this torrent nothing useful.
Here are those questions:
What about this?
@sledgehammer999 commented on GitHub (Jun 13, 2015):
Clearly .torrent file by itself is the indication of an error. So the least we can do for the user is to inform him of that. With that in mind:
Add it in paused mode and point it to the current default path.
If added in paused, the user can at least review the state and probably re-import the file.
The general idea should be: Do as little as possible in terms of destructive(or writing) operations and wait for the user to take action.
PS: Because in such a catastrophic failure we can't do much else. This is the last line of defense.
@glassez commented on GitHub (Jun 15, 2015):
Well. In this case, consider #3224 and we can begin to implement a transitioning code.
@sledgehammer999 commented on GitHub (Jun 17, 2015):
@glassez I came here to also suggest you prioritize the transitioning code. I want to provide alpha builds in the forum, so users can help testing the new code.
(of course I'll look into #3224)
@glassez commented on GitHub (Jun 17, 2015):
In that case, I'll do it now. But first we need to finish with #3224, because they have overlapping code.
@sledgehammer999 commented on GitHub (Jun 21, 2015):
@glassez 2 regressions:
@glassez commented on GitHub (Jun 21, 2015):
@sledgehammer999 I honestly have never used the RSS feature. Please give me some instructions how I can use this so I could run some tests.
@Matth7878 commented on GitHub (Jun 21, 2015):
@glassez I'm the one who did report this regression. To reproduce :
click on added rss feeds and you should see all torrents in right pane
@sledgehammer999 commented on GitHub (Jun 21, 2015):
@glassez I think the problem is here. IIRC you had left some old code there and I reminded you to update it in the same way you did for torrent addition in the rest of the code. Apparently both of us missed the obvious mistake. IMO it should be:
@sledgehammer999 commented on GitHub (Jun 21, 2015):
PS: I am not sure about the
thispointer.@glassez commented on GitHub (Jun 21, 2015):
@sledgehammer999 I think you right here. I'll test it tomorrow.
@ngosang commented on GitHub (Jun 23, 2015):
@glassez One more bug. (I don't know if it was already in v3.2.0)
@glassez commented on GitHub (Jun 24, 2015):
@ngosang Please try #3290.
@ngosang commented on GitHub (Jun 28, 2015):
@glassez Must be a bug in the trackers list. This happened when I added a magnet but I can't replicate.
Sometimes the info displayed in the tracker list is incorrect (all trackers display 0 peers). If I change the selected torrent or close/open the lower panel the error persists. In the transfer list (in the Seeds Peers columns) the displayed value is correct. This torrent has a lot of Seeds/Peers but in the tracker list all trackers report 0 peers (even dht).
@zeule commented on GitHub (May 11, 2016):
What about droping Qt4 support?
@glassez commented on GitHub (May 11, 2016):
If we were talking only about Windows, we could do this for a long time. But as I understand it, there are some problems with Qt5 support in Linux.
@zeule commented on GitHub (May 11, 2016):
I'm not aware of them. There might be a lack of Qt5 for a NAS systems. But you hardly find a desktop distribution without Qt5.
@glassez commented on GitHub (May 11, 2016):
@evsh you should ask somebody other. I don't know much about it. I remember we discussed it and there were problems in this.
@zeule commented on GitHub (May 11, 2016):
@all:
What were the problems with Qt5 and Linux? In this thread I found only the style problem, which is resolved long time ago.
@ngosang commented on GitHub (May 11, 2016):
I use Linux + KDE5 + Qt5 without any problem.
@glassez commented on GitHub (May 12, 2016):
However, @sledgehammer999 rejected it. Maybe we should ask him?
@zywo commented on GitHub (May 12, 2016):
I think @sledgehammer999 wants support Qt4 until EOL of Ubuntu 14.04 (2019).
@zeule commented on GitHub (May 12, 2016):
There is Qt 5.2.1 in Ubuntu 14.04. @sledgehammer999 ?
@zywo commented on GitHub (May 12, 2016):
See the 2nd part of this comment https://github.com/qbittorrent/qBittorrent/issues/5091#issuecomment-209096887
@zeule commented on GitHub (May 12, 2016):
Thanks, @zywo!
@zeule commented on GitHub (May 13, 2016):
I would be happy to implement plugins for qBt (not external executables, but shared modules). Don't think it is needed to name possible advantages of the plugin system. Our core API is Qt-centric and thus, it would be convenient to use Qt plugins engine. But, unfortunately, Qt4 and Qt5 have incompatible plugin APIs, that is why I'm interested in dropping Qt4 as soon as it became possible. Or it would be needed to maintain a custom plugins manager.
@glassez commented on GitHub (May 15, 2016):
For what purpose? And what do you see a benefit from their use?
@sledgehammer999 commented on GitHub (May 15, 2016):
For qt4 pretty much what @zywo linked to.
For plugins: Why!?!? Don't tell me you want something like deluge does(and other non-torrent programs). Facilities to extend program functionality. IMO that is too much work, code changes and for little benefit and I suspect it will add a lot of code complexity. Also we would have to be careful to be API/ABI compatible for released plugins. I am not ready to make such a commitment!!!
We are an open source program. If something gets denied for merging it is probably for good reason. Besides, I rarely deny new PRs. So IMO no need for plugin system.
@zeule commented on GitHub (May 23, 2016):
I don't think that a plugin architecture (aside from binary compatibility) would actually add complexity. Plugin architecture would allow formalising interfaces inside of qBt. And I think about it not only as a way to add additional functionality but also as a way to remove something. For example, WebUI might be a plugin, Python-based search might be a plugin to name big parts. Launching external program for completed torrents might be a plugin, extended categories might be a plugin, a colour theme might also be a plugin (and maybe you accept a Plasma-based one in that way :D). All these parts could be unneeded for a user and it would be great to have a possibility to disable them not only to save resources but to simplify UI too.
@ximply commented on GitHub (May 23, 2016):
I think that a plugin architecture is more for developers but not users.
@thalieht commented on GitHub (Sep 18, 2018):
This can be closed, right?