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.

Sign up for GitHub

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

plex-mpv-shim: init at 1.7.12 #77884

Merged
merged 2 commits into from
Mar 2, 2020
Merged

plex-mpv-shim: init at 1.7.12 #77884

merged 2 commits into from
Mar 2, 2020
+54 −0

Conversation

colemickens
Copy link
Member

Tested. Working as best as it can, I think.

(Specifically on an RPi4 with Sway)

  • 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.

Sorry, something went wrong.

@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch from 7fe056d to 5dbdebd Compare February 7, 2020 06:41
@ofborg ofborg bot added 6.topic: python 11.by: package-maintainer This PR was created by the maintainer of the package it changes labels Feb 7, 2020
jonringer
jonringer reviewed Feb 23, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

the second commit message should include unstable-2020-02-01 as the version, otherwise LGTM

@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch 2 times, most recently from 6d9c4e0 to 1e9c517 Compare February 23, 2020 22:41
@colemickens
Copy link
Member Author

@jonringer thanks! Updated the commit message to the correct format, (and bumped the unstable package to the latest commit).

@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch from 1e9c517 to ef5adec Compare February 24, 2020 12:16
@colemickens colemickens changed the title plex-mpv-shim: init at 1.7.0 plex-mpv-shim: init at 1.7.12 Feb 24, 2020
@colemickens
Copy link
Member Author

Update plex-mpv-shim itself to latest too. It work quite well now! Should be good to merge.

jonringer
jonringer reviewed Feb 25, 2020
View reviewed changes
pkgs/development/python-modules/mpv-jsonipc/default.nix Outdated

buildPythonPackage rec {
pname = "mpv-jsonipc";
version = "unstable-2020-02-19";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
version = "unstable-2020-02-19";
version = "unstable-2020-02-19";
disabled = pythonOlder "3.6";

to avoid:

  Processing ./python_mpv_jsonipc-1.1.5-py2-none-any.whl
  ERROR: Package 'python-mpv-jsonipc' requires a different Python: 2.7.17 not in '>=3.6'
cannot build derivation '/nix/store/y6499z8wxmlqjxpf8dgwim4pylqa9l7w-plex-mpv-shim-1.7.12.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/akz7a2cxndlbmq4k3hkx6y7lj7318dwl-env.drv': 2 dependencies couldn't be built
[3 built (2 failed), 1 copied (0.2 MiB), 0.0 MiB DL]
error: build of '/nix/store/akz7a2cxndlbmq4k3hkx6y7lj7318dwl-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/77884
2 package failed to build:
plex-mpv-shim python27Packages.mpv-jsonipc

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated per your suggestion. Pushed. nix-review pr 77884 passes now.

Question -- I understand why nix review tested the python36 review, but why does it look like yours try to build plex-mpv-shim with python36 whereas I was using plex-mpv-shim happily (aka, it wasn't building with python36 on my machine).

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, thanks. nix-review pr 77884 passes now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I pulled it from their assertion: Python: 2.7.17 not in '>=3.6'

@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch 4 times, most recently from 873c56c to 312851d Compare February 26, 2020 23:21
jonringer
jonringer reviewed Feb 26, 2020
View reviewed changes
pkgs/development/python-modules/mpv-jsonipc/default.nix Outdated
, tqdm, websocket_client, pythonOlder }:

buildPythonPackage rec {
pname = "mpv-jsonipc";
Copy link
Contributor

Choose a reason for hiding this comment

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

this should match the pypi name https://pypi.org/project/python-mpv-jsonipc/

Suggested change
pname = "mpv-jsonipc";
pname = "python-mpv-jsonipc";

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't realize it was being published on pypi. They're not tagging releases on GitHub though. I'm going to file an issue so we can ship a tagged release instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

iwalton3/python-mpv-jsonipc#1

Copy link
Member Author

Choose a reason for hiding this comment

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

@jonringer: I've got the pname change staged here locally, do I need to also rename the file?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes please

@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch from 312851d to e2e1ad9 Compare February 27, 2020 00:50
@colemickens colemickens mentioned this pull request Feb 27, 2020
Closed
@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch from e2e1ad9 to 09cf4ed Compare February 27, 2020 01:52
@colemickens colemickens force-pushed the nixpkgs-plex-mpv-shim branch from 09cf4ed to b1dde61 Compare February 27, 2020 02:34
jonringer
jonringer approved these changes Mar 2, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commits LGTM

[4 built, 5 copied (16.7 MiB), 3.3 MiB DL]
https://github.com/NixOS/nixpkgs/pull/77884
3 package built:
plex-mpv-shim python37Packages.python-mpv-jsonipc python38Packages.python-mpv-jsonipc

@jonringer
Copy link
Contributor

@GrahamcOfBorg build plex-mpv-shim python37Packages.python-mpv-jsonipc python38Packages.python-mpv-jsonipc

@jonringer jonringer merged commit bd25053 into NixOS:master Mar 2, 2020
@jojosch jojosch mentioned this pull request Mar 6, 2020
Merged
10 tasks
This was referenced Mar 25, 2020
Closed
Closed
@matthew-piziak
Copy link
Contributor

When I install and run plex-mpv-shim from unstable, I get the following error:

2020-03-27 14:52:17,744 [    INFO] Loaded settings from json: /home/matt/.config/plex-mpv-shim/conf.json
2020-03-27 14:52:17,744 [ WARNING] Cannot load GUI. Falling back to command line interface.
Traceback (most recent call last):
  File "/nix/store/0w18z9pf1qrq9lkhy7r5f0fhscms4c58-plex-mpv-shim-1.7.12/lib/python3.7/site-packages/plex_mpv_shim/mpv_shim.py", line 39, in main
    from .gui_mgr import userInterface
  File "/nix/store/0w18z9pf1qrq9lkhy7r5f0fhscms4c58-plex-mpv-shim-1.7.12/lib/python3.7/site-packages/plex_mpv_shim/gui_mgr.py", line 1, in <module>
    from pystray import Icon, MenuItem, Menu
ModuleNotFoundError: No module named 'pystray'
2020-03-27 14:52:18,116 [    INFO] Started GDM service
2020-03-27 14:52:18,119 [    INFO] Started HTTP server

@colemickens
Copy link
Member Author

@matthew-piziak Feel free to open a new issue. That should be a non-fatal error. If I get past it, I just hit another with the GUI. I think we can consider the GUI unsupported for now (didn't know it existed), but a new issue for it would be good.

@colemickens
Copy link
Member Author

To clarify what I just wrote, you can ignore that error about pystray. If you ignore it, plex-mpv-shim still works as expected. If I patch plex-mpv-shim to see pystray, it just hits an error about tkinter and gives up on the GUI again.

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

@doronbehar doronbehar

@jonringer jonringer

@FRidh FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@colemickens @jonringer @matthew-piziak @doronbehar