Comment 1 Mikhail Teterin 2016-04-29 02:40:42 UTC

I think, my switch to SourceForge proposed in Bug #209117 is better -- less juggling of the filename and the distfile is a lot smaller, because it really is .tar.xz -- rather than a misnamed .tar.gz :-)

> * Fixed QT option (RADIO->SINGLE):
This I'll be happy to incorporate for you, yes.

> * Changed Qt default to Qt5
As long as KDE uses Qt4, it does not seem fair to use Qt5 by default for anything, where Qt4 is an option too.

> * Implicitly fixed 9.X build by switching to Qt5 because an offending
> compile option is only set for QT<5
But, if a 9.x user changes to QT4, their build will break, would it not?

*** This bug has been marked as a duplicate of bug 209117 ***

Comment 2 Yuri Victorovich 2016-04-29 04:30:50 UTC

Sorry, I am a maintainer.

I submitted and approved this patch.

Comment 3 Yuri Victorovich 2016-04-29 04:36:13 UTC

> But, if a 9.x user changes to QT4, their build will break, would it not?

9.X is going away in a few months.

> As long as KDE uses Qt4, it does not seem fair to use Qt5
A lot of packages switched to Qt5 regardless of the kde. Kde also switched to Qt5 in 2014. I only created this option for some extra-flexibility. In fact Qt5 version looks much nicer for whatever reason.

Comment 4 Mikhail Teterin 2016-04-29 12:58:05 UTC

> I am a maintainer.

You are, indeed. And it was for this reason that:

 a) I submitted my patch as a PR (Bug #209117) instead of simply
    committing the update myself;
 b) My PR was automatically assigned to you by Bugzilla.

Both of the above happened at 2016-04-28 02:57 UTC. 17 hours later you filed your own (duplicate) PR completely ignoring mine...

Now, the way to handle options and even deciding to abandon 9.x early may be within a maintainer's discretion.

But why do you insist on using GitHub with its the concomitant ugliness, when it is SourceForge, that is "officially" used by upstream AND offers a much smaller tarball?

Compare:
 SIZE (qbittorrent-3.3.4.tar.xz) = 2841268
 SIZE (qBittorrent-qBittorrent-release-3.3.4_GH0.tar.gz) = 5101273

Comment 5 Mikhail Teterin 2016-04-30 17:27:03 UTC

Created attachment 169834 [details]
Make compatible with libtorrent-rasterbar-1.1.0

This patch is necessary for compatibility with libtorrent-rasterbar-1.1.0 -- the latest version, which I'd like to commit together with the upgraded qbittorrent-3.3.4. See:

   https://github.com/qbittorrent/qBittorrent/issues/5138

It should simply be dropped into files/

Comment 6 Mikhail Teterin 2016-04-30 20:22:10 UTC

Comment on attachment 169834 [details]
Make compatible with libtorrent-rasterbar-1.1.0

>A work-around for the ASIO controversy. See:
>
>	https://github.com/qbittorrent/qBittorrent/issues/5138

...

Not needed. This is much better:

--- Makefile    (revision 414299)
+++ Makefile    (working copy)
@@ -55,1 +55,0 @@
-CXXFLAGS+=             -DBOOST_ASIO_DYN_LINK
 CONFIGURE_ENV+=                zlib_CFLAGS=-I/usr/include

Comment 7 Yuri Victorovich 2016-04-30 20:42:25 UTC

** Please hold this PR. **

I will update the patch once I will sort out some issues.

Comment 8 Mikhail Teterin 2016-04-30 20:42:41 UTC

Created attachment 169836 [details]
Update libtorrent-rasterbar and qbittorrent at once

This combined patch updates both ports at once. Most of it has to do with libtorrent, but they need to be done together because the removal of -DBOOST_ASIO_DYN_LINK is only valid for the new version of libtorrent-rasterbar.

The update of qbittorrent consist of the RADIO vs. SINGLE options change, upgrade to 3.3.4 and switch to SourceForge from GitHub.

I intend to commit this soon. Please, take a look. Thank you!

Comment 9 Mikhail Teterin 2016-04-30 20:46:27 UTC

