From 1d15aa1505a934548e99425b03b676cebe17db5e Mon Sep 17 00:00:00 2001 From: Michael Herstine Date: Sun, 9 Jul 2023 15:17:51 -0700 Subject: [PATCH] Update the Continuous Integration job & upgrade Clap to v4. I suppose I conflated two changes in one commit. This patch: - updated the CI job & got it working again - upgraded to Clap v4 - updates README & NEWS accordingly Getting Clap v4. to *compile* was one thing-- the upgrade also required a broad revision of the use of the Clap API (substituting `get_flag()` for `contains_id()`, for instance. --- .github/workflows/ci.yml | 115 +++++++++++++++-------- NEWS | 6 +- README.org | 7 +- configure.ac | 2 +- mpdpopm/Cargo.toml.in | 9 +- mpdpopm/Makefile.am | 2 +- mpdpopm/src/bin/mppopm.rs | 181 ++++++++++++++++++++----------------- mpdpopm/src/bin/mppopmd.rs | 53 +++++++---- mpdpopm/src/filters_ast.rs | 44 +++++++-- mpdpopm/src/lib.rs | 116 ++++++++++++------------ 10 files changed, 319 insertions(+), 216 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c126837..b186519 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,15 +1,17 @@ -# Setting up a nightly build & test job for `mpdpopm'. Much thanks to -# BurntShushi from whom I shamelessly copied a lot of this -# -name: ci +# This is the continuous integration job for `mpdpopm`. It will run on +# both MacOS & Ubuntu, using various versions of Rust: a "pinned" +# version representing the least-supported, the current stable & the +# current nightly. +name: Continuous Integration + on: + workflow_dispatch: pull_request: + types: [opened, edited, reopened] # don't say `synchronize`-- that is taken care of by `push` push: - branches: - - master - - filters schedule: - cron: '00 01 * * *' + jobs: build: name: build @@ -19,70 +21,113 @@ jobs: - pinned - stable - nightly - # TODO(sp1ff): add more here - os: [ubuntu-18.04, macos-10.15] + os: [ubuntu-22.04, macos-12] include: - rust-build: pinned - os: ubuntu-18.04 - rust: 1.56.1 + os: ubuntu-22.04 + # 1.56.0 was the first to support 2021 edition + # 1.58 -- required by cargo-deb... + # but it seems a cargo-deb dependency (rayon-core) needs 1.59, + # whereas another (`log1) requires 1.60, and the `cargo-deb` + # README says 1.63. Dependency `toml_edit` needs 1.64! + rust: 1.64 - rust-build: stable - os: ubuntu-18.04 + os: ubuntu-22.04 rust: stable - rust-build: nightly - os: ubuntu-18.04 + os: ubuntu-22.04 rust: nightly - rust-build: pinned - os: macos-10.15 - rust: 1.56.1 + os: macos-12 + rust: 1.64 - rust-build: stable - os: macos-10.15 + os: macos-12 rust: stable - rust-build: nightly - os: macos-10.15 + os: macos-12 rust: nightly runs-on: ${{ matrix.os }} env: RUST_BACKTRACE: 1 + steps: + - name: Checkout repo - uses: actions/checkout@v2 + uses: actions/checkout@v3 + + # This is cheap, so do it early. I'd hate to install Rust, Eamcs, + # Tex &c only to find-out I'd forgotten a code TODO. + - name: Check for TODO-s + shell: bash + run: | + set -x + # `ripgrep` needs rust 1.70 to compile, but I stubbornly refuse + # to upgrade my "pinned" rust version! + # if rg -t rust 'TODO|TOOD|LATER|\\todo|todo!|dbg!'; then + if find . -iname '*.rs' -print0|xargs -0 grep -E 'TODO|TOOD|LATER|\\todo|todo!|dbg!'; then + echo "You have TODO-s" + exit 1 + fi + + - name: Install Rust + uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ matrix.rust }} - name: Install Tools (Ubuntu) - if: matrix.os == 'ubuntu-18.04' + if: matrix.os == 'ubuntu-22.04' shell: bash run: | pwd set -x + set -e sudo apt-get update sudo apt-get install -y autoconf automake emacs liblzma-dev texlive - name: Install Tools (macOS) - if: matrix.os == 'macos-10.15' + if: matrix.os == 'macos-12' shell: bash run: | - pwd set -x + # This seems wrong on multiple levels, but see here: + # + set +e + pwd + brew cleanup + # Will exit with non-zero status if it finds problems, but can be handy + # for trouble-shooting: + brew doctor brew update brew upgrade + set -e brew install autoconf automake emacs brew install --cask basictex - - name: Install Rust - uses: actions-rs/toolchain@v1 - with: - toolchain: ${{ matrix.rust }} - profile: minimal - override: true + - name: Install a modern version of Texinfo + if: matrix.os == 'macos-12' + shell: bash + run: | + set -x + mkdir tmp && cd tmp + # TODO(sp1ff): cache this + curl -L -O https://ftp.gnu.org/gnu/texinfo/texinfo-7.0.2.tar.gz + tar xf texinfo-7.0.2.tar.gz + cd texinfo-7.0.2 + ./configure + make + make install + type -p texi2dvi + texi2dvi --version - name: Install additional Rust tooling shell: bash run: | - cargo install cargo-deb + cargo install --verbose cargo-deb - name: Configure mpdpopm shell: bash run: | - set -x + set -ex ./bootstrap && ./configure - name: Build mpdpopm @@ -108,28 +153,24 @@ jobs: shell: bash run: | set -x - cd mpdpopm - pwd - cargo test --verbose + make check - name: Check the Autotools distribution (Ubuntu) - if: matrix.os == 'ubuntu-18.04' + if: matrix.os == 'ubuntu-22.04' shell: bash run: make distcheck - name: Check the Autotools distribution (MacOS) - if: matrix.os == 'macos-10.15' + if: matrix.os == 'macos-12' shell: bash run: | eval "$(/usr/libexec/path_helper)" make distcheck - name: Check the Debian package - if: matrix.os == 'ubuntu-18.04' + if: matrix.os == 'ubuntu-22.04' shell: bash run: | set -x cd mpdpopm cargo deb - - diff --git a/NEWS b/NEWS index 54dce66..b585420 100644 --- a/NEWS +++ b/NEWS @@ -4,7 +4,11 @@ mpdpopm News -- history of user-visible changes -*- outline -*- ** 0.3.2 build -No user-visible changes; strictly a hygiene build (updated to Tokio 1.0). +No user-visible changes; strictly a hygiene build: + + - updated to Tokio 1.0 + - updated to Clap 4 + - updated the Rust edition to 2021 ** 0.3.1 build No user-visible changes; strictly a hygiene build (re-vamped error-handling throughout). diff --git a/README.org b/README.org index 22bf8dd..dc901c4 100644 --- a/README.org +++ b/README.org @@ -2,11 +2,11 @@ #+AUTHOR: Michael Herstine #+DESCRIPTION: mpdpopm #+EMAIL: sp1ff@pobox.com -#+DATE: <2021-12-30 Thu 07:49> +#+DATE: <2023-07-15 Sat 17:12> #+AUTODATE: t * Introduction - +1 [[https://github.com/sp1ff/mpdpopm][mpdpopm]] provides a companion daemon to [[https://www.musicpd.org/][MPD]] for maintaining play counts, ratings and last-played timestamps, along with an associated CLI for talking to the companion daemon. Similar to [[https://github.com/vincent-petithory/mpdfav][mpdfav]], but written in Rust (which I prefer to Go), it will maintain this information in your sticker database. Along the lines of [[https://alip.github.io/mpdcron][mpdcron]], it will also allow you to keep that information up-to-date in your tags by invoking external (user-provided & -configured) commands. This README focuses on obtaining & installing [[https://github.com/sp1ff/mpdpopm][mpdpopm]]; the user manual is distributed with the package in [[https://www.gnu.org/software/texinfo/][Texinfo]] format. The HTML version of the user manual is hosted on my personal [[https://www.unwoundstack.com/doc/mpdpopm/curr][site]]. @@ -24,7 +24,7 @@ mppopm findadd "(rating > 128)" to add all songs with a rating greater than 128 to the play queue, or #+BEGIN_SRC bash -mppopm findadd "(lastplayed <= \"2020-03-27\")" +mppopm findadd "(lastplayed <= \"2022-07-15\")" #+END_SRC to add all songs that haven't been played in the last year. @@ -179,6 +179,7 @@ You'll likely want to run the program in the foreground initially for ease of tr Once you've got the daemon running to your satisfaction, if you're on a systemd-based Linux distribution, have a look at the sample systemd unit file thanks to [[https://github.com/tanshoku][tanshoku]]. [[https://github.com/tanshoku][tanshoku]] was kind enough to contribute a systemd unit for this purpose. At present, the build does not install it, but provides it as an example and leaves it to the user to install should they desire (and after they have edited it to suit their configuration). You can find it in =${prefix}/share/mpdpopm/examples= for the Autotools distribution, =/usr/local/share/mpdpopm/examples= for the Debian package, and in the =doc= folder for the pre-built binaries. + *** mppopm At this point, [[https://github.com/sp1ff/mpdpopm][mpdpopm]] will happily monitor your playback history & keep play counts & last played timestamps for you. If you would like to rate tracks, however, you will need to somehow induce your favorite mpd client to send a "rating" message to the [[https://github.com/sp1ff/mpdpopm][mpdpopm]] commands channel ("unwoundstack.com:commands" by default). Since this is unlikely to be convenient, I wrote an mpd client for the purpose: a little CLI called =mppopm=. You can simply execute diff --git a/configure.ac b/configure.ac index fcbe5dd..db9b036 100644 --- a/configure.ac +++ b/configure.ac @@ -1,7 +1,7 @@ AC_INIT([mpdpopm], [0.3.2], [sp1ff@pobox.com], [mpdpopm], [https://github.com/sp1ff/mpdpopm]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_SRCDIR([mpdpopm/Cargo.toml.in]) -AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability -Wno-override gnits std-options dist-bzip2 dist-xz]) +AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability -Wno-override gnits std-options dist-xz dist-zstd]) AC_CHECK_PROG(CARGO, [cargo], [yes], [no]) AS_IF(test x$CARGO = xno, diff --git a/mpdpopm/Cargo.toml.in b/mpdpopm/Cargo.toml.in index 9690ace..4d1a77d 100644 --- a/mpdpopm/Cargo.toml.in +++ b/mpdpopm/Cargo.toml.in @@ -2,7 +2,7 @@ name = "mpdpopm" version = "@PACKAGE_VERSION@" authors = ["@PACKAGE_AUTHOR@"] -edition = "2018" +edition = "2021" license-file = "../LICENSE" description = "Maintain ratings & playcounts for your mpd server" homepage = "https://github.com/sp1ff/mpdpopm" @@ -13,18 +13,17 @@ categories = ["multimedia", "network-programming", "database"] exclude = ["Cargo.toml.in", "Cargo.toml.orig", "Makefile", "Makefile.in", "Makefile.am", "rusty-tags.emacs", "src/vars.rs.am.in", "src/vars.rs.am", "flapdoodle.rs", "*.log"] [build-dependencies] -lalrpop = { version = "0.19", features = ["lexer"] } +lalrpop = { version = "0.20", features = ["lexer"] } [dependencies] async-trait = "0.1.31" backtrace = "0.3.46" boolinator = "2.4.0" chrono = "0.4.19" -clap = "=3.0.0-beta.1" +clap = "4.3.9" errno = "0.2.6" -# futures = "0.3.5" futures = "0.3.19" -lalrpop-util = "0.19" +lalrpop-util = { version = "0.20", features = ["lexer"] } lazy_static = "1.4.0" libc = "0.2.74" log = "0.4.8" diff --git a/mpdpopm/Makefile.am b/mpdpopm/Makefile.am index d28d3e0..ac72afd 100644 --- a/mpdpopm/Makefile.am +++ b/mpdpopm/Makefile.am @@ -61,7 +61,7 @@ $(srcdir)/src/vars.rs: $(builddir)/src/flapdoodle.rs dist-hook: cp -v Cargo.toml $(distdir)/mpdpopm cp -v src/flapdoodle.rs $(distdir)/src/vars.rs - cp -v mppopmd.{conf,service} $(disdir) + cp -v mppopmd.{conf,service} $(distdir) mppopmd$(EXEEXT): $(mppopmd_SOURCES) $(srcdir)/Cargo.toml $(srcdir)/src/vars.rs cd $(top_srcdir)/mpdpopm && \ diff --git a/mpdpopm/src/bin/mppopm.rs b/mpdpopm/src/bin/mppopm.rs index ace52d3..c40cb03 100644 --- a/mpdpopm/src/bin/mppopm.rs +++ b/mpdpopm/src/bin/mppopm.rs @@ -33,7 +33,8 @@ use mpdpopm::{ }; use backtrace::Backtrace; -use clap::{App, Arg}; +use clap::{value_parser, Arg, ArgAction, Command}; +use lazy_static::lazy_static; use log::{debug, info, trace, LevelFilter}; use log4rs::{ append::console::{ConsoleAppender, Target}, @@ -91,7 +92,7 @@ pub enum Error { }, } -impl std::fmt::Display for Error { +impl fmt::Display for Error { #[allow(unreachable_patterns)] // the _ arm is *currently* unreachable fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { @@ -115,6 +116,7 @@ impl std::fmt::Display for Error { } } } + impl fmt::Debug for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self) @@ -168,16 +170,16 @@ impl Default for Config { /// /// Several sub-commands take zero or more positional arguments meant to name tracks, with the /// convention that zero indicates that the sub-command should use the currently playing track. -/// This is a convenience function for mapping the value returned by [`values_of`] to a +/// This is a convenience function for mapping the value returned by [`get_many`] to a /// convenient representation of the user's intentions. /// -/// [`values_of`]: [`clap::ArgMatches::values_of`] -async fn map_tracks<'a, Iter: Iterator>( +/// [`get_many`]: [`clap::ArgMatches::get_many`] +async fn map_tracks<'a, Iter: Iterator>( client: &mut Client, args: Option, ) -> Result> { let files = match args { - Some(iter) => iter.map(|x| x.to_string()).collect(), + Some(iter) => iter.map(|x| x.clone()).collect(), None => { let file = match client.status().await.map_err(|err| Error::Client { source: err, @@ -206,7 +208,7 @@ async fn map_tracks<'a, Iter: Iterator>( //////////////////////////////////////////////////////////////////////////////////////////////////// /// Retrieve ratings for one or more tracks -async fn get_ratings<'a, Iter: Iterator>( +async fn get_ratings<'a, Iter: Iterator>( client: &mut Client, sticker: &str, tracks: Option, @@ -262,7 +264,7 @@ async fn set_rating( } /// Retrieve the playcount for one or more tracks -async fn get_play_counts<'a, Iter: Iterator>( +async fn get_play_counts<'a, Iter: Iterator>( client: &mut Client, sticker: &str, tracks: Option, @@ -324,7 +326,7 @@ async fn set_play_counts( } /// Retrieve the last played time for one or more tracks -async fn get_last_playeds<'a, Iter: Iterator>( +async fn get_last_playeds<'a, Iter: Iterator>( client: &mut Client, sticker: &str, tracks: Option, @@ -450,9 +452,9 @@ where //////////////////////////////////////////////////////////////////////////////////////////////////// -fn add_general_subcommands(app: App) -> App { +fn add_general_subcommands(app: Command) -> Command { app.subcommand( - App::new("get-rating") + Command::new("get-rating") .about("retrieve the rating for one or more tracks") .long_about( " @@ -465,15 +467,17 @@ Ratings are expressed as an integer between 0 & 255, inclusive, with the convention that 0 denotes \"un-rated\".", ) .arg( - Arg::with_name("with-uri") + Arg::new("with-uri") .short('u') .long("with-uri") - .about("Always show the song URI, even when there is only one track"), + .help("Always show the song URI, even when there is only one track") + .num_args(0) + .action(ArgAction::SetTrue), ) - .arg(Arg::with_name("track").multiple(true)), + .arg(Arg::new("track").num_args(0..)), ) .subcommand( - App::new("set-rating") + Command::new("set-rating") .about("set the rating for one track") .long_about( " @@ -490,11 +494,11 @@ per the Winamp convention: ***** 255 ", ) - .arg(Arg::with_name("rating").index(1).required(true)) - .arg(Arg::with_name("track").index(2)), + .arg(Arg::new("rating").index(1).required(true)) + .arg(Arg::new("track").index(2)), ) .subcommand( - App::new("get-pc") + Command::new("get-pc") .about("retrieve the play count for one or more tracks") .long_about( " @@ -504,26 +508,28 @@ on stdout. With multiple arguments, print their play counts on stdout, one per line, prefixed by the track name.", ) .arg( - Arg::with_name("with-uri") + Arg::new("with-uri") .short('u') .long("with-uri") - .about("Always show the song URI, even when there is only one"), + .help("Always show the song URI, even when there is only one") + .num_args(0) + .action(ArgAction::SetTrue), ) - .arg(Arg::with_name("track").multiple(true)), + .arg(Arg::new("track").num_args(0..)), ) .subcommand( - App::new("set-pc") + Command::new("set-pc") .about("set the play count for one track") .long_about( " With one argument, set the play count of the current song to that argument. With a second argument, set the play count for that song to the first.", ) - .arg(Arg::with_name("play-count").index(1).required(true)) - .arg(Arg::with_name("track").index(2)), + .arg(Arg::new("play-count").index(1).required(true)) + .arg(Arg::new("track").index(2)), ) .subcommand( - App::new("get-lp") + Command::new("get-lp") .about("retrieve the last played timestamp for one or more tracks") .long_about( " @@ -536,15 +542,17 @@ name. The last played timestamp is expressed in seconds since Unix epoch.", ) .arg( - Arg::with_name("with-uri") + Arg::new("with-uri") .short('u') .long("with-uri") - .about("Always show the song URI, even when there is only one"), + .help("Always show the song URI, even when there is only one") + .num_args(0) + .action(ArgAction::SetTrue), ) - .arg(Arg::with_name("track").multiple(true)), + .arg(Arg::new("track").num_args(0..)), ) .subcommand( - App::new("set-lp") + Command::new("set-lp") .about("set the last played timestamp for one track") .long_about( " @@ -552,12 +560,12 @@ With one argument, set the last played time of the current song. With two arguments, set the last played time for the second argument to the first. The last played timestamp is expressed in seconds since Unix epoch.", ) - .arg(Arg::with_name("last-played").index(1).required(true)) - .arg(Arg::with_name("track").index(2)), + .arg(Arg::new("last-played").index(1).required(true)) + .arg(Arg::new("track").index(2)), ) - .subcommand(App::new("get-playlists").about("retreive the list of stored playlists")) + .subcommand(Command::new("get-playlists").about("retreive the list of stored playlists")) .subcommand( - App::new("findadd") + Command::new("findadd") .about("search case-sensitively for songs matching matching a filter and add them to the queue") .long_about( " @@ -585,10 +593,10 @@ will add all songs whose artist tag matches the regexp \"pogues\" with a rating `findadd' is case-sensitive; for case-insensitive searching see the `searchadd' command. ", ) - .arg(Arg::with_name("filter").index(1).required(true)), + .arg(Arg::new("filter").index(1).required(true)), ) .subcommand( - App::new("searchadd") + Command::new("searchadd") .about("search case-insensitively for songs matching matching a filter and add them to the queue") .long_about( " @@ -616,10 +624,13 @@ will add all songs whose artist tag matches the regexp \"pogues\" with a rating `searchadd' is case-insensitive; for case-sensitive searching see the `findadd' command. ", ) - .arg(Arg::with_name("filter").index(1).required(true)), + .arg(Arg::new("filter").index(1).required(true)), ) } +lazy_static! { + static ref DEF_CFG: String = format!("{}/.mppopm", std::env::var("HOME").unwrap()); +} //////////////////////////////////////////////////////////////////////////////////////////////////// // The Big Kahuna // //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -628,48 +639,54 @@ will add all songs whose artist tag matches the regexp \"pogues\" with a rating async fn main() -> Result<()> { use mpdpopm::vars::{AUTHOR, VERSION}; - let def_cfg = format!("{}/.mppopm", std::env::var("HOME").unwrap()); - let mut app = App::new("mppopm") + let mut app = Command::new("mppopm") .version(VERSION) .author(AUTHOR) .about("`mppopmd' client") .arg( - Arg::with_name("verbose") + Arg::new("verbose") .short('v') .long("verbose") - .about("enable verbose logging"), + .num_args(0) + .action(ArgAction::SetTrue) + .help("enable verbose logging"), ) .arg( - Arg::with_name("debug") + Arg::new("debug") .short('d') .long("debug") - .about("enable debug logging (implies --verbose)"), + .num_args(0) + .action(ArgAction::SetTrue) + .help("enable debug logging (implies --verbose)"), ) .arg( - Arg::with_name("config") + Arg::new("config") .short('c') - .takes_value(true) + .long("config") + .value_parser(value_parser!(PathBuf)) .value_name("FILE") - .default_value(&def_cfg) - .about("path to configuration file"), + .default_value(DEF_CFG.as_str()) + .help("path to configuration file"), ) .arg( - Arg::with_name("host") + Arg::new("host") .short('H') - .takes_value(true) + .long("host") + .value_parser(value_parser!(String)) .value_name("HOST") - .about("MPD host"), + .help("MPD host"), ) .arg( - Arg::with_name("port") + Arg::new("port") .short('p') - .takes_value(true) + .long("port") + .value_parser(value_parser!(u16)) .value_name("PORT") - .about("MPD port"), + .help("MPD port"), ); app = add_general_subcommands(app); - app = app.arg(Arg::with_name("args").multiple(true)); + app = app.arg(Arg::new("args").num_args(0..)); let matches = app.get_matches(); @@ -678,7 +695,7 @@ async fn main() -> Result<()> { // explicitly named a configuration file, and it's not there, they presumably want to know // about that. let cfgpth = matches - .value_of("config") + .get_one::("config") .ok_or_else(|| Error::NoConfigArg {})?; let mut cfg = match std::fs::read_to_string(cfgpth) { // The config file (defaulted or not) existed & we were able to read its contents-- parse @@ -689,8 +706,8 @@ async fn main() -> Result<()> { })?, // The config file (defaulted or not) either didn't exist, or we were unable to read its // contents... - Err(err) => match (err.kind(), matches.occurrences_of("config")) { - (std::io::ErrorKind::NotFound, 0) => { + Err(err) => match (err.kind(), matches.value_source("config").unwrap()) { + (std::io::ErrorKind::NotFound, clap::parser::ValueSource::DefaultValue) => { // The user just accepted the default option value & that default didn't exist; we // proceed with default configuration settings. Config::default() @@ -713,7 +730,7 @@ async fn main() -> Result<()> { // 2. configuration // 3. environment variable - match matches.value_of("host") { + match matches.get_one::("host") { Some(host) => { cfg.host = String::from(host); } @@ -727,13 +744,8 @@ async fn main() -> Result<()> { } } - match matches.value_of("port") { - Some(port) => { - cfg.port = port.parse::().map_err(|err| Error::ExpectedInt { - source: err, - back: Backtrace::new(), - })?; - } + match matches.get_one::("port") { + Some(port) => cfg.port = *port, None => { if cfg.port == 0 { cfg.port = match std::env::var("MPD_PORT") { @@ -748,7 +760,7 @@ async fn main() -> Result<()> { } // Handle log verbosity: debug => verbose - let lf = match (matches.is_present("verbose"), matches.is_present("debug")) { + let lf = match (matches.get_flag("verbose"), matches.get_flag("debug")) { (_, true) => LevelFilter::Trace, (true, false) => LevelFilter::Debug, _ => LevelFilter::Warn, @@ -781,60 +793,67 @@ async fn main() -> Result<()> { return Ok(get_ratings( &mut client, &cfg.rating_sticker, - subm.values_of("track"), - subm.is_present("with-uri"), + subm.get_many::("track"), + subm.get_flag("with-uri"), ) .await?); } else if let Some(subm) = matches.subcommand_matches("set-rating") { return Ok(set_rating( &mut client, &cfg.commands_chan, - subm.value_of("rating").ok_or_else(|| Error::NoRating {})?, - subm.value_of("track"), + subm.get_one::("rating") + .ok_or_else(|| Error::NoRating {})?, + subm.get_one::("track") + .as_deref() + .map(|x| x.as_str()), ) .await?); } else if let Some(subm) = matches.subcommand_matches("get-pc") { return Ok(get_play_counts( &mut client, &cfg.playcount_sticker, - subm.values_of("track"), - subm.is_present("with-uri"), + subm.get_many::("track"), + subm.get_flag("with-uri"), ) .await?); } else if let Some(subm) = matches.subcommand_matches("set-pc") { return Ok(set_play_counts( &mut client, &cfg.commands_chan, - subm.value_of("play-count") + subm.get_one::("play-count") .ok_or_else(|| Error::NoPlayCount {})? .parse::() .map_err(|err| Error::ExpectedInt { source: err, back: Backtrace::new(), })?, - subm.value_of("track"), + subm.get_one::("track") + .as_deref() + .map(|x| x.as_str()), ) .await?); } else if let Some(subm) = matches.subcommand_matches("get-lp") { return Ok(get_last_playeds( &mut client, &cfg.lastplayed_sticker, - subm.values_of("track"), - subm.is_present("with-uri"), + subm.get_many::("track"), + subm.get_flag("with-uri"), ) .await?); } else if let Some(subm) = matches.subcommand_matches("set-lp") { return Ok(set_last_playeds( &mut client, &cfg.commands_chan, - subm.value_of("last-played") + subm.get_one::("last-played") .ok_or_else(|| Error::NoLastPlayed {})? .parse::() .map_err(|err| Error::ExpectedInt { source: err, back: Backtrace::new(), })?, - subm.value_of("track"), + subm.get_one::("track") + .as_deref() + .map(|x| x.as_str()), ) .await?); } else if let Some(_subm) = matches.subcommand_matches("get-playlists") { @@ -844,7 +863,7 @@ async fn main() -> Result<()> { &mut client, &cfg.commands_chan, // filter is mandatory - subm.value_of("filter").unwrap(), + subm.get_one::("filter").unwrap(), true, ) .await?); @@ -853,12 +872,12 @@ async fn main() -> Result<()> { &mut client, &cfg.commands_chan, // filter is mandatory - subm.value_of("filter").unwrap(), + subm.get_one::("filter").unwrap(), false, ) .await?); - } else if let Some(args) = matches.values_of("args") { - return Ok(send_command(&mut client, &cfg.commands_chan, args).await?); + } else if let Some(args) = matches.get_many::("args") { + return Ok(send_command(&mut client, &cfg.commands_chan, args.map(|x| x.as_str())).await?); } Err(Error::NoSubCommand) diff --git a/mpdpopm/src/bin/mppopmd.rs b/mpdpopm/src/bin/mppopmd.rs index 98dead7..2d5730b 100644 --- a/mpdpopm/src/bin/mppopmd.rs +++ b/mpdpopm/src/bin/mppopmd.rs @@ -31,8 +31,9 @@ use mpdpopm::mpdpopm; use mpdpopm::vars::LOCALSTATEDIR; use backtrace::Backtrace; -use clap::{App, Arg}; +use clap::{value_parser, Arg, ArgAction, Command}; use errno::errno; +use lazy_static::lazy_static; use libc::{ close, dup, exit, fork, getdtablesize, getpid, lockf, open, setsid, umask, write, F_TLOCK, }; @@ -127,6 +128,10 @@ impl fmt::Debug for Error { type Result = std::result::Result<(), Error>; +lazy_static! { + static ref DEF_CONF: String = format!("{}/mppopmd.conf", mpdpopm::vars::SYSCONFDIR); +} + /// Make this process into a daemon /// /// I first tried to use the [daemonize](https://docs.rs/daemonize) crate, without success. Perhaps @@ -273,9 +278,9 @@ fn daemonize() -> Result { /// Instead, stay synchronous until we've daemonized (or figured out that we don't need to), and /// only then fire-up the tokio runtime. fn main() -> Result { - use mpdpopm::vars::{AUTHOR, SYSCONFDIR, VERSION}; + use mpdpopm::vars::{AUTHOR, VERSION}; - let matches = App::new("mppopmd") + let matches = Command::new("mppopmd") .version(VERSION) .author(AUTHOR) .about("mpd + POPM") @@ -287,27 +292,37 @@ database, but it allows you to keep that information in your tags, as well, by i commands to keep your tags up-to-date.", ) .arg( - Arg::with_name("no-daemon") + Arg::new("no-daemon") .short('F') - .about("do not daemonize; remain in foreground"), + .long("no-daemon") + .num_args(0) + .action(ArgAction::SetTrue) + .help("do not daemonize; remain in foreground"), ) .arg( - Arg::with_name("config") + Arg::new("config") .short('c') - .takes_value(true) - .value_name("FILE") - .default_value(&format!("{}/mppopmd.conf", SYSCONFDIR)) - .about("path to configuration file"), + .long("config") + .num_args(1) + .value_parser(value_parser!(PathBuf)) + .default_value(DEF_CONF.as_str()) + .help("path to configuration file"), ) .arg( - Arg::with_name("verbose") + Arg::new("verbose") .short('v') - .about("enable verbose logging"), + .long("verbose") + .num_args(0) + .action(ArgAction::SetTrue) + .help("enable verbose logging"), ) .arg( - Arg::with_name("debug") + Arg::new("debug") .short('d') - .about("enable debug logging (implies --verbose)"), + .long("debug") + .num_args(0) + .action(ArgAction::SetTrue) + .help("enable debug logging (implies --verbose)"), ) .get_matches(); @@ -316,7 +331,7 @@ commands to keep your tags up-to-date.", // they explicitly named a configuration file, and it's not there, they presumably want to know // about that. let cfgpth = matches - .value_of("config") + .get_one::("config") .ok_or_else(|| Error::NoConfigArg {})?; let cfg = match std::fs::read_to_string(cfgpth) { // The config file (defaulted or not) existed & we were able to read its contents-- parse @@ -327,8 +342,8 @@ commands to keep your tags up-to-date.", })?, // The config file (defaulted or not) either didn't exist, or we were unable to read its // contents... - Err(err) => match (err.kind(), matches.occurrences_of("config")) { - (std::io::ErrorKind::NotFound, 0) => { + Err(err) => match (err.kind(), matches.value_source("config").unwrap()) { + (std::io::ErrorKind::NotFound, clap::parser::ValueSource::DefaultValue) => { // The user just accepted the default option value & that default didn't exist; we // proceed with default configuration settings. Config::default() @@ -347,14 +362,14 @@ commands to keep your tags up-to-date.", // `--verbose' & `--debug' work as follows: if `--debug' is present, log at level Trace, no // matter what. Else, if `--verbose' is present, log at level Debug. Else, log at level Info. - let lf = match (matches.is_present("verbose"), matches.is_present("debug")) { + let lf = match (matches.get_flag("verbose"), matches.get_flag("debug")) { (_, true) => LevelFilter::Trace, (true, false) => LevelFilter::Debug, _ => LevelFilter::Info, }; // If we're not running as a daemon... - if matches.is_present("no-daemon") { + if matches.get_flag("no-daemon") { // log to stdout & let our caller redirect that. let app = ConsoleAppender::builder() .target(Target::Stdout) diff --git a/mpdpopm/src/filters_ast.rs b/mpdpopm/src/filters_ast.rs index 2b433fc..72a207e 100644 --- a/mpdpopm/src/filters_ast.rs +++ b/mpdpopm/src/filters_ast.rs @@ -539,8 +539,12 @@ pub fn parse_iso_8601(mut buf: &mut &[u8]) -> Result { if buf.len() != 0 { if peek(buf) == Some('Z') { return Ok(Utc - .ymd(year, month, day) - .and_hms(hour, minute, second) + .with_ymd_and_hms(year, month, day, hour, minute, second) + .single() + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? .timestamp()); } else { let next = peek(buf); @@ -564,22 +568,42 @@ pub fn parse_iso_8601(mut buf: &mut &[u8]) -> Result { } if west { - return Ok(FixedOffset::west(hours * 3600 + minutes * 60) - .ymd(year, month, day) - .and_hms(hour, minute, second) + return Ok(FixedOffset::west_opt(hours * 3600 + minutes * 60) + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? + .with_ymd_and_hms(year, month, day, hour, minute, second) + .single() + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? .timestamp()); } else { - return Ok(FixedOffset::east(hours * 3600 + minutes * 60) - .ymd(year, month, day) - .and_hms(hour, minute, second) + return Ok(FixedOffset::east_opt(hours * 3600 + minutes * 60) + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? + .with_ymd_and_hms(year, month, day, hour, minute, second) + .single() + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? .timestamp()); } } } } Ok(Local - .ymd(year, month, day) - .and_hms(hour, minute, second) + .with_ymd_and_hms(year, month, day, hour, minute, second) + .single() + .ok_or(Error::BadISO8601String { + text: buf.to_vec(), + back: Backtrace::new(), + })? .timestamp()) } diff --git a/mpdpopm/src/lib.rs b/mpdpopm/src/lib.rs index 63aec25..bb10a89 100644 --- a/mpdpopm/src/lib.rs +++ b/mpdpopm/src/lib.rs @@ -229,23 +229,54 @@ pub async fn mpdpopm(cfg: Config) -> std::result::Result<(), Error> { let mut idle = Box::pin(idle_client.idle().fuse()); loop { select! { - _ = ctrl_c => { - info!("got ctrl-C"); - done = true; - break; - }, - _ = sighup => { - info!("got SIGHUP"); - done = true; - break; - }, - _ = sigkill => { - info!("got SIGKILL"); - done = true; - break; - }, - _ = tick => { - tick.set(sleep(Duration::from_millis(cfg.poll_interval_ms)).fuse()); + _ = ctrl_c => { + info!("got ctrl-C"); + done = true; + break; + }, + _ = sighup => { + info!("got SIGHUP"); + done = true; + break; + }, + _ = sigkill => { + info!("got SIGKILL"); + done = true; + break; + }, + _ = tick => { + tick.set(sleep(Duration::from_millis(cfg.poll_interval_ms)).fuse()); + if let Some(fut) = state.update(&mut client, + &cfg.playcount_command, + &mut cfg.playcount_command_args + .iter() + .cloned(), + music_dir).await.map_err(|err| Error::Playcounts{source: err, back: Backtrace::new()})? { + cmds.push(fut); + } + }, + next = cmds.next() => match next { + Some(out) => { + debug!("output status is {:#?}", out.out); + match out.upd { + Some(uri) => { + debug!("{} needs to be updated", uri); + client.update(&uri).await.map_err(|err| Error::Client { + source: err, + back: Backtrace::new(), + })?; + }, + None => debug!("No database update needed"), + } + }, + None => { + debug!("No more commands to process."); + } + }, + res = idle => match res { + Ok(subsys) => { + debug!("subsystem {} changed", subsys); + if subsys == IdleSubSystem::Player { if let Some(fut) = state.update(&mut client, &cfg.playcount_command, &mut cfg.playcount_command_args @@ -254,49 +285,18 @@ pub async fn mpdpopm(cfg: Config) -> std::result::Result<(), Error> { music_dir).await.map_err(|err| Error::Playcounts{source: err, back: Backtrace::new()})? { cmds.push(fut); } - }, - next = cmds.next() => match next { - Some(out) => { - debug!("output status is {:#?}", out.out); - match out.upd { - Some(uri) => { - debug!("{} needs to be updated", uri); - client.update(&uri).await.map_err(|err| Error::Client { - source: err, - back: Backtrace::new(), - })?; - }, - None => debug!("No database update needed"), - } - }, - None => { - debug!("No more commands to process."); - } - }, - res = idle => match res { - Ok(subsys) => { - debug!("subsystem {} changed", subsys); - if subsys == IdleSubSystem::Player { - if let Some(fut) = state.update(&mut client, - &cfg.playcount_command, - &mut cfg.playcount_command_args - .iter() - .cloned(), - music_dir).await.map_err(|err| Error::Playcounts{source: err, back: Backtrace::new()})? { - cmds.push(fut); - } - } else if subsys == IdleSubSystem::Message { - msg_check_needed = true; - } - break; - }, - Err(err) => { - debug!("error {} on idle", err); - done = true; - break; - } + } else if subsys == IdleSubSystem::Message { + msg_check_needed = true; } + break; + }, + Err(err) => { + debug!("error {} on idle", err); + done = true; + break; } + } + } } } // `idle_client' mutable borrow dropped here, which is important...