From e70174d089723118cc72bd6675903cfae8cba283 Mon Sep 17 00:00:00 2001 From: eladyn Date: Sat, 28 Dec 2024 02:56:43 +0100 Subject: [PATCH 1/6] config: refactor and replace structopt by clap This greatly refactors and simplifies the config parsing. Also, it removes the possiblity to specify user and password, since this has been phased out by Spotify and will be replaced by OAuth. --- Cargo.lock | 226 +++------------ Cargo.toml | 4 +- src/config.rs | 700 +++++++++++++---------------------------------- src/main.rs | 24 +- src/main_loop.rs | 16 +- src/process.rs | 18 -- src/setup.rs | 115 ++------ 7 files changed, 276 insertions(+), 827 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b177e3f..0fc9fc82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -74,15 +74,6 @@ dependencies = [ "libc", ] -[[package]] -name = "ansi_term" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" -dependencies = [ - "winapi", -] - [[package]] name = "anstream" version = "0.6.18" @@ -161,17 +152,6 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" -[[package]] -name = "atty" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" -dependencies = [ - "hermit-abi", - "libc", - "winapi", -] - [[package]] name = "autocfg" version = "1.4.0" @@ -399,19 +379,44 @@ dependencies = [ [[package]] name = "clap" -version = "2.34.0" +version = "4.5.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0610544180c38b88101fecf2dd634b174a62eef6946f84dfc6a7127512b381c" +checksum = "3135e7ec2ef7b10c6ed8950f0f792ed96ee093fa088608f1c76e569722700c84" dependencies = [ - "ansi_term", - "atty", - "bitflags 1.3.2", - "strsim 0.8.0", - "textwrap", - "unicode-width", - "vec_map", + "clap_builder", + "clap_derive", +] + +[[package]] +name = "clap_builder" +version = "4.5.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30582fc632330df2bd26877bde0c1f4470d57c582bbc070376afcd04d8cb4838" +dependencies = [ + "anstream", + "anstyle", + "clap_lex", + "strsim", ] +[[package]] +name = "clap_derive" +version = "4.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ac6a0c7b1a9e9a5186361f67dfa1b88213572f427fb9ab038efb2bd8c582dab" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.93", +] + +[[package]] +name = "clap_lex" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f46ad14479a25103f283c0f10005961cf086d8dc42205bb44c46ac563475dca6" + [[package]] name = "cmake" version = "0.1.52" @@ -597,7 +602,7 @@ dependencies = [ "ident_case", "proc-macro2", "quote", - "strsim 0.11.1", + "strsim", "syn 2.0.93", ] @@ -646,19 +651,6 @@ dependencies = [ "dbus", ] -[[package]] -name = "dbus-secret-service" -version = "4.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b42a16374481d92aed73ae45b1f120207d8e71d24fb89f357fadbd8f946fd84b" -dependencies = [ - "dbus", - "futures-util", - "num", - "once_cell", - "rand", -] - [[package]] name = "dbus-tokio" version = "0.7.6" @@ -1115,21 +1107,9 @@ dependencies = [ [[package]] name = "heck" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d621efb26863f0e9924c6ac577e8275e5e6b77455db64ffa6c65c904e9e132c" -dependencies = [ - "unicode-segmentation", -] - -[[package]] -name = "hermit-abi" -version = "0.1.19" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "62b467343b94ba476dcb2500d242dadbb39557df889310ac77c5d99100aaac33" -dependencies = [ - "libc", -] +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] name = "hex" @@ -1695,20 +1675,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "keyring" -version = "3.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f8fe839464d4e4b37d756d7e910063696af79a7e877282cb1825e4ec5f10833" -dependencies = [ - "byteorder", - "dbus-secret-service", - "log", - "security-framework 2.11.1", - "security-framework 3.1.0", - "windows-sys 0.59.0", -] - [[package]] name = "lazy_static" version = "1.5.0" @@ -2184,20 +2150,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "num" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" -dependencies = [ - "num-bigint", - "num-complex", - "num-integer", - "num-iter", - "num-rational", - "num-traits", -] - [[package]] name = "num-bigint" version = "0.4.6" @@ -2226,15 +2178,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "num-complex" -version = "0.4.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" -dependencies = [ - "num-traits", -] - [[package]] name = "num-conv" version = "0.1.0" @@ -2283,17 +2226,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "num-rational" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" -dependencies = [ - "num-bigint", - "num-integer", - "num-traits", -] - [[package]] name = "num-traits" version = "0.2.19" @@ -2597,30 +2529,6 @@ dependencies = [ "toml_edit", ] -[[package]] -name = "proc-macro-error" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" -dependencies = [ - "proc-macro-error-attr", - "proc-macro2", - "quote", - "syn 1.0.109", - "version_check", -] - -[[package]] -name = "proc-macro-error-attr" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" -dependencies = [ - "proc-macro2", - "quote", - "version_check", -] - [[package]] name = "proc-macro2" version = "1.0.92" @@ -3306,6 +3214,7 @@ version = "0.3.5" dependencies = [ "alsa", "chrono", + "clap", "color-eyre", "daemonize", "dbus", @@ -3317,7 +3226,6 @@ dependencies = [ "futures", "gethostname", "hex", - "keyring", "libc", "librespot-audio", "librespot-connect", @@ -3330,7 +3238,6 @@ dependencies = [ "pledge", "serde", "sha-1", - "structopt", "syslog", "thiserror 2.0.9", "time", @@ -3347,42 +3254,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "strsim" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a" - [[package]] name = "strsim" version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" -[[package]] -name = "structopt" -version = "0.3.26" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c6b5c64445ba8094a6ab0c3cd2ad323e07171012d9c98b0b15651daf1787a10" -dependencies = [ - "clap", - "lazy_static", - "structopt-derive", -] - -[[package]] -name = "structopt-derive" -version = "0.4.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcb5ae327f9cc13b68763b5749770cb9e048a99bd9dfdfa58d0cf05d5f64afe0" -dependencies = [ - "heck", - "proc-macro-error", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "subtle" version = "2.6.1" @@ -3572,15 +3449,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "textwrap" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d326610f408c7a4eb6f51c37c330e496b08506c9457c9d34287ecc38809fb060" -dependencies = [ - "unicode-width", -] - [[package]] name = "thiserror" version = "1.0.69" @@ -3903,18 +3771,6 @@ version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adb9e6ca4f869e1180728b7950e35922a7fc6397f7b641499e8f3ef06e50dc83" -[[package]] -name = "unicode-segmentation" -version = "1.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" - -[[package]] -name = "unicode-width" -version = "0.1.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" - [[package]] name = "untrusted" version = "0.9.0" @@ -3973,12 +3829,6 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "830b7e5d4d90034032940e4ace0d9a9a057e7a45cd94e6c007832e39edb82f6d" -[[package]] -name = "vec_map" -version = "0.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" - [[package]] name = "vergen" version = "9.0.2" diff --git a/Cargo.toml b/Cargo.toml index 66d3e156..ffd56cee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,12 +18,10 @@ fern = { version = "0.7.0", features = ["syslog-6"] } futures = "0.3.15" gethostname = "0.5.0" hex = "0.4" -keyring = { version = "3.5", optional = true, features = ["apple-native", "windows-native", "sync-secret-service"] } libc = "0.2.82" log = "0.4.6" serde = { version = "1.0.115", features = ["derive"] } sha-1 = "0.10" -structopt = "0.3.17" tokio = {version = "1.26.0", features = ["signal", "rt-multi-thread", "process", "io-std"] } tokio-stream = "0.1.7" url = "2.2.2" @@ -39,6 +37,7 @@ color-eyre = "0.6" directories = "5.0.1" thiserror = "2.0" time = { version = "0.3.37", default-features = false, features = ["formatting"] } +clap = { version = "4.5.23", features = ["derive"] } [target."cfg(unix)".dependencies] daemonize = "0.5" @@ -55,7 +54,6 @@ env_logger = "0.11" [features] alsa_backend = ["librespot-playback/alsa-backend", "alsa"] -dbus_keyring = ["keyring"] dbus_mpris = ["dbus", "dbus-tokio", "dbus-crossroads"] pipe_backend = [] default = ["alsa_backend", "pipe_backend"] diff --git a/src/config.rs b/src/config.rs index 9c635777..b4035afe 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,147 +1,39 @@ -use crate::{ - error::{Error as CrateError, ParseError}, - process::run_program, - utils, +use crate::utils; +use clap::{ + builder::{IntoResettable, PossibleValuesParser, TypedValueParser, ValueParser}, + Args, Parser, ValueEnum, }; use color_eyre::Report; +use directories::ProjectDirs; use gethostname::gethostname; use librespot_core::{cache::Cache, config::DeviceType as LSDeviceType, config::SessionConfig}; use librespot_playback::{ + audio_backend, config::{AudioFormat as LSAudioFormat, Bitrate as LSBitrate, PlayerConfig}, dither::{mk_ditherer, DithererBuilder, TriangularDitherer}, }; use log::{error, info, warn}; use serde::{de::Error, de::Unexpected, Deserialize, Deserializer}; use sha1::{Digest, Sha1}; -use std::{fmt, fs, path::Path, path::PathBuf, str::FromStr}; -use structopt::{clap::AppSettings, StructOpt}; +use std::{fs, path::Path, path::PathBuf}; use url::Url; const CONFIG_FILE_NAME: &str = "spotifyd.conf"; -#[cfg(not(any( - feature = "pulseaudio_backend", - feature = "portaudio_backend", - feature = "alsa_backend", - feature = "pipe_backend", - feature = "rodio_backend", - feature = "rodiojack_backend", -)))] -compile_error!("At least one of the backend features is required!"); -static BACKEND_VALUES: &[&str] = &[ - #[cfg(feature = "alsa_backend")] - "alsa", - #[cfg(feature = "pulseaudio_backend")] - "pulseaudio", - #[cfg(feature = "portaudio_backend")] - "portaudio", - #[cfg(feature = "rodio_backend")] - "rodio", - #[cfg(feature = "pipe_backend")] - "pipe", - #[cfg(feature = "rodiojack_backend")] - "rodiojack", -]; - -/// The backend used by librespot -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, StructOpt)] -#[serde(rename_all = "lowercase")] -pub enum Backend { - Alsa, - PortAudio, - PulseAudio, - Rodio, - Pipe, - RodioJack, -} - -fn default_backend() -> Backend { - Backend::from_str(BACKEND_VALUES.first().unwrap()).unwrap() -} - -impl FromStr for Backend { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "alsa" => Ok(Backend::Alsa), - "portaudio" => Ok(Backend::PortAudio), - "pulseaudio" => Ok(Backend::PulseAudio), - "rodio" => Ok(Backend::Rodio), - "pipe" => Ok(Backend::Pipe), - "rodiojack" => Ok(Backend::RodioJack), - _ => unreachable!(), - } - } -} - -impl fmt::Display for Backend { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Backend::Alsa => write!(f, "alsa"), - Backend::PortAudio => write!(f, "portaudio"), - Backend::PulseAudio => write!(f, "pulseaudio"), - Backend::Rodio => write!(f, "rodio"), - Backend::Pipe => write!(f, "pipe"), - Backend::RodioJack => write!(f, "rodiojack"), - } - } -} - -static VOLUME_CONTROLLER_VALUES: &[&str] = &[ - "softvol", - #[cfg(feature = "alsa_backend")] - "alsa", - #[cfg(feature = "alsa_backend")] - "alsa_linear", - "none", -]; - -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, StructOpt)] +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, ValueEnum)] #[serde(rename_all = "snake_case")] pub enum VolumeController { + #[cfg(feature = "alsa_backend")] Alsa, + #[cfg(feature = "alsa_backend")] AlsaLinear, #[serde(rename = "softvol")] SoftVolume, None, } -impl FromStr for VolumeController { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "alsa" => Ok(VolumeController::Alsa), - "alsa_linear" => Ok(VolumeController::AlsaLinear), - "softvol" => Ok(VolumeController::SoftVolume), - "none" => Ok(VolumeController::None), - _ => unreachable!(), - } - } -} - -static DEVICETYPE_VALUES: &[&str] = &[ - "computer", - "tablet", - "smartphone", - "speaker", - "tv", - "avr", - "stb", - "audiodongle", - "gameconsole", - "castaudio", - "castvideo", - "automobile", - "smartwatch", - "chromebook", - "carthing", - "homething", -]; - // Spotify's device type (copied from it's config.rs) -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, StructOpt)] +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, ValueEnum)] #[serde(rename_all = "snake_case")] pub enum DeviceType { Unknown, @@ -168,34 +60,8 @@ pub enum DeviceType { HomeThing, } -impl From for DeviceType { - fn from(item: LSDeviceType) -> Self { - match item { - LSDeviceType::Unknown => DeviceType::Unknown, - LSDeviceType::Computer => DeviceType::Computer, - LSDeviceType::Tablet => DeviceType::Tablet, - LSDeviceType::Smartphone => DeviceType::Smartphone, - LSDeviceType::Speaker => DeviceType::Speaker, - LSDeviceType::Tv => DeviceType::Tv, - LSDeviceType::Avr => DeviceType::Avr, - LSDeviceType::Stb => DeviceType::Stb, - LSDeviceType::AudioDongle => DeviceType::AudioDongle, - LSDeviceType::GameConsole => DeviceType::GameConsole, - LSDeviceType::CastAudio => DeviceType::CastAudio, - LSDeviceType::CastVideo => DeviceType::CastVideo, - LSDeviceType::Automobile => DeviceType::Automobile, - LSDeviceType::Smartwatch => DeviceType::Smartwatch, - LSDeviceType::Chromebook => DeviceType::Chromebook, - LSDeviceType::UnknownSpotify => DeviceType::UnknownSpotify, - LSDeviceType::CarThing => DeviceType::CarThing, - LSDeviceType::Observer => DeviceType::Observer, - LSDeviceType::HomeThing => DeviceType::HomeThing, - } - } -} - -impl From<&DeviceType> for LSDeviceType { - fn from(item: &DeviceType) -> Self { +impl From for LSDeviceType { + fn from(item: DeviceType) -> Self { match item { DeviceType::Unknown => LSDeviceType::Unknown, DeviceType::Computer => LSDeviceType::Computer, @@ -220,26 +86,18 @@ impl From<&DeviceType> for LSDeviceType { } } -impl FromStr for DeviceType { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - let dt = LSDeviceType::from_str(s).unwrap(); - Ok(dt.into()) - } +fn bitrate_parser() -> impl IntoResettable { + let possible_values: PossibleValuesParser = ["96", "160", "320"].into(); + possible_values.map(|val| match val.as_str() { + "96" => Bitrate::Bitrate96, + "160" => Bitrate::Bitrate160, + "320" => Bitrate::Bitrate320, + _ => unreachable!(), + }) } -impl fmt::Display for DeviceType { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let dt: LSDeviceType = self.into(); - write!(f, "{dt}") - } -} - -static BITRATE_VALUES: &[&str] = &["96", "160", "320"]; - /// Spotify's audio bitrate -#[derive(Clone, Copy, Debug, PartialEq, Eq, StructOpt)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, ValueEnum)] pub enum Bitrate { Bitrate96, Bitrate160, @@ -261,19 +119,6 @@ impl<'de> Deserialize<'de> for Bitrate { } } -impl FromStr for Bitrate { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "96" => Ok(Bitrate::Bitrate96), - "160" => Ok(Bitrate::Bitrate160), - "320" => Ok(Bitrate::Bitrate320), - _ => unreachable!(), - } - } -} - impl From for LSBitrate { fn from(bitrate: Bitrate) -> Self { match bitrate { @@ -284,41 +129,14 @@ impl From for LSBitrate { } } -#[cfg(feature = "dbus_mpris")] -static DBUSTYPE_VALUES: &[&str] = &["session", "system"]; - -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, StructOpt)] +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, ValueEnum)] #[serde(rename_all = "snake_case")] pub enum DBusType { Session, System, } -impl FromStr for DBusType { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "session" => Ok(DBusType::Session), - "system" => Ok(DBusType::System), - _ => unreachable!(), - } - } -} - -impl fmt::Display for DBusType { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - DBusType::Session => write!(f, "session"), - DBusType::System => write!(f, "system"), - } - } -} - -/// LibreSpot supported audio formats -static AUDIO_FORMAT_VALUES: &[&str] = &["F32", "S32", "S24", "S24_3", "S16"]; - -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, StructOpt)] +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, ValueEnum)] pub enum AudioFormat { F32, S32, @@ -327,33 +145,6 @@ pub enum AudioFormat { S16, } -impl FromStr for AudioFormat { - type Err = ParseError; - - fn from_str(s: &str) -> Result { - match s { - "F32" => Ok(AudioFormat::F32), - "S32" => Ok(AudioFormat::S32), - "S24" => Ok(AudioFormat::S24), - "S24_3" => Ok(AudioFormat::S24_3), - "S16" => Ok(AudioFormat::S16), - _ => unreachable!(), - } - } -} - -impl fmt::Display for AudioFormat { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - AudioFormat::F32 => write!(f, "F32"), - AudioFormat::S32 => write!(f, "S32"), - AudioFormat::S24 => write!(f, "S24"), - AudioFormat::S24_3 => write!(f, "S24_3"), - AudioFormat::S16 => write!(f, "S16"), - } - } -} - impl From for LSAudioFormat { fn from(audio_format: AudioFormat) -> Self { match audio_format { @@ -366,177 +157,176 @@ impl From for LSAudioFormat { } } -#[derive(Debug, Default, StructOpt)] -#[structopt( - about = "A Spotify daemon", - author, - name = "spotifyd", - setting(AppSettings::ColoredHelp) -)] +fn possible_backends() -> Vec<&'static str> { + audio_backend::BACKENDS.iter().map(|b| b.0).collect() +} + +#[derive(Debug, Default, Parser)] +#[command(version, about, long_about = None)] pub struct CliConfig { /// The path to the config file to use - #[structopt(long, value_name = "string")] + #[arg(long, value_name = "PATH")] pub config_path: Option, /// If set, starts spotifyd without detaching - #[structopt(long)] + #[arg(long)] pub no_daemon: bool, /// Prints more verbose output - #[structopt(long)] - pub verbose: bool, + #[arg(short, long, action = clap::ArgAction::Count)] + pub verbose: u8, /// Path to PID file. - #[structopt(long)] + #[cfg(unix)] + #[arg(long, value_name = "PATH")] pub pid: Option, - #[structopt(flatten)] + #[command(flatten)] pub shared_config: SharedConfigValues, } // A struct that holds all allowed config fields. // The actual config file is made up of two sections, spotifyd and global. -#[derive(Clone, Default, Deserialize, PartialEq, StructOpt)] +#[derive(Clone, Default, Debug, Deserialize, PartialEq, Args)] pub struct SharedConfigValues { - /// The Spotify account user name - #[structopt(conflicts_with = "username_cmd", long, short, value_name = "string")] - username: Option, - - /// A command that can be used to retrieve the Spotify account username - #[structopt( - conflicts_with = "username", - long, - short = "U", - value_name = "string", - visible_alias = "username_cmd" - )] - username_cmd: Option, - - /// The Spotify account password - #[structopt(conflicts_with = "password_cmd", long, short, value_name = "string")] - password: Option, - - /// Enables keyring password access - #[cfg_attr( - feature = "dbus_keyring", - structopt(long), - serde(alias = "use-keyring", default) - )] - #[cfg_attr(not(feature = "dbus_keyring"), structopt(skip), serde(skip))] - use_keyring: bool, - - /// Enables the MPRIS interface - #[cfg_attr( - feature = "dbus_mpris", - structopt(long), - serde(alias = "use-mpris", default) - )] - #[cfg_attr(not(feature = "dbus_mpris"), structopt(skip), serde(skip))] - use_mpris: Option, - - /// The Bus-type to use for the MPRIS interface - #[cfg_attr( - feature = "dbus_mpris", - structopt(long, possible_values = &DBUSTYPE_VALUES, value_name = "string") - )] - #[cfg_attr(not(feature = "dbus_mpris"), structopt(skip), serde(skip))] - dbus_type: Option, - - /// A command that can be used to retrieve the Spotify account password - #[structopt( - conflicts_with = "password", - long, - short = "P", - value_name = "string", - visible_alias = "password_cmd" - )] - password_cmd: Option, - - /// Whether the credentials should be debugged. - #[structopt(long)] - #[serde(skip)] - debug_credentials: bool, - /// A script that gets evaluated in the user's shell when the song changes - #[structopt(visible_alias = "onevent", long, value_name = "string")] + #[arg(visible_alias = "onevent", long, value_name = "CMD")] #[serde(alias = "onevent")] on_song_change_hook: Option, /// The cache path used to store credentials and music file artifacts - #[structopt(long, parse(from_os_str), short, value_name = "string")] + #[arg(long, short, value_name = "PATH")] cache_path: Option, /// The maximal cache size in bytes - #[structopt(long)] + #[arg(long, value_name = "BYTES")] max_cache_size: Option, /// Disable the use of audio cache - #[structopt(long)] - #[serde(default)] - no_audio_cache: bool, + #[arg( + long, + default_missing_value("true"), + require_equals = true, + num_args(0..=1), + value_name = "BOOL" + )] + no_audio_cache: Option, /// The audio backend to use - #[structopt(long, short, possible_values = &BACKEND_VALUES, value_name = "string")] - backend: Option, + #[arg(long, short, value_parser = possible_backends())] + backend: Option, /// The volume controller to use - #[structopt(long, short, possible_values = &VOLUME_CONTROLLER_VALUES, visible_alias = "volume-control")] + #[arg(value_enum, long, visible_alias = "volume-control")] #[serde(alias = "volume-control")] volume_controller: Option, - /// The audio device (or file handle if using pipe backend) - #[structopt(long, value_name = "string")] + /// The audio device (or pipe file) + #[arg(long)] device: Option, - /// The control device - #[structopt(long, value_name = "string")] - control: Option, - - /// The mixer to use - #[structopt(long, value_name = "string")] - mixer: Option, - /// The device name displayed in Spotify - #[structopt(long, short, value_name = "string")] + #[arg(long, short)] device_name: Option, /// The bitrate of the streamed audio data - #[structopt(long, short = "B", possible_values = &BITRATE_VALUES, value_name = "number")] + #[arg(long, short = 'B', value_parser = bitrate_parser())] bitrate: Option, /// The audio format of the streamed audio data - #[structopt(long, possible_values = &AUDIO_FORMAT_VALUES, value_name = "string")] + #[arg(value_enum, long)] audio_format: Option, /// Initial volume between 0 and 100 - #[structopt(long, value_name = "initial_volume")] - initial_volume: Option, + #[arg(long)] + initial_volume: Option, /// Enable to normalize the volume during playback - #[structopt(long)] - #[serde(default)] - volume_normalisation: bool, + #[arg( + long, + default_missing_value("true"), + require_equals = true, + num_args(0..=1), + value_name = "BOOL" + )] + volume_normalisation: Option, /// A custom pregain applied before sending the audio to the output device - #[structopt(long, value_name = "number")] + #[arg(long)] normalisation_pregain: Option, + #[arg( + long, + default_missing_value("true"), + require_equals = true, + num_args(0..=1), + value_name = "BOOL" + )] + disable_discovery: Option, + /// The port used for the Spotify Connect discovery - #[structopt(long, value_name = "number")] + #[arg(long)] zeroconf_port: Option, /// The proxy used to connect to spotify's servers - #[structopt(long, value_name = "string")] + #[arg(long, value_name = "URL")] proxy: Option, /// The device type shown to clients - #[structopt(long, possible_values = &DEVICETYPE_VALUES, value_name = "string")] + #[arg(value_enum, long)] device_type: Option, /// Start playing similar songs after your music has ended - #[structopt(long)] + #[arg( + long, + default_missing_value("true"), + require_equals = true, + num_args(0..=1), + value_name = "BOOL" + )] #[serde(default)] - autoplay: bool, + autoplay: Option, + + #[cfg(feature = "alsa_backend")] + #[command(flatten)] + #[serde(flatten)] + alsa_config: AlsaConfig, + + #[cfg(feature = "dbus_mpris")] + #[command(flatten)] + #[serde(flatten)] + mpris_config: MprisConfig, +} + +#[cfg(feature = "dbus_mpris")] +#[derive(Debug, Default, Clone, Deserialize, Args, PartialEq, Eq)] +pub struct MprisConfig { + /// Enables the MPRIS interface + #[arg( + long, + default_missing_value("true"), + require_equals = true, + num_args(0..=1), + value_name = "BOOL" + )] + #[serde(alias = "use-mpris")] + pub(crate) use_mpris: Option, + + /// The Bus-type to use for the MPRIS interface + #[arg(value_enum, long)] + pub(crate) dbus_type: Option, +} + +#[cfg(feature = "alsa_backend")] +#[derive(Debug, Default, Clone, Deserialize, Args, PartialEq, Eq)] +pub struct AlsaConfig { + /// The control device + #[arg(long)] + pub(crate) control: Option, + + /// The mixer to use + #[arg(long)] + pub(crate) mixer: Option, } #[derive(Debug, Default, Deserialize)] @@ -547,84 +337,13 @@ pub struct FileConfig { impl FileConfig { pub fn get_merged_sections(self) -> Option { - let global_config_section = self.global; - let spotifyd_config_section = self.spotifyd; - - let merged_config: Option; - // First merge the two sections together. The spotifyd has priority over global - // section. - if let Some(mut spotifyd_section) = spotifyd_config_section { - // spotifyd section exists. Try to merge it with global section. - #[allow(clippy::branches_sharing_code)] - if let Some(global_section) = global_config_section { - spotifyd_section.merge_with(global_section); - merged_config = Some(spotifyd_section); - } else { - // There is no global section. Just use the spotifyd section. - merged_config = Some(spotifyd_section); + match (self.global, self.spotifyd) { + (Some(global), Some(mut spotifyd)) => { + spotifyd.merge_with(global); + Some(spotifyd) } - } else { - // No spotifyd config available. Check for global and use that, if both are - // none, use none. - merged_config = global_config_section; - } - - merged_config - } -} - -impl fmt::Debug for SharedConfigValues { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let placeholder = "taken out for privacy"; - - macro_rules! extract_credential { - ( $e:expr ) => { - match $e { - Some(s) => match self.debug_credentials { - true => Some(s.as_str()), - false => Some(placeholder), - }, - None => None, - } - }; + (global, spotifyd) => global.or(spotifyd), } - - let password_value = extract_credential!(&self.password); - - let password_cmd_value = extract_credential!(&self.password_cmd); - - let username_value = extract_credential!(&self.username); - - let username_cmd_value = extract_credential!(&self.username_cmd); - - f.debug_struct("SharedConfigValues") - .field("username", &username_value) - .field("username_cmd", &username_cmd_value) - .field("password", &password_value) - .field("password_cmd", &password_cmd_value) - .field("use_keyring", &self.use_keyring) - .field("use_mpris", &self.use_mpris) - .field("dbus_type", &self.dbus_type) - .field("on_song_change_hook", &self.on_song_change_hook) - .field("cache_path", &self.cache_path) - .field("no-audio-cache", &self.no_audio_cache) - .field("backend", &self.backend) - .field("volume_controller", &self.volume_controller) - .field("device", &self.device) - .field("control", &self.control) - .field("mixer", &self.mixer) - .field("device_name", &self.device_name) - .field("bitrate", &self.bitrate) - .field("audio_format", &self.audio_format) - .field("initial_volume", &self.initial_volume) - .field("volume_normalisation", &self.volume_normalisation) - .field("normalisation_pregain", &self.normalisation_pregain) - .field("zeroconf_port", &self.zeroconf_port) - .field("proxy", &self.proxy) - .field("device_type", &self.device_type) - .field("autoplay", &self.autoplay) - .field("max_cache_size", &self.max_cache_size) - .finish() } } @@ -659,52 +378,46 @@ impl CliConfig { } impl SharedConfigValues { - pub fn merge_with(&mut self, other: SharedConfigValues) { + pub fn merge_with(&mut self, mut other: SharedConfigValues) { macro_rules! merge { - ($($x:ident),+) => { - $(self.$x = self.$x.clone().or_else(|| other.$x.clone());)+ + ($a:expr; and $b:expr => {$($x:ident),+}) => { + $($a.$x = $a.$x.take().or_else(|| $b.$x.take());)+ } } // Handles Option merging. - merge!( + merge!(self; and other => { backend, - username, - username_cmd, - password, - password_cmd, + volume_normalisation, normalisation_pregain, bitrate, initial_volume, device_name, - mixer, - control, device, volume_controller, cache_path, + no_audio_cache, on_song_change_hook, + disable_discovery, zeroconf_port, proxy, device_type, - use_mpris, max_cache_size, - dbus_type, - audio_format - ); - - // Handles boolean merging. - self.use_keyring |= other.use_keyring; - self.volume_normalisation |= other.volume_normalisation; - self.no_audio_cache |= other.no_audio_cache; - self.autoplay |= other.autoplay; + audio_format, + autoplay + }); + + #[cfg(feature = "dbus_mpris")] + merge!(self.mpris_config; and other.mpris_config => {use_mpris, dbus_type}); + #[cfg(feature = "alsa_backend")] + merge!(self.alsa_config; and other.alsa_config => {mixer, control}); } } pub(crate) fn get_config_file() -> Option { let etc_conf = format!("/etc/{}", CONFIG_FILE_NAME); - let dirs = directories::BaseDirs::new()?; + let dirs = directories::ProjectDirs::from("", "", "spotifyd")?; let mut path = dirs.config_dir().to_path_buf(); - path.push("spotifyd"); path.push(CONFIG_FILE_NAME); if path.exists() { @@ -722,41 +435,42 @@ fn device_id(name: &str) -> String { } pub(crate) struct SpotifydConfig { - pub(crate) username: Option, - pub(crate) password: Option, - #[allow(unused)] - pub(crate) use_keyring: bool, - pub(crate) use_mpris: bool, - pub(crate) dbus_type: DBusType, pub(crate) cache: Option, pub(crate) backend: Option, pub(crate) audio_device: Option, pub(crate) audio_format: LSAudioFormat, - #[allow(unused)] - pub(crate) control_device: Option, - #[allow(unused)] - pub(crate) mixer: Option, - #[allow(unused)] pub(crate) volume_controller: VolumeController, pub(crate) initial_volume: Option, pub(crate) device_name: String, pub(crate) player_config: PlayerConfig, pub(crate) session_config: SessionConfig, pub(crate) onevent: Option, - #[allow(unused)] + #[cfg(unix)] pub(crate) pid: Option, pub(crate) shell: String, + pub(crate) discovery: bool, pub(crate) zeroconf_port: Option, - pub(crate) device_type: String, + pub(crate) device_type: LSDeviceType, + #[cfg(feature = "dbus_mpris")] + pub(crate) mpris: MprisConfig, + #[cfg(feature = "alsa_backend")] + pub(crate) alsa_config: AlsaConfig, } pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { - let audio_cache = !config.shared_config.no_audio_cache; + let audio_cache = !config.shared_config.no_audio_cache.unwrap_or(false); let size_limit = config.shared_config.max_cache_size; let cache = config .shared_config .cache_path + .or_else(|| { + ProjectDirs::from("", "", "spotifyd").map(|dirs| dirs.cache_dir().to_path_buf()) + }) + .or_else(|| { + warn!("failed to determine cache directory, please specify one manually!"); + None + }) .map(|path| { Cache::new( Some(&path), @@ -783,12 +497,6 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { .unwrap_or(AudioFormat::S16) .into(); - let backend = config - .shared_config - .backend - .unwrap_or_else(default_backend) - .to_string(); - let volume_controller = config .shared_config .volume_controller @@ -797,14 +505,15 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { let initial_volume: Option = config .shared_config .initial_volume - .and_then(|input| match input.parse::() { - Ok(v) if (0..=100).contains(&v) => Some(v), - _ => { - warn!("Could not parse initial_volume (must be in the range 0-100)"); - None + .filter(|val| { + if (0..=100).contains(val) { + true + } else { + warn!("initial_volume must be in range 0..100"); + false } }) - .map(|volume| (volume as i32 * 0xFFFF / 100) as u16); + .map(|volume| (volume as i32 * (u16::MAX as i32) / 100) as u16); let device_name = config .shared_config @@ -816,15 +525,13 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { let normalisation_pregain = config.shared_config.normalisation_pregain.unwrap_or(0.0); - let dbus_type = config.shared_config.dbus_type.unwrap_or(DBusType::Session); - let autoplay = config.shared_config.autoplay; - let device_type = config .shared_config .device_type .unwrap_or(DeviceType::Speaker) - .to_string(); + .into(); + #[cfg(unix)] let pid = config.pid.map(|f| { f.into_os_string() .into_string() @@ -836,30 +543,6 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { "sh".to_string() }); - let mut username = config.shared_config.username; - if username.is_none() { - info!("No username specified. Checking username_cmd"); - match config.shared_config.username_cmd { - Some(ref cmd) => match run_program(&shell, cmd) { - Ok(s) => username = Some(s.trim().to_string()), - Err(e) => error!("{}", CrateError::subprocess_with_err(&shell, cmd, e)), - }, - None => info!("No username_cmd specified"), - } - } - - let mut password = config.shared_config.password; - if password.is_none() { - info!("No password specified. Checking password_cmd"); - - match config.shared_config.password_cmd { - Some(ref cmd) => match run_program(&shell, cmd) { - Ok(s) => password = Some(s.trim().to_string()), - Err(e) => error!("{}", CrateError::subprocess_with_err(&shell, cmd, e)), - }, - None => info!("No password_cmd specified"), - } - } let mut proxy_url = None; match config.shared_config.proxy { Some(s) => match Url::parse(&s) { @@ -888,7 +571,7 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { // should consider adding them to Spotifyd's config system. let pc = PlayerConfig { bitrate, - normalisation: config.shared_config.volume_normalisation, + normalisation: config.shared_config.volume_normalisation.unwrap_or(false), normalisation_pregain_db: normalisation_pregain, gapless: true, ditherer, @@ -896,33 +579,32 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { }; SpotifydConfig { - username, - password, - use_keyring: config.shared_config.use_keyring, - use_mpris: config.shared_config.use_mpris.unwrap_or(true), - dbus_type, cache, - backend: Some(backend), + backend: config.shared_config.backend, audio_device: config.shared_config.device, audio_format, - control_device: config.shared_config.control, - mixer: config.shared_config.mixer, volume_controller, initial_volume, device_name, player_config: pc, session_config: SessionConfig { - autoplay: Some(autoplay), + autoplay: config.shared_config.autoplay, device_id, proxy: proxy_url, ap_port: Some(443), ..Default::default() }, onevent: config.shared_config.on_song_change_hook, - pid, shell, + discovery: !config.shared_config.disable_discovery.unwrap_or(false), zeroconf_port: config.shared_config.zeroconf_port, device_type, + #[cfg(unix)] + pid, + #[cfg(feature = "dbus_mpris")] + mpris: config.shared_config.mpris_config, + #[cfg(feature = "alsa_backend")] + alsa_config: config.shared_config.alsa_config, } } @@ -933,12 +615,12 @@ mod tests { #[test] fn test_section_merging() { let mut spotifyd_section = SharedConfigValues { - password: Some("123456".to_string()), + device_type: Some(DeviceType::Computer), ..Default::default() }; let global_section = SharedConfigValues { - username: Some("testUserName".to_string()), + device_name: Some("spotifyd-test".to_string()), ..Default::default() }; @@ -952,15 +634,7 @@ mod tests { let merged_config = file_config.get_merged_sections().unwrap(); // Add the new field to spotifyd section. - spotifyd_section.username = Some("testUserName".to_string()); + spotifyd_section.device_name = Some("spotifyd-test".to_string()); assert_eq!(merged_config, spotifyd_section); } - #[test] - fn test_default_backend() { - let spotifyd_config = get_internal_config(CliConfig::default()); - assert_eq!( - spotifyd_config.backend.unwrap(), - default_backend().to_string() - ); - } } diff --git a/src/main.rs b/src/main.rs index a61068c1..941f2e6b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,5 @@ use crate::config::CliConfig; +use clap::Parser; #[cfg(unix)] use color_eyre::eyre::eyre; use color_eyre::{ @@ -14,7 +15,6 @@ use log::{info, trace, LevelFilter}; use pledge::pledge; #[cfg(windows)] use std::fs; -use structopt::StructOpt; use tokio::runtime::Runtime; #[cfg(feature = "alsa_backend")] @@ -34,16 +34,15 @@ enum LogTarget { Syslog, } -fn setup_logger(log_target: LogTarget, verbose: bool) -> eyre::Result<()> { - let log_level = if verbose { - LevelFilter::Trace - } else { - LevelFilter::Info +fn setup_logger(log_target: LogTarget, verbose: u8) -> eyre::Result<()> { + let log_level = match verbose { + 0 => LevelFilter::Info, + 1 => LevelFilter::Debug, + 2.. => LevelFilter::Trace, }; - let mut logger = fern::Dispatch::new().level(log_level); - logger = if verbose { + logger = if verbose > 0 { logger.format(|out, message, record| { out.finish(format_args!( "[{} {}] {}", @@ -107,7 +106,7 @@ fn main() -> eyre::Result<()> { color_eyre::install().wrap_err("Couldn't initialize error reporting")?; - let mut cli_config: CliConfig = CliConfig::from_args(); + let mut cli_config = CliConfig::parse(); let is_daemon = !cli_config.no_daemon; @@ -218,9 +217,8 @@ fn main() -> eyre::Result<()> { let runtime = Runtime::new().unwrap(); runtime.block_on(async { - let initial_state = setup::initial_state(internal_config); + let initial_state = setup::initial_state(internal_config)?; initial_state.run().await; - }); - - Ok(()) + Ok(()) + }) } diff --git a/src/main_loop.rs b/src/main_loop.rs index 0155c4bf..41827773 100644 --- a/src/main_loop.rs +++ b/src/main_loop.rs @@ -1,4 +1,5 @@ -use crate::config::DBusType; +#[cfg(feature = "dbus_mpris")] +use crate::config::{DBusType, MprisConfig}; #[cfg(feature = "dbus_mpris")] use crate::dbus_mpris::DbusServer; use crate::process::spawn_program_on_event; @@ -77,11 +78,9 @@ pub(crate) struct MainLoop { pub(crate) device_type: DeviceType, pub(crate) device_name: String, pub(crate) player_event_program: Option, - #[cfg_attr(not(feature = "dbus_mpris"), allow(unused))] - pub(crate) use_mpris: bool, - #[cfg_attr(not(feature = "dbus_mpris"), allow(unused))] - pub(crate) dbus_type: DBusType, pub(crate) credentials_provider: CredentialsProvider, + #[cfg(feature = "dbus_mpris")] + pub(crate) mpris_config: MprisConfig, } struct ConnectionInfo> { @@ -140,9 +139,12 @@ impl MainLoop { } #[cfg(feature = "dbus_mpris")] - let mpris_event_tx = if self.use_mpris { + let mpris_event_tx = if self.mpris_config.use_mpris.unwrap_or(true) { let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); - *dbus_server.as_mut() = Either::Left(DbusServer::new(rx, self.dbus_type)); + *dbus_server.as_mut() = Either::Left(DbusServer::new( + rx, + self.mpris_config.dbus_type.unwrap_or(DBusType::Session), + )); Some(tx) } else { None diff --git a/src/process.rs b/src/process.rs index 39d92ac4..3d781e6c 100644 --- a/src/process.rs +++ b/src/process.rs @@ -8,24 +8,6 @@ use tokio::{ process::{self, Command}, }; -/// Blocks while provided command is run in a subprocess using the provided -/// shell. If successful, returns the contents of the subprocess's `stdout` as a -/// `String`. -pub(crate) fn run_program(shell: &str, cmd: &str) -> Result { - info!("Running {:?} using {:?}", cmd, shell); - let output = std::process::Command::new(shell) - .arg("-c") - .arg(cmd) - .output() - .map_err(|e| Error::subprocess_with_err(shell, cmd, e))?; - if !output.status.success() { - let s = std::str::from_utf8(&output.stderr).map_err(|_| Error::subprocess(shell, cmd))?; - return Err(Error::subprocess_with_str(shell, cmd, s)); - } - let s = String::from_utf8(output.stdout).map_err(|_| Error::subprocess(shell, cmd))?; - Ok(s) -} - /// Spawns provided command in a subprocess using the provided shell. fn spawn_program(shell: &str, cmd: &str, env: HashMap<&str, String>) -> Result { info!( diff --git a/src/setup.rs b/src/setup.rs index 735c9ca7..4cecd879 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -4,20 +4,20 @@ use crate::{ config, main_loop::{self, CredentialsProvider}, }; -#[cfg(feature = "dbus_keyring")] -use keyring::Entry; -use librespot_core::{authentication::Credentials, cache::Cache, config::DeviceType}; -use librespot_playback::mixer::MixerConfig; +use color_eyre::{ + eyre::{eyre, Context}, + Section, +}; use librespot_playback::{ - audio_backend::{Sink, BACKENDS}, - config::AudioFormat, - mixer::{self, Mixer}, + audio_backend::{self}, + mixer::{self, Mixer, MixerConfig}, }; -#[allow(unused_imports)] // cfg -use log::{debug, error, info, warn}; -use std::{str::FromStr, sync::Arc, thread, time::Duration}; +use log::{debug, error, info}; +use std::{sync::Arc, thread, time::Duration}; -pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLoop { +pub(crate) fn initial_state( + config: config::SpotifydConfig, +) -> color_eyre::Result { let mixer: Arc = { match config.volume_controller { config::VolumeController::None => { @@ -27,8 +27,8 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo #[cfg(feature = "alsa_backend")] config::VolumeController::Alsa | config::VolumeController::AlsaLinear => { let audio_device = config.audio_device.clone(); - let control_device = config.control_device.clone(); - let mixer = config.mixer.clone(); + let control_device = config.alsa_config.control.clone(); + let mixer = config.alsa_config.mixer.clone(); info!("Using alsa volume controller."); let linear = matches!( config.volume_controller, @@ -59,34 +59,10 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo let zeroconf_port = config.zeroconf_port.unwrap_or(0); - let device_type: DeviceType = DeviceType::from_str(&config.device_type).unwrap_or_default(); - - let username = config.username; - #[allow(unused_mut)] // mut is needed behind the dbus_keyring flag. - let mut password = config.password; - - #[cfg(feature = "dbus_keyring")] - if config.use_keyring { - match (&username, &password) { - (None, _) => warn!("Can't query the keyring without a username"), - (Some(_), Some(_)) => { - info!("Keyring is ignored, since you already configured a password") - } - (Some(username), None) => { - info!("Checking keyring for password"); - let entry = Entry::new("spotifyd", username); - match entry.and_then(|e| e.get_password()) { - Ok(retrieved_password) => password = Some(retrieved_password), - Err(e) => error!("Keyring did not return any results: {e}"), - } - } - } - } - let credentials_provider = - if let Some(credentials) = get_credentials(&cache, &username, &password) { + if let Some(credentials) = cache.as_ref().and_then(|c| c.credentials()) { CredentialsProvider::SpotifyCredentials(credentials) - } else { + } else if config.discovery { info!("no usable credentials found, enabling discovery"); debug!("Using device id '{}'", session_config.device_id); const RETRY_MAX: u8 = 4; @@ -98,7 +74,7 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo session_config.client_id.clone(), ) .name(config.device_name.clone()) - .device_type(device_type) + .device_type(config.device_type) .port(zeroconf_port) .launch() { @@ -106,7 +82,9 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo Err(err) => { error!("failed to enable discovery: {err}"); if retry_counter >= RETRY_MAX { - panic!("failed to enable discovery (and no credentials provided)"); + return Err(err).with_context(|| { + "failed to enable discovery (and no credentials provided)" + }); } info!("retrying discovery in {} seconds", backoff.as_secs()); thread::sleep(backoff); @@ -117,11 +95,16 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo } }; discovery_stream.into() + } else { + return Err(eyre!( + "no cached credentials available and discovery disabled" + )) + .with_suggestion(|| "consider enabling discovery or authenticating via OAuth"); }; - let backend = find_backend(backend.as_ref().map(String::as_ref)); + let backend = audio_backend::find(backend).expect("available backends should match ours"); - main_loop::MainLoop { + Ok(main_loop::MainLoop { credentials_provider, mixer, session_config, @@ -133,48 +116,10 @@ pub(crate) fn initial_state(config: config::SpotifydConfig) -> main_loop::MainLo initial_volume: config.initial_volume, has_volume_ctrl, shell: config.shell, - device_type, + device_type: config.device_type, device_name: config.device_name, player_event_program: config.onevent, - use_mpris: config.use_mpris, - dbus_type: config.dbus_type, - } -} - -fn get_credentials( - cache: &Option, - username: &Option, - password: &Option, -) -> Option { - if let Some(credentials) = cache.as_ref().and_then(Cache::credentials) { - if Option::zip(username.as_deref(), credentials.username.as_deref()) - .is_some_and(|(user_config, user_cached)| user_config == user_cached) - { - return Some(credentials); - } - } - - Some(Credentials::with_password( - username.as_ref()?, - password.as_ref()?, - )) -} - -fn find_backend(name: Option<&str>) -> fn(Option, AudioFormat) -> Box { - match name { - Some(name) => { - BACKENDS - .iter() - .find(|backend| name == backend.0) - .unwrap_or_else(|| panic!("Unknown backend: {}.", name)) - .1 - } - None => { - let &(name, back) = BACKENDS - .first() - .expect("No backends were enabled at build time"); - info!("No backend specified, defaulting to: {}.", name); - back - } - } + #[cfg(feature = "dbus_mpris")] + mpris_config: config.mpris, + }) } From 761ba925a4af4d7ca8dc9c1230d7d6f6272a480a Mon Sep 17 00:00:00 2001 From: eladyn Date: Mon, 30 Dec 2024 02:03:43 +0100 Subject: [PATCH 2/6] config: allow both number and string for initial_volume --- src/config.rs | 184 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 131 insertions(+), 53 deletions(-) diff --git a/src/config.rs b/src/config.rs index b4035afe..83d6550a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,9 +1,12 @@ use crate::utils; use clap::{ builder::{IntoResettable, PossibleValuesParser, TypedValueParser, ValueParser}, - Args, Parser, ValueEnum, + Args, Parser, Subcommand, ValueEnum, +}; +use color_eyre::{ + eyre::{bail, Context}, + Report, }; -use color_eyre::Report; use directories::ProjectDirs; use gethostname::gethostname; use librespot_core::{cache::Cache, config::DeviceType as LSDeviceType, config::SessionConfig}; @@ -12,10 +15,19 @@ use librespot_playback::{ config::{AudioFormat as LSAudioFormat, Bitrate as LSBitrate, PlayerConfig}, dither::{mk_ditherer, DithererBuilder, TriangularDitherer}, }; -use log::{error, info, warn}; -use serde::{de::Error, de::Unexpected, Deserialize, Deserializer}; +use log::{debug, error, info, warn}; +use serde::{ + de::{self, Error, Unexpected}, + Deserialize, Deserializer, +}; use sha1::{Digest, Sha1}; -use std::{fs, path::Path, path::PathBuf}; +use std::{ + borrow::Cow, + convert::TryInto, + fs, + path::{Path, PathBuf}, + str::FromStr, +}; use url::Url; const CONFIG_FILE_NAME: &str = "spotifyd.conf"; @@ -161,30 +173,71 @@ fn possible_backends() -> Vec<&'static str> { audio_backend::BACKENDS.iter().map(|b| b.0).collect() } +fn number_or_string<'de, D>(de: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let Some(val) = Option::::deserialize(de)? else { + return Ok(None); + }; + + let unexpected = match val { + toml::Value::Integer(num) => { + let num: u8 = num.try_into().map_err(de::Error::custom)?; + return Ok(Some(num)); + } + toml::Value::String(num) => { + return u8::from_str(&num) + .map(Some) + .inspect(|_| warn!("Configuration Warning: `initial_volume` should be a number rather than a string, this will become a hard error in the future")) + .map_err(de::Error::custom) + } + toml::Value::Float(f) => Unexpected::Float(f), + toml::Value::Boolean(b) => Unexpected::Bool(b), + toml::Value::Datetime(_) => Unexpected::Other("datetime"), + toml::Value::Array(_) => Unexpected::Seq, + toml::Value::Table(_) => Unexpected::Map, + }; + Err(de::Error::invalid_type(unexpected, &"number")) +} + #[derive(Debug, Default, Parser)] -#[command(version, about, long_about = None)] +#[command(version, about, long_about = None, args_conflicts_with_subcommands = true)] pub struct CliConfig { /// The path to the config file to use - #[arg(long, value_name = "PATH")] + #[arg(long, value_name = "PATH", global = true)] pub config_path: Option, + /// Prints more verbose output + #[arg(short, long, action = clap::ArgAction::Count, global = true)] + pub verbose: u8, + /// If set, starts spotifyd without detaching #[arg(long)] pub no_daemon: bool, - /// Prints more verbose output - #[arg(short, long, action = clap::ArgAction::Count)] - pub verbose: u8, - /// Path to PID file. #[cfg(unix)] #[arg(long, value_name = "PATH")] pub pid: Option, + #[command(subcommand)] + pub mode: Option, + #[command(flatten)] pub shared_config: SharedConfigValues, } +#[derive(Debug, Subcommand)] +pub enum ExecutionMode { + #[command(visible_alias = "auth")] + Authenticate { + /// The port to use for the OAuth redirect + #[arg(long, default_value_t = 8000)] + oauth_port: u16, + }, +} + // A struct that holds all allowed config fields. // The actual config file is made up of two sections, spotifyd and global. #[derive(Clone, Default, Debug, Deserialize, PartialEq, Args)] @@ -195,7 +248,7 @@ pub struct SharedConfigValues { on_song_change_hook: Option, /// The cache path used to store credentials and music file artifacts - #[arg(long, short, value_name = "PATH")] + #[arg(long, short, value_name = "PATH", global = true)] cache_path: Option, /// The maximal cache size in bytes @@ -239,6 +292,7 @@ pub struct SharedConfigValues { /// Initial volume between 0 and 100 #[arg(long)] + #[serde(deserialize_with = "number_or_string")] initial_volume: Option, /// Enable to normalize the volume during playback @@ -378,6 +432,56 @@ impl CliConfig { } impl SharedConfigValues { + pub fn get_cache(&self, for_oauth: bool) -> color_eyre::Result { + let Some(cache_path) = self.cache_path.as_deref().map(Cow::Borrowed).or_else(|| { + ProjectDirs::from("", "", "spotifyd") + .map(|dirs| Cow::Owned(dirs.cache_dir().to_path_buf())) + }) else { + bail!("Failed to determine cache directory, please specify one manually"); + }; + + if for_oauth { + let mut creds_path = cache_path.into_owned(); + creds_path.push("oauth"); + Cache::new(Some(creds_path), None, None, None) + } else { + let audio_cache = !self.no_audio_cache.unwrap_or(false); + + let mut creds_path = cache_path.to_path_buf(); + creds_path.push("zeroconf"); + Cache::new( + Some(creds_path.as_path()), + Some(cache_path.as_ref()), + audio_cache.then_some(cache_path.as_ref()), + self.max_cache_size, + ) + } + .wrap_err("Failed to initialize cache") + } + + pub fn proxy_url(&self) -> Option { + match &self.proxy { + Some(s) => match Url::parse(s) { + Ok(url) => { + if url.scheme() != "http" { + error!("Only HTTP proxies are supported!"); + None + } else { + Some(url) + } + } + Err(err) => { + error!("Invalid proxy URL: {}", err); + None + } + }, + None => { + debug!("No proxy specified"); + None + } + } + } + pub fn merge_with(&mut self, mut other: SharedConfigValues) { macro_rules! merge { ($a:expr; and $b:expr => {$($x:ident),+}) => { @@ -436,6 +540,7 @@ fn device_id(name: &str) -> String { pub(crate) struct SpotifydConfig { pub(crate) cache: Option, + pub(crate) oauth_cache: Option, pub(crate) backend: Option, pub(crate) audio_device: Option, pub(crate) audio_format: LSAudioFormat, @@ -458,32 +563,19 @@ pub(crate) struct SpotifydConfig { } pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { - let audio_cache = !config.shared_config.no_audio_cache.unwrap_or(false); - - let size_limit = config.shared_config.max_cache_size; - let cache = config - .shared_config - .cache_path - .or_else(|| { - ProjectDirs::from("", "", "spotifyd").map(|dirs| dirs.cache_dir().to_path_buf()) - }) - .or_else(|| { - warn!("failed to determine cache directory, please specify one manually!"); - None - }) - .map(|path| { - Cache::new( - Some(&path), - Some(&path), - audio_cache.then_some(&path), - size_limit, - ) - }) - .transpose() - .unwrap_or_else(|e| { - warn!("Cache couldn't be initialized: {e}"); - None - }); + let (cache, oauth_cache) = match ( + config.shared_config.get_cache(false), + config.shared_config.get_cache(true), + ) { + (Ok(cache), Ok(oauth_cache)) => (Some(cache), Some(oauth_cache)), + (a, b) => { + // at least one of the results are err + let err = a.or(b).map(|_| ()).unwrap_err(); + warn!("{err}"); + (None, None) + } + }; + let proxy_url = config.shared_config.proxy_url(); let bitrate: LSBitrate = config .shared_config @@ -543,21 +635,6 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { "sh".to_string() }); - let mut proxy_url = None; - match config.shared_config.proxy { - Some(s) => match Url::parse(&s) { - Ok(url) => { - if url.scheme() != "http" { - error!("Only HTTP proxies are supported!"); - } else { - proxy_url = Some(url); - } - } - Err(err) => error!("Invalid proxy URL: {}", err), - }, - None => info!("No proxy specified"), - } - // choose default ditherer the same way librespot does let ditherer: Option = match audio_format { LSAudioFormat::S16 | LSAudioFormat::S24 | LSAudioFormat::S24_3 => { @@ -580,6 +657,7 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig { SpotifydConfig { cache, + oauth_cache, backend: config.shared_config.backend, audio_device: config.shared_config.device, audio_format, From 0c222f0eee723fa2a61e10f313cfc5507d113294 Mon Sep 17 00:00:00 2001 From: eladyn Date: Mon, 30 Dec 2024 02:06:19 +0100 Subject: [PATCH 3/6] auth: implement oauth support --- Cargo.lock | 1 + Cargo.toml | 1 + src/main.rs | 28 ++++++------ src/main_loop.rs | 35 +++++++++------ src/oauth.rs | 75 +++++++++++++++++++++++++++++++ src/setup.rs | 112 ++++++++++++++++++++++++++++------------------- 6 files changed, 178 insertions(+), 74 deletions(-) create mode 100644 src/oauth.rs diff --git a/Cargo.lock b/Cargo.lock index 0fc9fc82..7310f5d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3232,6 +3232,7 @@ dependencies = [ "librespot-core", "librespot-discovery", "librespot-metadata", + "librespot-oauth", "librespot-playback", "librespot-protocol", "log", diff --git a/Cargo.toml b/Cargo.toml index ffd56cee..360bb895 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ librespot-discovery = "0.6" librespot-connect = "0.6" librespot-metadata = "0.6" librespot-protocol = "0.6" +librespot-oauth = "0.6" toml = "0.8.19" color-eyre = "0.6" directories = "5.0.1" diff --git a/src/main.rs b/src/main.rs index 941f2e6b..f041ab91 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,15 +2,14 @@ use crate::config::CliConfig; use clap::Parser; #[cfg(unix)] use color_eyre::eyre::eyre; -use color_eyre::{ - eyre::{self, Context}, - Help, SectionExt, -}; +use color_eyre::eyre::{self, Context}; +use config::ExecutionMode; #[cfg(unix)] use daemonize::Daemonize; #[cfg(unix)] use log::error; use log::{info, trace, LevelFilter}; +use oauth::run_oauth; #[cfg(target_os = "openbsd")] use pledge::pledge; #[cfg(windows)] @@ -25,6 +24,7 @@ mod dbus_mpris; mod error; mod main_loop; mod no_mixer; +mod oauth; mod process; mod setup; mod utils; @@ -63,7 +63,7 @@ fn setup_logger(log_target: LogTarget, verbose: u8) -> eyre::Result<()> { facility: syslog::Facility::LOG_DAEMON, hostname: None, process: "spotifyd".to_owned(), - pid: 0, + pid: std::process::id(), }; logger.chain( syslog::unix(log_format) @@ -106,8 +106,15 @@ fn main() -> eyre::Result<()> { color_eyre::install().wrap_err("Couldn't initialize error reporting")?; - let mut cli_config = CliConfig::parse(); + let cli_config = CliConfig::parse(); + match cli_config.mode { + None => run_daemon(cli_config), + Some(ExecutionMode::Authenticate { oauth_port }) => run_oauth(cli_config, oauth_port), + } +} + +fn run_daemon(mut cli_config: CliConfig) -> eyre::Result<()> { let is_daemon = !cli_config.no_daemon; let log_target = if is_daemon { @@ -138,14 +145,7 @@ fn main() -> eyre::Result<()> { cli_config .load_config_file_values() - .wrap_err("could not load the config file") - .with_section(|| { - concat!( - "the config format should be valid TOML\n", - "we recently changed the config format, see https://github.com/Spotifyd/spotifyd/issues/765" - ) - .header("note:") - })?; + .wrap_err("could not load the config file")?; trace!("{:?}", &cli_config); // Returns the old SpotifydConfig struct used within the rest of the daemon. diff --git a/src/main_loop.rs b/src/main_loop.rs index 41827773..39a22c9e 100644 --- a/src/main_loop.rs +++ b/src/main_loop.rs @@ -32,28 +32,35 @@ use std::sync::Arc; type DbusServer = Pending<()>; pub(crate) enum CredentialsProvider { - Discovery(Peekable), - SpotifyCredentials(Credentials), -} - -impl From for CredentialsProvider { - fn from(stream: Discovery) -> Self { - CredentialsProvider::Discovery(stream.peekable()) - } + Discovery { + stream: Peekable, + last_credentials: Option, + }, + CredentialsOnly(Credentials), } impl CredentialsProvider { async fn get_credentials(&mut self) -> Credentials { match self { - CredentialsProvider::Discovery(stream) => stream.next().await.unwrap(), - CredentialsProvider::SpotifyCredentials(creds) => creds.clone(), + CredentialsProvider::Discovery { + stream, + last_credentials, + } => { + let new_creds = match last_credentials.take() { + Some(creds) => stream.next().now_or_never().flatten().unwrap_or(creds), + None => stream.next().await.unwrap(), + }; + *last_credentials = Some(new_creds.clone()); + new_creds + } + CredentialsProvider::CredentialsOnly(creds) => creds.clone(), } } // wait for an incoming connection if the underlying provider is a discovery stream async fn incoming_connection(&mut self) { match self { - CredentialsProvider::Discovery(stream) => { + CredentialsProvider::Discovery { stream, .. } => { let peeked = Pin::new(stream).peek().await; if peeked.is_none() { future::pending().await @@ -153,9 +160,6 @@ impl MainLoop { 'mainloop: loop { let connection = tokio::select!( _ = &mut ctrl_c => { - if let CredentialsProvider::Discovery(stream) = self.credentials_provider { - let _ = stream.into_inner().shutdown().await; - } break 'mainloop; } connection = self.get_connection() => { @@ -253,6 +257,9 @@ impl MainLoop { } } } + if let CredentialsProvider::Discovery { stream, .. } = self.credentials_provider { + let _ = stream.into_inner().shutdown().await; + } #[cfg(feature = "dbus_mpris")] if let Either::Left(dbus_server) = Either::as_pin_mut(dbus_server.as_mut()) { if dbus_server.shutdown() { diff --git a/src/oauth.rs b/src/oauth.rs new file mode 100644 index 00000000..1effeee7 --- /dev/null +++ b/src/oauth.rs @@ -0,0 +1,75 @@ +use color_eyre::{ + eyre::{self, Context as _}, + Section as _, +}; +use librespot_core::SessionConfig; +use librespot_core::{authentication::Credentials, Session}; +use log::info; +use tokio::runtime::Runtime; + +use crate::{config::CliConfig, setup_logger, LogTarget}; + +pub(crate) fn run_oauth(mut cli_config: CliConfig, oauth_port: u16) -> eyre::Result<()> { + setup_logger(LogTarget::Terminal, cli_config.verbose)?; + + cli_config + .load_config_file_values() + .wrap_err("failed to read config file")?; + + let cache = cli_config + .shared_config + .get_cache(true) + .with_note(|| "The result of the authentication needs to be cached to be usable later.")?; + + const OAUTH_SCOPES: &[&str] = &[ + "app-remote-control", + "playlist-modify", + "playlist-modify-private", + "playlist-modify-public", + "playlist-read", + "playlist-read-collaborative", + "playlist-read-private", + "streaming", + "ugc-image-upload", + "user-follow-modify", + "user-follow-read", + "user-library-modify", + "user-library-read", + "user-modify", + "user-modify-playback-state", + "user-modify-private", + "user-personalized", + "user-read-birthdate", + "user-read-currently-playing", + "user-read-email", + "user-read-play-history", + "user-read-playback-position", + "user-read-playback-state", + "user-read-private", + "user-read-recently-played", + "user-top-read", + ]; + + let session_config = SessionConfig { + proxy: cli_config.shared_config.proxy_url(), + ..Default::default() + }; + + let token = librespot_oauth::get_access_token( + &session_config.client_id, + &format!("http://127.0.0.1:{oauth_port}/login"), + OAUTH_SCOPES.to_vec(), + ) + .wrap_err("token retrieval failed")?; + + let creds = Credentials::with_access_token(token.access_token); + + Runtime::new().unwrap().block_on(async move { + let session = Session::new(session_config, Some(cache)); + session.connect(creds, true).await + })?; + + info!("\nLogin successful! You are now ready to run spotifyd."); + + Ok(()) +} diff --git a/src/setup.rs b/src/setup.rs index 4cecd879..2d271ab3 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -4,10 +4,8 @@ use crate::{ config, main_loop::{self, CredentialsProvider}, }; -use color_eyre::{ - eyre::{eyre, Context}, - Section, -}; +use color_eyre::{eyre::eyre, Section}; +use futures::StreamExt as _; use librespot_playback::{ audio_backend::{self}, mixer::{self, Mixer, MixerConfig}, @@ -50,7 +48,6 @@ pub(crate) fn initial_state( } }; - let cache = config.cache; let player_config = config.player_config; let session_config = config.session_config; let backend = config.backend.clone(); @@ -59,48 +56,71 @@ pub(crate) fn initial_state( let zeroconf_port = config.zeroconf_port.unwrap_or(0); - let credentials_provider = - if let Some(credentials) = cache.as_ref().and_then(|c| c.credentials()) { - CredentialsProvider::SpotifyCredentials(credentials) - } else if config.discovery { - info!("no usable credentials found, enabling discovery"); - debug!("Using device id '{}'", session_config.device_id); - const RETRY_MAX: u8 = 4; - let mut retry_counter = 0; - let mut backoff = Duration::from_secs(5); - let discovery_stream = loop { - match librespot_discovery::Discovery::builder( - session_config.device_id.clone(), - session_config.client_id.clone(), - ) - .name(config.device_name.clone()) - .device_type(config.device_type) - .port(zeroconf_port) - .launch() - { - Ok(discovery_stream) => break discovery_stream, - Err(err) => { - error!("failed to enable discovery: {err}"); - if retry_counter >= RETRY_MAX { - return Err(err).with_context(|| { - "failed to enable discovery (and no credentials provided)" - }); - } - info!("retrying discovery in {} seconds", backoff.as_secs()); - thread::sleep(backoff); - retry_counter += 1; - backoff *= 2; - info!("trying to enable discovery (retry {retry_counter}/{RETRY_MAX})"); + let creds = if let Some(creds) = config.oauth_cache.as_ref().and_then(|c| c.credentials()) { + info!( + "Login via OAuth as user {}.", + creds.username.as_deref().unwrap_or("unknown") + ); + Some(creds) + } else if let Some(creds) = config.cache.as_ref().and_then(|c| c.credentials()) { + info!( + "Restoring previous login as user {}.", + creds.username.as_deref().unwrap_or("unknown") + ); + Some(creds) + } else { + None + }; + + let discovery = if config.discovery { + info!("Starting zeroconf server to advertise on local network."); + debug!("Using device id '{}'", session_config.device_id); + const RETRY_MAX: u8 = 4; + let mut retry_counter = 0; + let mut backoff = Duration::from_secs(5); + loop { + match librespot_discovery::Discovery::builder( + session_config.device_id.clone(), + session_config.client_id.clone(), + ) + .name(config.device_name.clone()) + .device_type(config.device_type) + .port(zeroconf_port) + .launch() + { + Ok(discovery_stream) => break Some(discovery_stream), + Err(err) => { + error!("failed to enable discovery: {err}"); + if retry_counter >= RETRY_MAX { + error!("maximum amount of retries exceeded"); + break None; } + info!("retrying discovery in {} seconds", backoff.as_secs()); + thread::sleep(backoff); + retry_counter += 1; + backoff *= 2; + info!("trying to enable discovery (retry {retry_counter}/{RETRY_MAX})"); } - }; - discovery_stream.into() - } else { - return Err(eyre!( - "no cached credentials available and discovery disabled" - )) - .with_suggestion(|| "consider enabling discovery or authenticating via OAuth"); - }; + } + } + } else { + None + }; + + let credentials_provider = match (discovery, creds) { + (Some(stream), creds) => CredentialsProvider::Discovery { + stream: stream.peekable(), + last_credentials: creds, + }, + (None, Some(creds)) => CredentialsProvider::CredentialsOnly(creds), + (None, None) => { + return Err( + eyre!("Discovery unavailable and no credentials found.").with_suggestion(|| { + "Try enabling discovery or logging in first with `spotifyd authenticate`." + }), + ); + } + }; let backend = audio_backend::find(backend).expect("available backends should match ours"); @@ -108,7 +128,7 @@ pub(crate) fn initial_state( credentials_provider, mixer, session_config, - cache, + cache: config.cache, audio_device: config.audio_device, audio_format: config.audio_format, player_config, From b0ed8084c5a6bdcf369d2c2ee736e0fdfa45506d Mon Sep 17 00:00:00 2001 From: eladyn Date: Mon, 30 Dec 2024 02:43:26 +0100 Subject: [PATCH 4/6] chore: properly remove any references to `dbus_keyring` feature --- .github/ISSUE_TEMPLATE/bug-report.md | 1 - .github/workflows/cd.yml | 4 ++-- .github/workflows/ci.yml | 4 ++-- Cargo.toml | 2 +- hooks/pre-commit | 8 ++++---- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md index c5c9d79d..6d967fdf 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.md +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -33,7 +33,6 @@ assignees: '' **Compilation flags** - [ ] dbus_mpris -- [ ] dbus_keyring - [x] alsa_backend - [ ] portaudio_backend - [ ] pulseaudio_backend diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 787cce72..a87b7d6d 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -18,9 +18,9 @@ jobs: - artifact_type: 'slim' # Slim version has no features enabled by default. feature: '' - artifact_type: 'default' - feature: 'dbus_keyring,dbus_mpris' # Default version has all extra features enabled + feature: 'dbus_mpris' # Default version has all extra features enabled - artifact_type: 'full' - feature: 'dbus_keyring,dbus_mpris' # Full version has all extra features and audio backends enabled + feature: 'dbus_mpris' # Full version has all extra features and audio backends enabled - build_target: macos os: macos-latest artifact_prefix: macos diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d551d6e8..4b089568 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,9 +59,9 @@ jobs: os: [macos-latest, ubuntu-latest] include: - os: macos-latest - features: portaudio_backend,rodio_backend,dbus_keyring + features: portaudio_backend,rodio_backend - os: ubuntu-latest - features: alsa_backend,rodio_backend,dbus_keyring,dbus_mpris + features: alsa_backend,rodio_backend,dbus_mpris steps: - name: Installing Rust toolchain diff --git a/Cargo.toml b/Cargo.toml index 360bb895..142d0704 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,7 @@ rodiojack_backend = ["librespot-playback/rodiojack-backend"] [package.metadata.deb] depends = "$auto, systemd, pulseaudio" -features = ["pulseaudio_backend", "dbus_keyring", "dbus_mpris"] +features = ["pulseaudio_backend", "dbus_mpris"] assets = [ ["target/release/spotifyd", "usr/bin/", "755"], ["README.md", "usr/share/doc/spotifyd/README", "644"], diff --git a/hooks/pre-commit b/hooks/pre-commit index c07342f9..e43d760e 100755 --- a/hooks/pre-commit +++ b/hooks/pre-commit @@ -34,16 +34,16 @@ for path in $(git diff --name-only --cached); do done echo "→ Building pre-commit build artifacts..." -cargo check --quiet --no-default-features --features "rodio_backend,dbus_keyring" +cargo check --quiet --no-default-features --features "rodio_backend" if [ $? -ne 0 ]; then exit 1 fi -cargo build --quiet --no-default-features --features "rodio_backend,dbus_keyring" +cargo build --quiet --no-default-features --features "rodio_backend" # Linting is only done with the rodio backend and the keyring feature as those should be # compilable for every supported platform without external library needs. echo "→ Linting Rust code..." -cargo clippy --no-default-features --features "rodio_backend,dbus_keyring" -- -D warnings +cargo clippy --no-default-features --features "rodio_backend" -- -D warnings echo "→ Testing Rust code..." -cargo test --no-default-features --features "rodio_backend,dbus_keyring" +cargo test --no-default-features --features "rodio_backend" From 062027cf776e91f187c57ff7acd5a8b1851df365 Mon Sep 17 00:00:00 2001 From: eladyn Date: Sun, 19 Jan 2025 02:26:55 +0100 Subject: [PATCH 5/6] config: properly handle missing initial_volume --- src/config.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index 83d6550a..ba0881f0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -177,9 +177,7 @@ fn number_or_string<'de, D>(de: D) -> Result, D::Error> where D: Deserializer<'de>, { - let Some(val) = Option::::deserialize(de)? else { - return Ok(None); - }; + let val = toml::Value::deserialize(de)?; let unexpected = match val { toml::Value::Integer(num) => { @@ -292,7 +290,7 @@ pub struct SharedConfigValues { /// Initial volume between 0 and 100 #[arg(long)] - #[serde(deserialize_with = "number_or_string")] + #[serde(deserialize_with = "number_or_string", default)] initial_volume: Option, /// Enable to normalize the volume during playback From d8ccb993e0f90911c246d454769d1f7b89ea30b6 Mon Sep 17 00:00:00 2001 From: eladyn Date: Mon, 24 Feb 2025 00:50:04 +0100 Subject: [PATCH 6/6] config: validate backend from config file --- src/config.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/config.rs b/src/config.rs index ba0881f0..7a31a097 100644 --- a/src/config.rs +++ b/src/config.rs @@ -173,6 +173,26 @@ fn possible_backends() -> Vec<&'static str> { audio_backend::BACKENDS.iter().map(|b| b.0).collect() } +fn deserialize_backend<'de, D>(de: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let backend = String::deserialize(de)?; + let possible_backends = possible_backends(); + if possible_backends.contains(&backend.as_str()) { + Ok(Some(backend)) + } else { + Err(de::Error::invalid_value( + Unexpected::Str(&backend), + &format!( + "a valid backend (available: {})", + possible_backends.join(", ") + ) + .as_str(), + )) + } +} + fn number_or_string<'de, D>(de: D) -> Result, D::Error> where D: Deserializer<'de>, @@ -265,6 +285,7 @@ pub struct SharedConfigValues { /// The audio backend to use #[arg(long, short, value_parser = possible_backends())] + #[serde(deserialize_with = "deserialize_backend")] backend: Option, /// The volume controller to use