spotify-player icon indicating copy to clipboard operation
spotify-player copied to clipboard

Comprehensive CI suite

Open LucasFA opened this issue 1 year ago • 4 comments

Well, we just had #349, an issue about breaking with rust versions up to and including 1.73. That was a case from the bump in MSVR in souvlaki: Sinono3/souvlaki#46, breaking pulseaudio. Let's test this doesn't happen.

There's quite a few features, so we can't check each combination, but we could at least check each backend and feature once and use more sparse testing with the rest of the feature space. Another with as many features as possible, too.

There's also some interaction between target features and target OS, at least around the daemon code, so there's even more combinations.

I'm going to list all possible features (if I miss any please warn)

Audio backends

  • Rodio (default)
  • rodiojack (rodio with a jackaudio feature in librespot I think)
  • jackaudio
  • alsa
  • pulseaudio
  • portaudio
  • sdl
  • gstreamer

Each requires their own dependencies to compile.

Features

  • Streaming (enabled by default, requires Audio backend)
  • Lyrics (disabled)
  • Clipboard (enabled)
  • Media control (enabled by default, A:)
  • Image (disabled)
  • Sixel (disabled, implies Image)
  • Daemon (disabled, requires B)

A: Media control is compiled in by default but only actually active on Linux by default because of what seems described as a bug or something. B: requires: 1. not(Windows) (README), 2. streaming (code), 3. not((W || macOS) && media-control) (code, C), 4. C: I don't know if what I found was intended. As written it's that way, I think. Lines 285 of spotify-player/main.rs. Also, I think some of these can be turned into compile time errors.

LucasFA avatar Jan 26 '24 21:01 LucasFA

There's #385, but I am still puzzles by this tangent: the Daemon feature requires

  • not be on Windows (according to the README)
  • not to have the media-control feature enabled, if on Windows or Mac (according to the main.rs, which leads to a sys exit of 1)

One way or the other this can be turned into a compile time error - but to confirm, compiling on Windows with the daemon feature just doesn't work, right? I don't want to write a compile_error! only to find out it does work

LucasFA avatar Mar 02 '24 18:03 LucasFA

but to confirm, compiling on Windows with the daemon feature just doesn't work, right?

I'm not sure if it will fail to compile. The readme was added based on https://github.com/aome510/spotify-player/pull/263#issuecomment-1742623193.

One way or the other this can be turned into a compile time error

Sounds good to me but maybe only for "not to have the media-control feature enabled, if on Windows or Mac". I haven't confirmed the first one.

aome510 avatar Mar 04 '24 03:03 aome510

I'm not sure if it will fail to compile. The readme was added based on https://github.com/aome510/spotify-player/pull/263#issuecomment-1742623193.

FWIW, I just cargo b --features daemon --target x86_64-pc-windows-gnu and the daemonize crate does fail to compile, starting with

error[E0433]: failed to resolve: could not find `unix` in `os`
  --> /home/lucasfa/.local/share/cargo/registry/src/index.crates.io-6f17d22bba15001f/daemonize-0.5.0/src/lib.rs:55:14
   |
55 | use std::os::unix::ffi::OsStringExt;
   |              ^^^^ could not find `unix` in `os`

Which I'd take to mean that the crate only supports unix-likes.

maybe only for "not to have the media-control feature enabled, if on Windows or Mac".

Sounds good

LucasFA avatar Mar 04 '24 12:03 LucasFA

FWIW, I just cargo b --features daemon --target x86_64-pc-windows-gnu and the daemonize crate does fail to compile, starting with

Got it. Thanks for the confirmation.

aome510 avatar Mar 04 '24 14:03 aome510