Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

deluge 2.0.3 and libtorrentRasterbar 1.2.3 #77762

Open
wants to merge 4 commits into
base: master
from

Conversation

@petabyteboy
Copy link
Contributor

petabyteboy commented Jan 15, 2020

Motivation for this change

Alternative to #64496 and #64542

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
dtzWill and others added 4 commits Jul 6, 2019
* let's try 2.0 version now, no time better than the present! Maybe!
* bz2 -> xz
* maybe python3
* disable pyGtkGlade for deps, maybe not needed?
* fix gtk/etc deps, deluge-gtk works! \o/
* restore installation of images and such

The old version is kept available as some torrent trackers have not
updated their whitelists yet.
@petabyteboy petabyteboy force-pushed the petabyteboy:feature/deluge2 branch from 93593bb to 20c6866 Jan 15, 2020
@ofborg ofborg bot requested review from ebzzry, domenkozar and Phreedom Jan 15, 2020
@dtzWill

This comment has been minimized.

Copy link
Contributor

dtzWill commented Jan 15, 2020

Thank you! Sorry for not following-up on my PR 🐈 .

On that thread there was discussion from some folks experiencing missing icons (quite possibly fixed with a rebase or changes you've made), so hopefully that's not a problem still.... but the other detail was ensuring the NixOS service module is fixed/updated accordingly. I don't know anything about that.

As a start, let's see what running the test does:

@GrahamcOfBorg test deluge

@petabyteboy

This comment has been minimized.

Copy link
Contributor Author

petabyteboy commented Jan 15, 2020

Tests are not working currently.
Icon issue is still there when deluge is run from nix-shell, I will have a look.
I hope we can make it so that users can just decide what deluge version to use by setting a package option. Not sure if it is necessary to run them in parallel.

@petabyteboy

This comment has been minimized.

Copy link
Contributor Author

petabyteboy commented Jan 15, 2020

I think the best option for the service might be to use the stateVersion to determine the default package. This is because upgrading changes the application state irreversably, so we want users who are currently using 1.x to make a conscious choice on that.
On the other hand we want new users to get the latest version by default.

@peterhoeg

This comment has been minimized.

Copy link
Member

peterhoeg commented Jan 17, 2020

The nixos module requires some serious work for deluge 2.

I have a WIP branch here https://github.com/peterhoeg/nixpkgs/commits/u/deluge that might help you.


enableParallelBuilding = true;
nativeBuildInputs = [ automake autoconf libtool pkgconfig ];
buildInputs = [ boostPython openssl zlib python libiconv geoip ncurses ];

This comment has been minimized.

Copy link
@bricewge

bricewge Jan 23, 2020

Contributor

geoip has been removed from libtorrent-rasterbar since 1.1: https://github.com/arvidn/libtorrent/blob/55460a60bc722f27d1300bddd9743d4d59492da7/include/libtorrent/session_handle.hpp#L550

Why the uses of ncurses? I can't find any reference to it in their repo?

"--with-libiconv=yes"
"--with-boost=${boostPython.dev}"
"--with-boost-libdir=${boostPython.out}/lib"
"--with-libiconv=yes"

This comment has been minimized.

Copy link
@bricewge

bricewge Jan 23, 2020

Contributor

"--with-libiconv=yes" duplicated, libtorrent-rasterbar-1.1 has the same typo.

@jtojnar jtojnar mentioned this pull request Jan 26, 2020
5 of 10 tasks complete
@svalaskevicius

This comment has been minimized.

Copy link
Contributor

svalaskevicius commented Jan 26, 2020

Do the toolbar icons appear in this pr? I had to add adwaita icon theme for them in my pr (I didn't see this one before)
edit : I'm not running gnome, just xmonad, so they're not available by default

@svalaskevicius

This comment has been minimized.

Copy link
Contributor

svalaskevicius commented Jan 26, 2020

Oh and hicolor icon theme as well for the logs to stop spamming about missing icons

@petabyteboy

This comment has been minimized.

Copy link
Contributor Author

petabyteboy commented Feb 17, 2020

Does anyone still need to run deluge 1.x? I just rechecked all my trackers and it seems like they all support deluge 2.x now, so I have no need for running the old version.

@peterhoeg

This comment has been minimized.

Copy link
Member

peterhoeg commented Feb 20, 2020

The nixos module also needs to support this before we can merge it.

@petabyteboy

This comment has been minimized.

Copy link
Contributor Author

petabyteboy commented Feb 20, 2020

That's why I'm asking, there is still significant work to be done to support both versions in parallel, which would be unnecessary if noone is using it.
But yes, the declarative configuration for deluge 2.0 something I have not taken a closer look yet, but I will do.

@peterhoeg

This comment has been minimized.

Copy link
Member

peterhoeg commented Feb 20, 2020

That's why I'm asking, there is still significant work to be done to support both versions in parallel, which would be unnecessary if noone is using it.

If I were you, I definitely wouldn't try supporting both in tandem considering how much has changed - it would be a ton of work for very little gain.

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.