qbittorrent: 4.1.0 -> 4.1.1 #41252

Anton-Latukha · 2018-05-30T10:17:35Z

Motivation for this change

qBittorrent 4.1.0 -> 4.1.1
SourceForge -> GitHub (development moved to GH long ago)
Updated description
Clean-up

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Anton-Latukha · 2018-05-30T20:23:13Z

Something in upstream broke bot evaluation for all new PRs.

orivej · 2018-05-30T20:30:12Z

@GrahamcOfBorg build qbittorrent

GrahamcOfBorg · 2018-05-30T20:33:26Z

Success on x86_64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1
shrinking /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1/share/man/
strip is /nix/store/92d2ifxcni4n3zx9s8wnkcjlvnx5ajlc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1
checking for references to /build in /nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/0wpx62wfs6ih1gm5bajkym6gc53r4aqx-qbittorrent-4.1.1

GrahamcOfBorg · 2018-05-30T20:38:17Z

Success on aarch64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1
shrinking /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1/share/man/
strip is /nix/store/8yfik687kfccisxnad42j19lfb7ij9b4-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1
checking for references to /build in /nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/awxq7y2krci5jzkwlhmzxcy5qnx0yy9h-qbittorrent-4.1.1

orivej · 2018-05-30T20:31:14Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

-  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg ]
-    ++ optional guiSupport dbus_libs;
+  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg
+    (if guiSupport then [ dbus_libs ] else [ ]) # D(esktop)-Bus depends on GUI support


Please don't nest arrays in buildInputs. The old buildInputs are correct (and with stdenv.lib at the top is ok; alternatively you can add lib argument and write lib.optional).

Yes, I included what previously was list concatenation into list.
Fixing.

Since optional in the source code is

{ optionals = cond: elems: if cond then elems else []; }

I prefer to use if then else for uniformal look, when there are already if then else needed in the code.

orivej · 2018-05-30T20:34:24Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

 , boost, libtorrentRasterbar, qtbase, qttools, qtsvg
 , debugSupport ? false # Debugging
 , guiSupport ? true, dbus_libs ? null # GUI (disable to run headless)
 , webuiSupport ? true # WebUI
 }:

-assert guiSupport -> (dbus_libs != null);


If you remove this assertion, you should also make dbus_libs non-optional above; and please rename them to dbus.

Yes. No way to avoid assertion here.

Reverted.

orivej · 2018-05-30T20:36:09Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

+    rev = "release-${version}";
+    owner = "qbittorrent";
+    repo = "qbittorrent";
+    sha256 = "09bf1jr2sfdps8cb154gjw7zhdcpsamhnfbgacdmkfyd7qgcbykf";


The usual and expected order is owner, repo, rev, sha256.

orivej · 2018-05-30T21:43:40Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

-  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg ]
-    ++ optional guiSupport dbus_libs;
+  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg
+    (if guiSupport then dbus else [] ) # D(esktop)-Bus depends on GUI support


This is still a nested array. The package may build like this, but such anomalies do cause issues, e.g. cross compilation support expects that all elements of buildInputs are derivations (and [] is not).

Yes. I thought "[]" evaluates to something primitive.
Worth a try 8)

orivej · 2018-05-30T21:49:02Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

    homepage    = https://www.qbittorrent.org/;
    license     = licenses.gpl2;
-    platforms   = platforms.linux;
+    platforms   = with platforms; unix;


We should not extend the list of supported platforms without testing that they are actually supported, otherwise we will likely just burden macOS Hydra builders with an always broken build. I have asked GrahamcOfBorg to build this for Mac too; if it does not report success and no one else confirms that this builds on Darwin, the platforms should be restricted back to Linux.

That is why I do it so.
I also do not have Darwin. So ask bot to build.

Probably should look to run macOS on QEMU for testing.

The bot has attempted to build qbittorrent on Darwin and it did not succeed: #41252 (comment), https://logs.nix.ci/?key=nixos/nixpkgs.41252&attempt_id=ca7f7512-a616-42ca-bb60-f50432dc9303 . Supporting Darwin requires additional work, and it is out of scope of this PR.

Anton-Latukha · 2018-05-30T22:38:41Z

Please make GrahamcOfBorg build for Darwin, if it successful - we would leave unix platform.

orivej · 2018-05-30T22:21:20Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

-  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg ]
-    ++ optional guiSupport dbus_libs;
+  buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg
+  ] ++ (if guiSupport then [ dbus ] else []); # D(esktop)-Bus depends on GUI support


This evaluates correctly.

The comment should be the other way around, "GUI support depends on dbus", but assert guiSupport -> (dbus != null); above says exactly this, so the comment is redundant.

Please understand that the old version,

buildInputs = [ boost libtorrentRasterbar qtbase qttools qtsvg ] ++ optional guiSupport dbus;

