-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
qbittorrent-enhanced: 4.6.7.10 -> 5.0.2.10; add nox variant #358112
qbittorrent-enhanced: 4.6.7.10 -> 5.0.2.10; add nox variant #358112
Conversation
7ab5ba0
to
0bf40bc
Compare
pkgs/top-level/all-packages.nix
Outdated
@@ -15462,6 +15462,8 @@ with pkgs; | |||
}; | |||
qbittorrent-nox = qbittorrent.override { guiSupport = false; }; | |||
|
|||
qbittorrent-enhanced-nox = callPackage ../by-name/qb/qbittorrent-enhanced/package.nix { guiSupport = false; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qbittorrent-enhanced-nox = callPackage ../by-name/qb/qbittorrent-enhanced/package.nix { guiSupport = false; }; | |
qbittorrent-enhanced-nox = qbittorrent-enhanced.override { guiSupport = false; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does .override
work on package.nix
? since this package is different from the above (qbittorrent-nox) which is a default.nix
#358115 (comment)
Also, can the name of the binary be overridden as well? since even tho the application is qbittorrent-enhanced, the binary is just qbittorrent, making it fail on macos.
Should I change the name from postInstall and mainProgram to just qbittorrent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.override
should also work.
But I wonder what's the different between .override
and callPackage .. { }
, is it related to package splicing?
I've seen quite a lot of callPackage ../by-name
usages:
$ grep -F 'callPackage ../by-name' pkgs/top-level/all-packages.nix | wc -l
147
But .override
with packages from by-name
can not be easily counted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of duplicating all the code with qbittorrent, I think we can also rewrite to something like:
{
lib,
fetchFromGitHub,
qbittorrent,
guiSupport ? true,
}:
(qbittorrent.override { inherit guiSupport; }).overrideAttrs (old: rec {
pname = "qbittorrent-enhanced" + lib.optionalString (!guiSupport) "-nox";
version = "5.0.2.10";
src = fetchFromGitHub {
owner = "c0re100";
repo = "qBittorrent-Enhanced-Edition";
rev = "release-${version}";
hash = "sha256-9RCG530zWQ+qzP0Y+y69NFlBWVA8GT29dY8aC1cvq7o=";
};
meta = old.meta // {
description = "Unofficial enhanced version of qBittorrent, a BitTorrent client";
homepage = "https://github.com/c0re100/qBittorrent-Enhanced-Edition";
changelog = "https://github.com/c0re100/qBittorrent-Enhanced-Edition/blob/release-${version}/Changelog";
maintainers = with lib.maintainers; [ ByteSudoer ];
};
})
Or even better, integrate into qbittorrent like the -nox
variant.
@azuwis It seems to build and work.
Then I'm guessing in all-packages.nix, it would be:
@ByteSudoer Would you mind if I did the above, deleted the |
On second thought, I think the solution here #358112 (review) is good enough. Integrating into qbittorrent may cause to much complexity, and not worth it. |
What about this?
Does this still feel too complicated? |
I think #358112 (review) is concise and easy to read. |
0bf40bc
to
612f825
Compare
|
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.