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.5 #77762
Conversation
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 |
Tests are not working currently. |
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. |
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. |
pkgs/development/libraries/libtorrent-rasterbar/1.2/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/libraries/libtorrent-rasterbar/1.2/default.nix
Outdated
Show resolved
Hide resolved
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) |
Oh and hicolor icon theme as well for the logs to stop spamming about missing icons |
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. |
The nixos module also needs to support this before we can merge it. |
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. |
Icons are working as soon as I add |
Fixed module and test for deluge 2.x while preserving support for deluge 1.x |
Do we want to fix the icon thing? I think I saw a comment somewhere that users are expected to have one icon theme in their environment according to the manual, so this would not strictly be a bug. Also, could users still change the icon theme used by deluge by adding one to their environment if we include one? |
I don't think so, according to the GNOME wikipage this is something
everyone as to decide for themselves. I guess if we wanted that changed
we'd need to open up an issue on the matter.
…On 3/19/20 4:18 AM, Milan wrote:
Do we want to fix the icon thing? I think I saw a comment somewhere that
users are expected to have one icon theme in their environment according
to the manual, so this would not strictly be a bug.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#77762 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABAEDYBZRAB6TCPDMGEIMDRIGFJLANCNFSM4KHDNJCQ>.
|
I moved this to my overlay and am using it on all my machines. From my side this is ready to be merged. |
@petabyteboy, do you mind fixing up the merge conflicts so we can get this merged? |
Feel free to do so. I'm running this since two weeks. Other than concerns about data migration it should be good to go. The topic of data migration is solved sufficiently for me, but others might have different standards. |
@bricewge raised some issues though. Would also be great to hear from maintainers @domenkozar @ebzzry |
Oh, I will address those comments now. |
So while working on the comments I also rebased it to master and noticed that something was not quite right. It turns libtorrentRasterbar was already updated in #83414, and deluge 1.3.15 continues to build fine with the new version. This is a surprise to me, but well, let's leave it that way then. |
Rebased, squashed together the libtorrentRasterbar updates and updated it to 1.2.5. |
With the state check I think it's safe to merge despite the user having to migrate from 1.x to 2.x, but we definitely need a few lines in the release notes. |
I added a note to the release notes, I hope I found the correct section (it is called backwards incompatible but most changes described there are more like forward incompatible). |
@worldofpeace can you tell me if the release note is ok like this? I would like to merge this. |
@petabyteboy Note looks good and is in the correct section. Seems fine to me, let's go 👍 |
…2.1 -> 4.2.2" This partially reverts commit cc03fb4. The libtorrentRasterbar update broke deluge 1.x, the hash was not updated and obsolete dependencies and flags were not removed.
https://raw.githubusercontent.com/arvidn/libtorrent/libtorrent-1_2_5/ChangeLog The old release is kept available as libtorrentRasterbar-1_1_x for deluge 1.x.
* 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.
Thanks everyone ✨ |
Motivation for this change
Alternative to #64496 and #64542
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)