is the most usual in Nixpkgs and it is more readable once you get accustomed. You may like the new version more now, but you will probably prefer the old version in the future, and in such cases as this essentially everyone writes optional. The way you've made configureFlags consistent is fine (although going in the other direction with optionals (!guiSupport) [ "--disable-gui --enable-systemd" ] ++ optional (!webuiSupport) "--disable-webui" is more conventional; this is a good example: https://github.com/NixOS/nixpkgs/blob/18.03/pkgs/applications/networking/mailreaders/claws-mail/default.nix#L69), but the old buildInputs are perfect and can not be improved.

No. D-Bus features is additional option to the GUI build.
There are --disable-qt-dbus for Autotools and DBUS={ON,OFF} option for CMake, and that only for GUI build.

About optionals I agree.

orivej · 2018-05-30T22:52:45Z

I suppose it is in the process of building, but just in case:
@GrahamcOfBorg build qbittorrent

GrahamcOfBorg · 2018-05-30T22:55:44Z

Success on x86_64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1
shrinking /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1/share/man/
strip is /nix/store/92d2ifxcni4n3zx9s8wnkcjlvnx5ajlc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1
checking for references to /build in /nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/b2rqg320hdblzb1jc6fxmlpg7v8vinsj-qbittorrent-4.1.1

GrahamcOfBorg · 2018-05-30T23:00:22Z

Success on aarch64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1
shrinking /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1/share/man/
strip is /nix/store/8yfik687kfccisxnad42j19lfb7ij9b4-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1
checking for references to /build in /nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/64kgdm1hl0p8c87pv13r8s39dps8brwb-qbittorrent-4.1.1

GrahamcOfBorg · 2018-05-31T01:05:39Z

Failure on x86_64-darwin (full log)

Attempted: qbittorrent

Partial log (click to expand)

checking pkg-config is at least version 0.23... yes
checking whether to enable the Debug build... no
checking whether to enable the stacktrace feature... yes
checking whether to enable the GUI... yes
checking whether to install the systemd service file... no
checking whether to enable the WebUI... yes
checking for Qt5 qmake >= 5.5.1... not found
configure: error: Could not find qmake
builder for '/nix/store/2f4cfh2gsrdr3nrly5ijsjk045nj3xsg-qbittorrent-4.1.1.drv' failed with exit code 1
error: build of '/nix/store/2f4cfh2gsrdr3nrly5ijsjk045nj3xsg-qbittorrent-4.1.1.drv' failed

orivej

Thank you! This is almost ready.

orivej · 2018-05-31T01:50:47Z

pkgs/applications/networking/p2p/qbittorrent/default.nix


-  # Otherwise qm_gen.pri assumes lrelease-qt5, which does not exist.
+  # HACK: 2018-05-31: Otherwise qm_gen.pri assumes lrelease-qt5, which does not exist.


Actually this is not a hack, since this is how we are supposed to configure the path to lrelease: qbittorrent/qBittorrent@f4dc5c6

orivej · 2018-05-31T01:54:03Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

    homepage    = https://www.qbittorrent.org/;
    license     = licenses.gpl2;
-    platforms   = platforms.linux;
+    platforms   = with platforms; unix;


The bot has attempted to build qbittorrent on Darwin and it did not succeed: #41252 (comment), https://logs.nix.ci/?key=nixos/nixpkgs.41252&attempt_id=ca7f7512-a616-42ca-bb60-f50432dc9303 . Supporting Darwin requires additional work, and it is out of scope of this PR.

orivej · 2018-05-31T01:56:02Z

pkgs/applications/networking/p2p/qbittorrent/default.nix

-    (if webuiSupport then "" else "--disable-webui")
-  ] ++ optional debugSupport "--enable-debug";
+    "--with-boost=${boost.dev}" ]
+    ++ optional (!guiSupport) "--disable-gui --enable-systemd" # Also place qbittorrent-nox systemd service files


optionals here would be a bit cleaner, see https://github.com/NixOS/nixpkgs/blob/18.03/pkgs/applications/networking/mailreaders/claws-mail/default.nix#L72-L76

orivej · 2018-05-31T02:06:42Z

@Anton-Latukha Would you add yourself to the maintainers of qbittorrent?

Anton-Latukha · 2018-05-31T07:36:01Z

@orivej Thank you.
All - done.

xeji · 2018-05-31T08:11:41Z

@GrahamcOfBorg build qbittorrent

GrahamcOfBorg · 2018-05-31T08:11:53Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: qbittorrent

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-05-31T08:14:58Z

Success on x86_64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1
shrinking /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1/share/man/
strip is /nix/store/92d2ifxcni4n3zx9s8wnkcjlvnx5ajlc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1
checking for references to /build in /nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/s65mrynh9mlp0wqgxr8clq7px2si5fif-qbittorrent-4.1.1

GrahamcOfBorg · 2018-05-31T09:06:53Z

Success on aarch64-linux (full log)

