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

Jump to bottom

slimserver: add perlPackages.NetHTTPSNB as a dependency #109299

Merged
merged 2 commits into from Jan 14, 2021
Merged

slimserver: add perlPackages.NetHTTPSNB as a dependency #109299

merged 2 commits into from Jan 14, 2021

Conversation

mfrischknecht
Copy link
Contributor

Motivation for this change

This PR fixes #108617.

slimserver v. 7.9.2 introduced the CPAN module Net::HTTPS::NB as a new dependency. Because this dependency is currently not added through the package's derivation, the installation of plugins is no longer possible because the application fails to download its plugin repository database. Instead, the server logs the following message:

Jan 06 07:28:49 nixos systemd[1]: Started Slim Server for Logitech Squeezebox Players.
Jan 06 07:28:50 nixos slimserver.pl[20347]: Async::HTTP: Unable to load IO::Socket::SSL, will try connecting to SSL servers in non-SSL mode
Jan 06 07:28:50 nixos slimserver.pl[20347]: Base class package "Net::HTTPS::NB" is empty.
Jan 06 07:28:50 nixos slimserver.pl[20347]:     (Perhaps you need to 'use' the module which defines that package first,
Jan 06 07:28:50 nixos slimserver.pl[20347]:     or make that module available in @INC (@INC contains: [...]).
Jan 06 07:28:50 nixos slimserver.pl[20347]:  at /nix/store/1yzk49wk4wwxd2k0dzz3lhvg0p3pslzc-perl5.32.0-slimserver-7.9.2/Slim/Networking/Async/Socket/HTTPS.pm line 16.
Jan 06 07:28:50 nixos slimserver.pl[20347]: BEGIN failed--compilation aborted at /nix/store/1yzk49wk4wwxd2k0dzz3lhvg0p3pslzc-perl5.32.0-slimserver-7.9.2/Slim/Networking/Async/Socket/HTTPS.pm line 16.
Jan 06 07:28:50 nixos slimserver.pl[20347]: Compilation failed in require at /nix/store/1yzk49wk4wwxd2k0dzz3lhvg0p3pslzc-perl5.32.0-slimserver-7.9.2/Slim/Networking/Async/HTTP.pm line 22.
Jan 06 07:28:53 nixos slimserver.pl[20347]: [21-01-06 07:28:51.8711] main::init (387) Starting Logitech Media Server (v7.9.2, TRUNK, UNKNOWN) perl 5.032000 - aarch64-linux-thread-multi

Note that the initial error message (Unable to load IO::Socket::SSL [...]) is an explicitly logged message that is not correct in this specific case.

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.

SuperSandro2000
SuperSandro2000 reviewed Jan 13, 2021
View reviewed changes
pkgs/top-level/perl-packages.nix Outdated
description = "Non-blocking HTTPS client";
license = stdenv.lib.licenses.artistic1; /* source states "you can redistribute it and/or modify it under the same terms as Perl itself" */
};
doCheck = false; /* wants network */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
doCheck = false; /* wants network */
# requires network
doCheck = false;

pkgs/top-level/perl-packages.nix Show resolved Hide resolved
pkgs/top-level/perl-packages.nix Outdated
meta = {
homepage = "https://github.com/olegwtf/p5-Net-HTTPS-NB/blob/master/lib/Net/HTTPS/NB.pm";
description = "Non-blocking HTTPS client";
license = stdenv.lib.licenses.artistic1; /* source states "you can redistribute it and/or modify it under the same terms as Perl itself" */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
license = stdenv.lib.licenses.artistic1; /* source states "you can redistribute it and/or modify it under the same terms as Perl itself" */
# source states "you can redistribute it and/or modify it under the same terms as Perl itself"
license = stdenv.lib.licenses.artistic1; /* */

@mfrischknecht
Copy link
Contributor Author

I tested this change by rebasing it onto nixos-unstable (git rebase --onto origin/nixos-unstable HEAD~2) and performing nixos-rebuild -I "nixpkgs=$(pwd)" switch, after which the "Plugins" tab in the server's settings menu correctly displayed downloadable plugins.

@mfrischknecht
Copy link
Contributor Author

Applied/rebased the coding style suggestions outlined above into perlPackages.NetHTTPSNB: init at 0.15

stigtsp
stigtsp requested changes Jan 13, 2021
View reviewed changes
Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI there!

Please see the suggestions:

pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/perl-packages.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 109299 run on x86_64-linux 1

3 packages built:
  • perl530Packages.NetHTTPSNB
  • perl532Packages.NetHTTPSNB
  • slimserver

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

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

2 packages built:
  • perl530Packages.NetHTTPSNB
  • perl532Packages.NetHTTPSNB

@mfrischknecht
Copy link
Contributor Author

Hi @stigtsp,

thank you very much for your corrections! I have again rebased them into perlPackages.NetHTTPSNB: init at 0.15.

@mfrischknecht
perlPackages.NetHTTPSNB: init at 0.15
b8bd0db
@mfrischknecht
slimserver: add perlPackages.NetHTTPSNB as a dependency
52f6216 
Slimserver v. 7.9.2 introduced the CPAN package `Net::HTTPS::NB`[1][2]
as a dependency (distributed directly inside the Git repository, cf. [3]).

Without this package present in the Perl include path, the new version
fails to load the repository database for squeezebox server plugins
(configured via `Settings > Plugins`) and thus does not list any plugins
that aren't already installed on the respective system.

[1]: https://metacpan.org/pod/Net::HTTPS::NB
[2]: https://github.com/olegwtf/p5-Net-HTTPS-NB
[3]: https://github.com/Logitech/slimserver/blob/7938c061194816970c05fcfdd9607bf359f532ca/CPAN/Net/HTTPS/NB.pm
stigtsp
stigtsp approved these changes Jan 14, 2021
View reviewed changes
Copy link
Member

@stigtsp stigtsp left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks for the contribution!

  • Tested perlPackages.NetHTTPSNB OK on linux x64
  • Basic test of the slimserver service OK on linux x64 via nixos-shell, plugins appear. (Would be great to have a nixos/tests/ for it).

Result of nixpkgs-review pr 109299 run on x86_64-linux 1

3 packages built:
  • perl530Packages.NetHTTPSNB
  • perl532Packages.NetHTTPSNB
  • slimserver

@stigtsp stigtsp merged commit 47639c9 into NixOS:master Jan 14, 2021
@stigtsp stigtsp mentioned this pull request Jan 14, 2021
Merged
10 tasks
@stigtsp
Copy link
Member

stigtsp commented Jan 14, 2021

@SuperSandro2000 Can you recheck perlPackages.NetHTTPSNB on darwin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@stigtsp stigtsp stigtsp approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

slimserver >= 7.9.2 fails to install plugins due to a missing dependency (Net::HTTPS::NB)
3 participants
@mfrischknecht @SuperSandro2000 @stigtsp