Couchpotato: init at 3.0.1 + couchpotato module #21907
Conversation
|
@fadenb, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @joachifm, @offlinehacker and @FRidh to be potential reviewers. |
| @@ -1178,6 +1178,45 @@ in { | |||
| }; | |||
| }; | |||
|
|
|||
| couchpotato = buildPythonPackage rec { | |||
There was a problem hiding this comment.
couchpotato is used as an application and not as a library. Therefore, it doesn't belong in python-packages.nix. Instead, it should fit somewhere else in Nixpkgs tree based on what it is/does.
| sha256 = "1xwjis3ijh1rff32mpdsphmsclf0lkpd3phpgxkccrigq1m9r3zh"; | ||
| }; | ||
|
|
||
| phases = [ "unpackPhase" "patchPhase" "installPhase" "postInstall" ]; |
There was a problem hiding this comment.
please remove this since there is no need for it
There was a problem hiding this comment.
you might want to pass format = "other";. That gives you a buildPythonPackage for non-setuptools installations.
| substituteInPlace CouchPotato.py --replace "dirname(os.path.abspath(__file__))" "os.path.join(dirname(os.path.abspath(__file__)), '../${python.sitePackages}')" | ||
| ''; | ||
|
|
||
| installPhase = '' |
There was a problem hiding this comment.
you're making a proper package out of it. That's good. I wish upstream would do that as well.
| cp -r libs/* $out/${python.sitePackages}/ | ||
| cp -r couchpotato $out/${python.sitePackages}/ | ||
|
|
||
| cp CouchPotato.py $out/bin/ |
There was a problem hiding this comment.
having a binary with an extension is rather ugly. Maybe we want to rename it instead to couchpotato ?
| ''; | ||
|
|
||
| postInstall = '' | ||
| wrapProgram "$out/bin/CouchPotato.py" --prefix PYTHONPATH : "$PYTHONPATH:$out/${python.sitePackages}" \ |
There was a problem hiding this comment.
the wrapping is done in the fixupPhase so this shouldn't be necessary.
| description = "Automatic movie downloading via NZBs and torrents"; | ||
| license = licenses.gpl3; | ||
| maintainers = with maintainers; [ fadenb ]; | ||
| }; |
|
@FRidh: Thanks for your input. I tried to address all the issues:
|
|
Great. But then at least use |
| @@ -0,0 +1,43 @@ | |||
| { stdenv, fetchurl, pythonPackages, pkgs, lib, ... }: | |||
There was a problem hiding this comment.
stdenv isn't needed either, nor are the dots.
That's still undocumented actually. |
Motivation for this change
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)