Update librespot to fix #1356 / #1358

[raku-cat]

As the title says, since spotify broke things with an api change as referenced in #1356 and #1358, a fix was implemented in librespot but it hasn't been versioned yet, so this pulls the commit that fixed it and I've done the minimum of what was needed to get things working considering the changes in librespot between 0.6 and now.
I'm far from a rust dev nor am I familiar with the inner workings of this project, I basically just brute forced based on compiler errors until it compiled, I can confirm it does build and launch, and the issue with the 500 errors is resolved without any host file tweaking.
I'm not expecting that this'll get merged, just getting the ball rolling.

[todorangrg]

thanks for starting this! wanted to give it a try, but it doesn't compile with MPRIS support :(

[raku-cat]

thanks for starting this! wanted to give it a try, but it doesn't compile with MPRIS support :(

Not surprised 😅 I should clarify this built for me with whatever cargo build --release --locked gives, anything beyond that I'm not sure about, I know there's a few warnings due to librespot deprecations like for the oauth backend that make it fail if you run with warnings as errors.

[todorangrg]

here is a patch file which make mpris compile and work for me (playerctl controls, metadata etc. works). Also switched oauth so the deprecation warning goes away. Now for my compile flags no warnings. I suggest you add it as a commit to your PR.
I compile with --release --features pulseaudio_backend,dbus_mpris

fixed_mpris_and_oauth_warning.PATCH

DISCLAIMER: I am a sw engineer but I don't know rust nor librespot (c++ guy). This was mostly vibe-coded by giving GPT-5 the breaking changelog since last tag + access to librespot master. So yeah, should be considered just as a starting point.

[prynhart1]

Thanks for this hotfix! I've managed to get up and running again thanks to your MR.

[eladyn]

Thank you for your effort!
The code so far looks mostly good to me; I added some small review comments.
I'm not entirely sure if we should be using an untagged dev version of librespot; I'd probably need to do some proper testing. Considering that a quick workaround (blocking apresolve.spotify.com) also works very well, I would tend towards waiting for librespot to do a release, but I'd also be open to releasing it earlier.

[eladyn]

To be in line with the previous behaviour, maybe something like

Suggested change

	env.insert("REPEAT", format!("{:?}", context));
	env.insert("REPEAT_TRACK", format!("{:?}", track));
	let val = match (context, track) {
	(_, true) => "track",
	(true, false) => "all",
	(false, false) => "none",
	}
	.to_string();
	env.insert("REPEAT", val);

could work? (not tested)

[eladyn]

todo!() will crash the application, when this line is reached, so it should generally not be in production code.
In our case, since we never activate this signal, we shouldn't receive this event at all and could either do unreachable!("this event is not enabled") (which would also crash the application, but communicates the purpose more clearly) or just add it as a second case to the Seeked branch like so:

PlayerEvent::Seeked {
            play_request_id,
            track_id,
            position_ms,
} | PlayerEvent::PositionChanged {
            play_request_id,
            track_id,
            position_ms,
}

Both would be fine to me, but the first one sounds like the cleaner solution.

[raku-cat]

Thank you for your effort! The code so far looks mostly good to me; I added some small review comments. I'm not entirely sure if we should be using an untagged dev version of librespot; I'd probably need to do some proper testing. Considering that a quick workaround (blocking apresolve.spotify.com) also works very well, I would tend towards waiting for librespot to do a release, but I'd also be open to releasing it earlier.

I tend to agree as well, I did tag it to the specific commit that fixed the issue but I think even then it'd be more appropriate to wait for a librespot release, as I said in the OP this was more to provide a patch for users in the meantime/provide a working base, this is basically my only rust development ever and much like the above commenter was done with GPT + reading compiler warnings, so I'm sure much of it is nonsensical (like the usage of todo!()) and done without proper understanding of the context of the code or how its used within the project.
My assumption was this PR would get closed when a more proper librespot release comes out and that'd be properly integrated in spotifyd.

[Exceen]

building it with the patch (cargo build --release --no-default-features --features alsa_backend,dbus_mpris) works for me. thanks!

[raku-cat]

Went ahead and converted the PR to a draft, forgot it was a thing to be honest lol, makes more sense for my intent with this PR per my last comment, though.
git apply-ed the patch in #1360 (comment) and made the changes suggested, as well as cleaned up errors reported from linting, the commit script was able to build, and doing a release build and testing my usual workflow is also working. Still would need more extensive testing, but this PR should now be a little bit more comprehensive of a patch now though.

[raku-cat]

Looks like librespot released their new version https://github.com/librespot-org/librespot/releases/tag/v0.7.0
With that I'll be closing this, looks like #1362 has already been made, looks like much better work 😅

[fabienjuif]

Ho sorry @raku-cat I've missed this PR :/
Not sure this is better tho, and if you want, feel free to fork mine

[raku-cat]

Ho sorry @raku-cat I've missed this PR :/ Not sure this is better tho, and if you want, feel free to fork mine

No worries at all! Yours looks much better made, it was always my intention to close this PR in any case.