(In reply to yuri from comment #7)
> some issues

You aren't suffering from SIGILL as I describe in Bug #209171, are you?

Comment 10 Yuri Victorovich 2016-04-30 20:47:27 UTC

9.X build is currently broken due to the compilers mixup there.
I need to resolve this first.

Comment 11 Yuri Victorovich 2016-04-30 20:53:21 UTC

No, I didn't see SIGILL. I have SSE4 support.

I don't know how this works. SSE support should be in the ports options, with default being off.

I bet a lot of ports suffer from this problem. Just not too many people have older CPUs. Mine is also very old, but not that old.

Comment 12 Yuri Victorovich 2016-04-30 20:56:52 UTC

BTW, I asked an upstream to make sure their tarballs don't vary in size: https://github.com/qbittorrent/qBittorrent/issues/5204

A lot of ports use GH now. I personally rather like the concept. SourceForge used to bundle malware with the distributed there software (http://www.howtogeek.com/218764/warning-don%E2%80%99t-download-software-from-sourceforge-if-you-can-help-it/), that's how they lost trust. It is also very outdated.

Comment 13 Mikhail Teterin 2016-05-01 00:40:07 UTC

(In reply to yuri from comment #12)
> BTW, I asked an upstream to make sure their tarballs don't vary in size

It is not up to them. GitHub generates those on the fly for each git-branch of the project. And only .zip and .tar.gz are available.

SourceForge hosts, what the project uploaded, so it may be .tar.xz or anything else. It is the xz vs. gzip, that explains the difference in sizes -- asking qbittorrent developers about it pointless.

There may also be a discrepancy between the RELEASE the developers host on their official download site (which for qbittorrent is on SourceForge) and what may be in the git-branch NAMED LIKE release.

> SourceForge used to bundle malware

That problem only affected precompiled binaries, not source code. As long as the upstream maintainers consider SourceForge the official download location, the port should follow.

Comment 14 Yuri Victorovich 2016-05-01 04:45:20 UTC

Github serves the valid .tar.gz of about the same size SF has. .xz is smaller, xz seems to compress better. But who cares? Switching yields no practical benefit. Github *is* an upstream for qbittorrent.

Comment 15 Mikhail Teterin 2016-05-01 08:09:04 UTC

(In reply to yuri from comment #14)
> Github *is* an upstream for qbittorrent.
This is ridiculous... Open up http://www.qbittorrent.org/ in your browser and click on "Download":
 http://www.qbittorrent.org/download.php

Scroll down to the "Source Tarballs" section of the page. The two links are:
http://sourceforge.net/projects/qbittorrent/files/qbittorrent/qbittorrent-3.3.4/qbittorrent-3.3.4.tar.gz/download
http://sourceforge.net/projects/qbittorrent/files/qbittorrent/qbittorrent-3.3.4/qbittorrent-3.3.4.tar.xz/download

If you don't have ANYTHING ELSE, I'll proceed with the commit. If you have fixes for 9.x coming, let's concentrate on that.

Comment 16 Yuri Victorovich 2016-05-01 08:29:46 UTC

> This is ridiculous...

You are ridiculous. You are initiating meaningless discussions, without any benefit whatsoever. Wasting your own time, and everybody else's time debating nonproblems.

9.X is broken, I need to resolve this. I will update the patch once done.

Comment 17 Mikhail Teterin 2016-05-04 14:46:57 UTC

(In reply to yuri from comment #16)
> 9.X is broken, I need to resolve this. I will update the patch once done.

If this breakage is not triggered by the upgrade to 3.3.4, the upgrade should not be blocked for so long, IMHO. If it is caused by the switch to QT5, let's undo that for the time being?

Then, once you figure out, how to fix 9.x, file another PR?

Comment 18 Mikhail Teterin 2016-05-05 15:26:27 UTC

Created attachment 169995 [details]
Update libtorrent-rasterbar and qbittorrent at once

This updated version of the patch fixes misuse of ioctl deep inside libtorrent-rasterbar:

 https://github.com/arvidn/libtorrent/issues/702

and removes attempts to use GeoIP, that the library stopped supporting some time ago:

 https://github.com/arvidn/libtorrent/issues/701

Comment 19 Guido Falsi 2016-06-30 08:43:05 UTC

*** Bug 209117 has been marked as a duplicate of this bug. ***

Comment 20 Kubilay Kocak 2016-09-12 15:06:54 UTC

bug 212159 attachment 174493 [details] sets qbittorent BROKEN on OSVERSION < 100000 

In lieu of a patch fixing builds on 9.x (comment 16), which is 4 months outstanding, the port should be set to BROKEN now in a first commit prior to any other changes in this issue, if it is indeed broken.

Additionally: maintainer-feedback <maintainer-email> should *always* be set when waiting or asking for responses/updates/patches from the maintainer. If not set maintainers could technically argue they were never notified.

Further, it is almost always the case that later bugs be closed duplicates of earlier ones, whether or not they contain more information. Not doing so discourages searching for existing issues before reporting and fails to respect other peoples contributions & time spent reporting issues.

While I'm here, re-triage and add See Also: references

Comment 21 Kubilay Kocak 2016-09-12 15:08:50 UTC

Walter has confirmed that attachment 169995 [details] in this issue also resolves bug 212159

@Mikhail, if you could close that bug when this issues changes land, that would be appreciated.

Comment 22 Matthew Rezny 2016-09-25 01:51:49 UTC

Created attachment 175148 [details]
update qBittorrent to 3.3.7

I have been working on an update to latest qBittorrent while working on libtorrent-rasterbar updates. This patch has been run tested for weeks in conjunction with the libtorrent-rasterbar 1.1.1 update in PR 212235. I cleaned up the port significantly while updating it.

* Drop GH bits and use official release tarball from upstream which is a tar.xz less than half the size of GH's tar.gz. Also, the USES=tar:xz is now actually correct.
* Change USES C++11-lang to C++11-lib as qBittorrent 3.3.5 and later require full C++11
* Add a patch to not need thread local storage on FBSD versions without sufficient support in libc (10-STABLE and 11.0 have it, 10.3 and prior releases do not)
* correct the QT4_USE and QT5_USE strings for the two cases. qt5widgets is moved to the interface block as that is clearly a GUI lib and concurrent gets moved with it as build tests reveal it is only needed when the GUI is built. xml is moved out of the interface block because it is needed by the nox11 build too.
* Consolidate the conditional parts where possible and correct the boolean logic.
* Add USES_GL=gl for Qt5 GUI build to satisfy QA checks.
* Move --with-qt4 to an _ON helper because there is no --with-qt5; that is the default.
* Change the port to default to Qt5 as that is what is default upstream (qBt) and supported upstream (Qt), and there is no downside to doing so given that there is no KDE integration.
* Update comment to reflect use of Qt5
* Remove the append to CXXFLAGS for Qt4 build because Q_COMPILER_INITIALIZER_LISTS is already defined and this was only causing a warning that it was being redefined differently.
* Remove USE_OPENSSL. Converting to USES=ssl produced a QA warning that USES=ssl is not needed and it does not appear to link to libssl or libcrypto.
* Remove INSTALLS_ICONS; that is only for Gtk/Gnome stuff and this is a Qt app.
* Remove the zlib CLFAGS and LIBS as those were only needed to build on 9, which no longer works due to the C++11 requirement.
* Set BROKEN just for 9, but lower so portlint doesn't complain.

QA: poudriere testport 10/11 i386/amd64 both Qt4 and Qt5 versions of main and slave, portlint only complains about missing INSTALLS_ICONS which is unnecessary according to handbook.

Comment 23 Mikhail Teterin 2016-09-30 01:22:39 UTC

(In reply to matthew from comment #22)
Matthew, your patch for qbittorrent is very similar to what I have, but it would not be suitable for upstream because outside of FreeBSD, the __FreeBSD_version will not be defined at all. I think, this will mean being defined to 0. Your first hunk may be better written as:

+#if defined(Q_OS_MAC) || defined(__FreeBSD__) && __FreeBSD_version < 1003506

The subsequent hunks depend not so much on __FreeBSD-foo, but on whether or not <QThreadStorage> was included. So, maybe

+#ifdef(QTHREADSTORAGE_H)

?

At any rate, given that the current version of libtorrent-rasterbar is still marked as vulnerable, I think, we should upgrade that to 1.1.1 together with qbittorrent.

Comment 24 Mikhail Teterin 2016-09-30 06:31:00 UTC

Created attachment 175297 [details]
Update libtorrent-rasterbar to 1.1.1 and qbittorrent to 3.3.7

Gentlemen? How about this patch... It updates both ports at once.

I also checked, whether deluge still builds -- it does.

Comment 25 Matthew Rezny 2016-10-01 22:54:39 UTC

Created attachment 175344 [details]
update net-p2p/qbittorrent to 3.3.7

Mikhail,

Have you seen the separate libtorrent-rasterbar update to 1.1.1 in PR 212235 which has already been thoroughly tested? Why must you further complicate this already messy PR by introducing an incomplete patch to a separate port? One PR per port!

Also, your qBittorent patch misses a lot of the cleanup work that I did and carefully tested, including the patch to string.cpp. Therefore, I must resubmit my update patch as the superior alternative. You should not have marked it obsolete. I have changed the string.cpp patch to be more easily upstreamable should someone want to submit it, but I kept the conditional logic the same for clarity.

Comment 26 Mikhail Teterin 2016-10-02 15:06:20 UTC

(In reply to matthew from comment #25)
> Have you seen the separate libtorrent-rasterbar update to 1.1.1 in PR #212235

No, I haven't. Not until now. I'll add my separate hunk for libtorrent-rasterbar to that other PR for your consideration.

> One PR per port!

Never heard of this rule, is it codified in some bylaws? Yes, it usually makes sense, except in cases like this one, where one port is tightly related to another. You marked PR #212235 as blocking this one yourself...

> my update patch as the superior alternative

Alright, alright. Enjoy.

Comment 27 Matthew Rezny 2016-10-02 21:49:03 UTC

(In reply to Mikhail Teterin from comment #26)

See https://wiki.freebsd.org/KubilayKocak/Bugzilla/DosAndDonts

These ports do not have to be committed together. There are other consumers of libtorrent-rasterbar, and it was actually the Deluge users contacting me that prompted the separate update of that port. The update to libtorrrent-rasterbar should be committed first since qBittorrent newer than 3.3.5 needs the library compiled in C++11 mode, hence the depends/blocks relationship.

Comment 28 Yuri Victorovich 2016-10-18 09:11:51 UTC

Created attachment 175898 [details]
Patch updating net-p2p/libtorrent-rasterbar to 1.1.1

Part of the first previous patch updating net-p2p/libtorrent-rasterbar.

Comment 29 Yuri Victorovich 2016-10-18 09:15:03 UTC

Created attachment 175899 [details]
Patch updating net-p2p/qbittorrent to 3.3.7

Modified patch updating qBittorrent to 3.3.7. Passes poudriere on 10.3 amd64.

Thank you Mikhail and Matt for your work on the patches.

Comment 30 Yuri Victorovich 2016-10-18 09:34:23 UTC

Created attachment 175900 [details]
Patch updating net-p2p/qbittorrent to 3.3.7

Comment 31 Matthew Rezny 2016-10-19 05:16:12 UTC

(In reply to yuri from comment #29)

Yuri,

Thank you for responding on this PR with a revised patch of your own. That is a pleasant surprise after having been informed there was a maintainer timeout on this PR before. However, this PR should be kept to just qBittorrent as there is already another PR for the update of libtorrent-rasterbar. The patch for libtorrent-rasterbar that is attached here differs from that in 212235 and thus is questionable (not compiled in C++11 mode so might not link with qBt 3.3.5+). Since this PR is about qBittorrent, I will not comment further except to say that we have enough to cover with qBittorrent here and any changes to libtorrent-rasterbar should be proposed in its own PR.

I compared our patches since I had tested my patch with more platform and option combinations in poudriere. Setting the HAVE_GUI variable in one place is a good improvement for readability. Unfortunately, there are a few other things that appear to be missed in your last patch. Most importantly, the handling of --with-qt4 option is unchanged, which will trigger warnings with the default port options when a non-existant --with-qt5 is passed to configure. Minor differences are a harmless but unnecessary INSTALLS_ICONS (which is only for Gtk/Gnome stuff according to Porter's Handbook) and a straggling #USE_OPENSSL. Also, I believe complier:c++11-lib is the technically correct requirement, but that only makes a functional difference on 9.x, which you haven't marked as BROKEN, but which I would be very surprised to see able to build qBt successfully.

Comment 32 Yuri Victorovich 2016-10-19 05:39:11 UTC

Comment on attachment 175898 [details]
Patch updating net-p2p/libtorrent-rasterbar to 1.1.1

Ok then,

Let this patch be committed.

Comment 33 Matthew Rezny 2016-10-19 06:02:20 UTC

Created attachment 175924 [details]
update net-p2p/qbittorrent to 3.3.7

OK, I revised my patch to incorporate your switch for the GUI since I like that improvement. Verified with a quick round of build tests (master and slave) after the change.

Comment 34 Yuri Victorovich 2016-10-19 06:15:03 UTC

Thanks!

Comment 35 Kurt Jaeger 2016-10-19 18:04:29 UTC

testbuilding the latest version, @work

Comment 36 Kurt Jaeger 2016-10-19 18:08:25 UTC

Is there a reason not to use INSTALLS_ICONS=yes even if pkg-plist
has plenty of icons and portlint -AC complains ?

Comment 37 Kurt Jaeger 2016-10-19 18:37:09 UTC

(In reply to Kurt Jaeger from comment #36)
Ok, I found the point about INSTALL_ICONS in the PR 8-}

Comment 38 commit-hook 2016-10-19 18:41:10 UTC

A commit references this bug:

Author: pi
Date: Wed Oct 19 18:40:33 UTC 2016
New revision: 424281
URL: https://svnweb.freebsd.org/changeset/ports/424281

Log:
  net-p2p/qbittorrent: update 3.3.3 -> 3.3.7

  - Lots of changes, see link to the Changes
  - Fixed QT option (RADIO->SINGLE), see details in PR#208038
  - Changed Qt default to Qt5

  PR:		209132
  Changes:	http://www.qbittorrent.org/news.php
  Submitted by:	Yuri Victorovich <yuri@rawbw.com> (maintainer), mi, matthew@reztek.cz

Changes:
  head/net-p2p/qbittorrent/Makefile
  head/net-p2p/qbittorrent/distinfo
  head/net-p2p/qbittorrent/files/patch-src_base_utils_string.cpp

Comment 39 Kurt Jaeger 2016-10-19 18:43:10 UTC

Committed, thanks to all involved for their reviews and details!