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

libtorrentRasterbar: 1.2.6 -> 2.0.1 #107194

Merged
2 commits merged into from Dec 20, 2020
Merged

libtorrentRasterbar: 1.2.6 -> 2.0.1 #107194

2 commits merged into from Dec 20, 2020

Conversation

ghost
Copy link

@ghost ghost commented Dec 19, 2020

Motivation for this change
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.

@ghost
Copy link
Author

ghost commented Dec 19, 2020

This breaks qbittorrent build

@ghost
Copy link
Author

ghost commented Dec 19, 2020

@GrahamcOfBorg build qbittorrent

@ghost ghost changed the title libtorrentRasterbar: 1.2.6 -> 1.2.11 libtorrentRasterbar: 1.2.6 -> 2.0.1 Dec 19, 2020
@ghost
Copy link
Author

ghost commented Dec 19, 2020

Appearently 2.x is backwards-compatible to previous versions when built with the right options, while the fix release 1.2.11 broke things that compiled with 1.2.6. So I updated deluge-2_x and qbittorrent both to libtorrent 2.0.x and removed libtorrent 1.2.x.

@ghost
Copy link
Author

ghost commented Dec 19, 2020

Nope nope nope. While qbittorrent refuses to build with 1.2.11, deluge doesn't work correctly with 2.0.x.

@prusnak
Copy link
Member

prusnak commented Dec 19, 2020

We might consider treewide rename libtorrentRasterbar - > libtorrent-rasterbar to match the upstream name, but that's just an idea (and maybe for another PR).

Also we could rename 2.0/default.nix to default.nix and 1.2/default.nixto1.2.nixwhile the latter will just call the former and override theversionandsrc` attributes. (But that's only when these two files are very very similar to each other, I have not checked the other contents).

@ghost
Copy link
Author

ghost commented Dec 19, 2020

These kinds of changes can lead to huge bikeshedding discussions. I don't really care if it's changed, but I'd like to keep the discussion out of this PR.

I'd like to get one more review that explicitly confirms the new libtorrent 2.0.x works fine with qbittorrent. I have only tested deluge extensively since I am running it myself.

@prusnak
Copy link
Member

prusnak commented Dec 19, 2020

These kinds of changes can lead to huge bikeshedding discussions.

I agree, that's why I approved this PR and only added the ideas as a separate comment.

@ghost
Copy link
Author

ghost commented Dec 20, 2020

@GrahamcOfBorg eval

@SuperSandro2000
Copy link
Member

Can you solve the eval error from ofborg?

@ghost ghost requested review from FRidh and jonringer as code owners December 20, 2020 13:10
@ghost
Copy link
Author

ghost commented Dec 20, 2020

Can you solve the eval error from ofborg?

Yes, should be done with the last push.

@ghost
Copy link
Author

ghost commented Dec 20, 2020

@GrahamcOfBorg build qbittorrent deluge deluge-1_x

@SuperSandro2000
Copy link
Member

@ofborg eval

@ghost
Copy link
Author

ghost commented Dec 20, 2020

@ofborg eval

Does running eval manually make a difference? I already saw the eval results for the latest commit before.

@SuperSandro2000
Copy link
Member

Does running eval manually make a difference? I already saw the eval results for the latest commit before.

It should not make any difference.
I had the tab open before that and it did only update shortly after I commented.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 107194 run on x86_64-darwin 1

8 packages marked as broken and skipped:
  • deluge
  • deluge-2_x
  • libtorrentRasterbar
  • libtorrentRasterbar-1_2_x
  • libtorrentRasterbar-2_0_x
  • python37Packages.libtorrentRasterbar
  • python38Packages.libtorrentRasterbar
  • python39Packages.libtorrentRasterbar

@ghost
Copy link
Author

ghost commented Dec 20, 2020

Thanks! So did it build anything? I'm a bit confused because it doesn't list libtorrentRasterbar-1_1_x. Just out of curiousity: can you try running something like:

NIXPKGS_ALLOW_BROKEN=1 nix build -f https://github.com/petabyteboy/nixpkgs/archive/feature/libtorrentrasterbar-1-2-11.tar.gz deluge
NIXPKGS_ALLOW_BROKEN=1 nix build -f https://github.com/petabyteboy/nixpkgs/archive/feature/libtorrentrasterbar-1-2-11.tar.gz qbittorrent

@SuperSandro2000
Copy link
Member

NIXPKGS_ALLOW_BROKEN=1 nix build -f https://github.com/petabyteboy/nixpkgs/archive/feature/libtorrentrasterbar-1-2-11.tar.gz deluge

yeah it builds them.

@ghost
Copy link
Author

ghost commented Dec 20, 2020

I will open a third PR to attempt to unbreak them on darwin. I did some more extensive testing on qbittorrent so I'll go ahead with this.

@ghost ghost merged commit 66e4781 into NixOS:master Dec 20, 2020
ghost pushed a commit that referenced this pull request Dec 20, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants