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

spotifyd: 0.2.19 -> 0.2.20 (mpris enabled) #73042

Closed
wants to merge 2 commits into from
Closed

Conversation

MtP76
Copy link

@MtP76 MtP76 commented Nov 8, 2019

Motivation for this change
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 nix-review --run "nix-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.
Notify maintainers

cc @

Co-Authored-By: Alexey Shmalko <rasen.dubi@gmail.com>
@MtP76 MtP76 requested a review from rasendubi November 9, 2019 11:35
Copy link
Contributor

@anderslundstedt anderslundstedt left a comment

Choose a reason for hiding this comment

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

I get the following error:

[ERROR] Caught panic with message: Failed to initialize DBus connection: D-Bus error: Unable to autolaunch a dbus-daemon without a $DISPLAY for X11 (org.freedesktop.DBus.Error.NotSupported)

Use withDBUS ? false instead of withDBUS ? true?

@anderslundstedt
Copy link
Contributor

anderslundstedt commented Nov 9, 2019

With this version bump my spotifyd service failed with

[ERROR] Caught panic with message: called `Result::unwrap()` on an `Err` value: Error("EOF while parsing a value", line: 1, column: 0)

The fix was to clear out the /var/spotifyd/cache directory. I guess the old cache was not compatible with the new version. Is there any way we can ensure that the cache is cleared when upgrading versions?

EDIT: Or perhaps we should ask upstream to fix this problem?

Copy link
Member

@rasendubi rasendubi left a comment

Choose a reason for hiding this comment

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

The cache issue is not Nix-specific, so I would go for upstream.

The dbus start issue seems to be caused by spotifyd not running with X11. Not sure how much of an issue that is as I don't use spotifyd.

@MtP76 what do you think?

rasendubi
rasendubi previously approved these changes Nov 10, 2019
@anderslundstedt
Copy link
Contributor

The cache issue is not Nix-specific, so I would go for upstream.

Will do. If upstream fixes this issue then I would vote for skipping the 0.2.20 version, so as to not break the spotifyd service for people.

The dbus start issue seems to be caused by spotifyd not running with X11. Not sure how much of an issue that is as I don't use spotifyd.

Definitely an issue since one use case (for example, my use case) is running spotifyd as a headless Spotify Connect client.

@anderslundstedt
Copy link
Contributor

The cache issue is not Nix-specific, so I would go for upstream.

Will do.

Spotifyd/spotifyd#433

@anderslundstedt
Copy link
Contributor

After clearing the cache I cannot reproduce the cache problem (see Spotifyd/spotifyd#433 for more details). Upstream will not fix the problem unless it happens more often. I guess we will have to accept that this version bump might break the spotifyd service? I would still need to see the DBUS without X11 problem solved before approving this pull request, though.

@rasendubi
Copy link
Member

As a quick fix, we could encode spotifyd version in the cache directory name? Instead of /var/cache/spotifyd, it would be /var/cache/spotifyd-0.2.20

@anderslundstedt
Copy link
Contributor

That would indeed solve the problem. But there are the obvious disadvantages to clearing the cache: songs need to be recached and credentials need to be added again. If the problem I ran into is rare enough then perhaps such a quickfix is not worth it? I do not know what to prefer here.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/12

@MtP76 MtP76 closed this Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants