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
pulseaudio-dlna #35245
pulseaudio-dlna #35245
Conversation
pkgs/top-level/python-packages.nix
Outdated
@@ -11319,6 +11319,38 @@ in { | |||
}; | |||
}); | |||
|
|||
notify2 = buildPythonPackage rec { |
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.
Please add this to a dedicated file in python-modules
and add a reference here with callPackages as described in the header of this file.
pkgs/top-level/python-packages.nix
Outdated
|
||
doCheck = false; # Requires real X session to run tests | ||
|
||
src = pkgs.fetchurl { |
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.
fetchPypi
is preferred here.
pkgs/top-level/python-packages.nix
Outdated
@@ -11319,6 +11319,38 @@ in { | |||
}; | |||
}); | |||
|
|||
notify2 = buildPythonPackage rec { | |||
pname = "notify2"; | |||
name = "${pname}-${version}"; |
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.
this the default for buildPythonPackage
if pname
and version
is set.
{ fetchFromGitHub, stdenv, pythonPackages, lame, opusTools, faac, flac, sox, vorbisTools }: | ||
|
||
pythonPackages.buildPythonApplication rec { | ||
name = "pulseaudio-dlna"; |
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.
pname
|
||
pythonPackages.buildPythonApplication rec { | ||
name = "pulseaudio-dlna"; | ||
version = "2017-11-01"; |
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.
Why not 0.5.2? If you want a development snapshot, you can simply override the version.
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.
I did the latest as the author has not released in a long time.
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.
As the project uses a couple python libraries this seems appropriate to not roadblock other projects, when updating python libraries.
"sha256" = "1dfn7036vrq49kxv4an7rayypnm5dlawsf02pfsldw877hzdamqk"; | ||
}; | ||
|
||
propagatedBuildInputs = with pythonPackages; [ dbus-python docopt requests setproctitle protobuf psutil futures chardet notify2 netifaces pyroute2 lxml zeroconf ] ++ [ lame opusTools faac flac sox vorbisTools ]; |
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.
Please split into multiple lines.
pkgs/top-level/python-packages.nix
Outdated
# ]; | ||
# | ||
# | ||
propagatedBuildInputs = [ self.pygobject3 self.dbus-python pkgs.gtk3 pkgs.gobjectIntrospection ]; |
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.
Are you sure about gtk3
in propagedBuildInputs
? It looks like it only uses Python libraries and gtk3 does not contain any.
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.
I couldn't get it to build without it but I am not positive it is needed. I will do more testing now.
owner = "masmu"; | ||
repo = "pulseaudio-dlna"; | ||
"rev" = "4472928dd23f274193f14289f59daec411023ab0"; | ||
"sha256" = "1dfn7036vrq49kxv4an7rayypnm5dlawsf02pfsldw877hzdamqk"; |
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.
rev
without "
; same for sha256
.
ab3cf89
to
44423c8
Compare
made the changes you requested @Mic92 |
@Mic92 rolling back the version issue we discussed, as the 0.5.2 version does not correctly connect to the pulseaudio daemon in 17.09 otherwise. I did email the author about possibly cutting a newer release though. |
44423c8
to
ddda926
Compare
sha256 = "1dfn7036vrq49kxv4an7rayypnm5dlawsf02pfsldw877hzdamqk"; | ||
}; | ||
|
||
propagatedBuildInputs = with pythonPackages; [ dbus-python docopt |
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.
maybe a newline instead of all this white space?
description = "A lightweight streaming server which brings DLNA / UPNP and Chromecast support to PulseAudio and Linux"; | ||
homepage = https://github.com/masmu/pulseaudio-dlna; | ||
|
||
license = stdenv.lib.licenses.gpl3Plus; |
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.
no stdenv.lib
faac | ||
flac | ||
sox | ||
vorbisTools ]; |
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.
Maybe create arguments like mp3Support ? true, lame
and then use ++ lib.optional mp3Support lame
, so people can disable these optional dependencies.
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.
I think pulseaudio-dlna has no tests, does it? If it doesn't, you should set doCheck = false
and add a comment.
ddda926
to
8cf5f8e
Compare
@dotlambda made all the external apps that can be called optional but defaulted to true and bad include you pointed out. fixed formatting as per @FRidh request. |
@mogorman Can please split the commit into two seperate ones for notify2 and pulseaudio-dlna? @GrahamcOfBorg build python2.pkgs.notify2 python3.pkgs.notify2 pulseaudio-dlna |
Failure on x86_64-linux (full log) Partial log (click to expand)
|
Failure on aarch64-linux (full log) Partial log (click to expand)
|
We should set |
8cf5f8e
to
2b3989f
Compare
@dotlambda 2 commits and changes made. |
@GrahamcOfBorg build python2.pkgs.notify2 python3.pkgs.notify2 pulseaudio-dlna |
1 similar comment
@GrahamcOfBorg build python2.pkgs.notify2 python3.pkgs.notify2 pulseaudio-dlna |
Success on x86_64-linux (full log) Partial log (click to expand)
|
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.
I have not tested this, but it looks very nice.
Success on aarch64-linux (full log) Partial log (click to expand)
|
Motivation for this change
Resolves #35231
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)