Attempted: qbittorrent

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1
shrinking /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1/bin/qbittorrent
gzipping man pages under /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1/share/man/
strip is /nix/store/8yfik687kfccisxnad42j19lfb7ij9b4-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1/bin
patching script interpreter paths in /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1
checking for references to /build in /nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1...
postPatchMkspecs
/nix/store/dywlskjx3rfqr03iffcia7mgn52lc14a-qbittorrent-4.1.1

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels May 30, 2018

Anton-Latukha changed the title ~~qbittorrent: 4.1.0 -> 4.1.1~~ [WIP] qbittorrent: 4.1.0 -> 4.1.1 May 30, 2018

Anton-Latukha changed the title ~~[WIP] qbittorrent: 4.1.0 -> 4.1.1~~ qbittorrent: 4.1.0 -> 4.1.1 May 30, 2018

Anton-Latukha mentioned this pull request May 30, 2018

qbittorrent: 4.1.0 -> 4.1.1 #41266

Closed

Anton-Latukha added 2 commits May 30, 2018 23:15

qbittorrent: SourceForge -> GitHub

6ebea4f

qbittorrent: 4.1.0 -> 4.1.1

1c32512

Anton-Latukha force-pushed the qbittorrent-4.11 branch from 96dfbe8 to e814260 Compare May 30, 2018 20:16

orivej suggested changes May 30, 2018

View reviewed changes

orivej requested a review from viric May 30, 2018 21:01

Anton-Latukha force-pushed the qbittorrent-4.11 branch 2 times, most recently from 4fda877 to fc373fa Compare May 30, 2018 21:35

orivej suggested changes May 30, 2018

View reviewed changes

Anton-Latukha force-pushed the qbittorrent-4.11 branch from fc373fa to 0eeaa88 Compare May 30, 2018 22:13

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 labels May 30, 2018

orivej reviewed May 30, 2018

View reviewed changes

Anton-Latukha force-pushed the qbittorrent-4.11 branch 2 times, most recently from 3f7ccd4 to 7f5edc7 Compare May 31, 2018 00:45

orivej suggested changes May 31, 2018

View reviewed changes

Anton-Latukha added 4 commits May 31, 2018 10:14

qbittorrent: upd description

1a6d18b

qbittorrent: rm dep: which

a7166e7

qbittorrent: clean-up

3909203

qbittorrent: add Anton-Latukha maintainer

09f2d9b

Anton-Latukha force-pushed the qbittorrent-4.11 branch from 7f5edc7 to 09f2d9b Compare May 31, 2018 07:32

GrahamcOfBorg added 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 1-10 labels May 31, 2018

orivej approved these changes Jun 1, 2018

View reviewed changes

orivej merged commit c97fcb4 into NixOS:master Jun 1, 2018

Anton-Latukha deleted the qbittorrent-4.11 branch June 1, 2018 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qbittorrent: 4.1.0 -> 4.1.1 #41252

qbittorrent: 4.1.0 -> 4.1.1 #41252

Anton-Latukha commented May 30, 2018 •

edited

Loading

Anton-Latukha commented May 30, 2018

orivej commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

orivej May 30, 2018

Anton-Latukha May 30, 2018

Anton-Latukha May 30, 2018

orivej May 30, 2018

Anton-Latukha May 30, 2018

orivej May 30, 2018

Anton-Latukha May 30, 2018

orivej May 30, 2018

Anton-Latukha May 30, 2018

orivej May 30, 2018

Anton-Latukha May 30, 2018 •

edited

Loading

orivej May 31, 2018

Anton-Latukha commented May 30, 2018

orivej May 30, 2018

Anton-Latukha May 31, 2018

orivej commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 31, 2018

orivej left a comment

orivej May 31, 2018

orivej May 31, 2018

orivej May 31, 2018

orivej commented May 31, 2018

Anton-Latukha commented May 31, 2018

xeji commented May 31, 2018

GrahamcOfBorg commented May 31, 2018

GrahamcOfBorg commented May 31, 2018

GrahamcOfBorg commented May 31, 2018


		# Otherwise qm_gen.pri assumes lrelease-qt5, which does not exist.
		# HACK: 2018-05-31: Otherwise qm_gen.pri assumes lrelease-qt5, which does not exist.

qbittorrent: 4.1.0 -> 4.1.1 #41252

qbittorrent: 4.1.0 -> 4.1.1 #41252

Conversation

Anton-Latukha commented May 30, 2018 • edited Loading

Motivation for this change

Things done

Anton-Latukha commented May 30, 2018

orivej commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anton-Latukha May 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anton-Latukha commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orivej commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 30, 2018

GrahamcOfBorg commented May 31, 2018

orivej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orivej commented May 31, 2018

Anton-Latukha commented May 31, 2018

xeji commented May 31, 2018

GrahamcOfBorg commented May 31, 2018

GrahamcOfBorg commented May 31, 2018

GrahamcOfBorg commented May 31, 2018

Anton-Latukha commented May 30, 2018 •

edited

Loading

Anton-Latukha May 30, 2018 •

edited

Loading