diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index bc81c15..6081f60 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -103,6 +103,7 @@ body: label: Log description: | Run QuickPlay from a terminal with `RUST_LOG=debug` (PowerShell: `$env:RUST_LOG="debug"; .\waveflow-quickplay.exe file.flac`) and paste the lines around the bug. + This issue is public and the log prints full file paths, which can show your user name and your folder and file names: replace whatever you would rather keep private before posting. render: shell - type: textarea diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20aaff8..9817fab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Linux build dependencies diff --git a/.github/workflows/label-pr.yml b/.github/workflows/label-pr.yml index 4d2c048..a513418 100644 --- a/.github/workflows/label-pr.yml +++ b/.github/workflows/label-pr.yml @@ -1,16 +1,26 @@ # Labels every pull request: `scope:` from the touched paths # (.github/labeler.yml), `type:` from the Conventional Commits prefix of the # title, `size:` from the diff. Same scheme as the WaveFlow repository. +# +# `pull_request_target` runs in the base repository with a token that can +# write labels, so pull requests from forks are labelled too. It is safe here +# because nothing from the pull request is checked out or run: the labeler +# reads its rules from the base branch and the scripts only call the API. name: Label PR on: - pull_request: + pull_request_target: types: [opened, synchronize, edited] permissions: contents: read pull-requests: write +# Title edits and pushes in quick succession: only the latest run labels. +concurrency: + group: label-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: label: runs-on: ubuntu-latest @@ -36,7 +46,13 @@ jobs: with: github-token: ${{ env.LABEL_TOKEN }} script: | - const title = context.payload.pull_request.title; + // The title as it is now, not as it was when this event fired. + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + }); + const title = pr.title; const prefixMap = { 'feat': 'type: feat', 'fix': 'type: fix', @@ -49,12 +65,13 @@ jobs: }; const match = title.match(/^(\w+)(\(.+\))?[!]?:/); const label = prefixMap[match?.[1]]; - const current = await github.rest.issues.listLabelsOnIssue({ + const current = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, + per_page: 100, }); - for (const l of current.data.filter(l => l.name.startsWith('type:'))) { + for (const l of current.filter(l => l.name.startsWith('type:'))) { if (l.name !== label) { await github.rest.issues.removeLabel({ owner: context.repo.owner, @@ -94,12 +111,13 @@ jobs: { max: 500, label: 'size: l' }, ]; const sizeLabel = sizeLabels.find(s => lines <= s.max)?.label || 'size: xl'; - const current = await github.rest.issues.listLabelsOnIssue({ + const current = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, + per_page: 100, }); - for (const l of current.data.filter(l => l.name.startsWith('size:'))) { + for (const l of current.filter(l => l.name.startsWith('size:'))) { if (l.name !== sizeLabel) { await github.rest.issues.removeLabel({ owner: context.repo.owner, diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cf9b4cc..c74e35c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -32,17 +32,38 @@ permissions: env: RELEASE_TAG: ${{ inputs.tag || github.ref_name }} + # The packaging tools, pinned: packaging/installer.nsi is a copy of + # cargo-packager 0.11.8's template and is refreshed when this moves. + CARGO_PACKAGER_VERSION: "0.11.8" + CARGO_GENERATE_RPM_VERSION: "0.21.1" + +# One run per tag at a time: a tag push and a manual rebuild of the same tag +# would otherwise replace each other's files and checksums on the release. +concurrency: + group: release-${{ inputs.tag || github.ref_name }} + cancel-in-progress: false jobs: version: name: Tag matches Cargo.toml runs-on: ubuntu-latest timeout-minutes: 5 + outputs: + sha: ${{ steps.tag.outputs.sha }} steps: + # The input must name a tag, not a branch that happens to be called + # v; every job then checks out the commit it points to now, + # so a tag moved during the run cannot mix two sources. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ env.RELEASE_TAG }} + ref: refs/tags/${{ env.RELEASE_TAG }} persist-credentials: false + - id: tag + run: | + sha=$(git rev-parse "refs/tags/$RELEASE_TAG^{commit}") + test "$sha" = "$(git rev-parse HEAD)" + echo "sha=$sha" >> "$GITHUB_OUTPUT" + echo "$RELEASE_TAG is $sha" - run: | version=$(grep -m1 '^version' Cargo.toml | cut -d'"' -f2) if [ "v$version" != "$RELEASE_TAG" ]; then @@ -60,15 +81,16 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ env.RELEASE_TAG }} + ref: ${{ needs.version.outputs.sha }} persist-credentials: false - run: rustup show active-toolchain - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - run: cargo test --locked - - run: cargo install cargo-packager --locked + # pwsh, the Windows runners' shell: environment variables are $env:. + - run: cargo install cargo-packager --version "$env:CARGO_PACKAGER_VERSION" --locked # Everything is compiled before the certificate enters the runner, so # no build script of any dependency runs while the key is importable; - # cargo-packager then finds the build up to date. + # cargo-packager has no build hook (Cargo.toml) and packages this. - run: cargo build --release --locked # Authenticode, with WaveFlow's certificate (the same two secrets as @@ -91,14 +113,19 @@ jobs: } Write-Host "::add-mask::$env:PFX_PASSWORD" $pfx = Join-Path $env:RUNNER_TEMP "signing.pfx" - [IO.File]::WriteAllBytes($pfx, [Convert]::FromBase64String($env:PFX_BASE64)) - $password = ConvertTo-SecureString $env:PFX_PASSWORD -AsPlainText -Force - $cert = Import-PfxCertificate -FilePath $pfx -CertStoreLocation Cert:\CurrentUser\My -Password $password - Remove-Item $pfx - if (-not $cert.HasPrivateKey) { Write-Error "the PFX has no private key"; exit 1 } + try { + [IO.File]::WriteAllBytes($pfx, [Convert]::FromBase64String($env:PFX_BASE64)) + $password = ConvertTo-SecureString $env:PFX_PASSWORD -AsPlainText -Force + $cert = Import-PfxCertificate -FilePath $pfx -CertStoreLocation Cert:\CurrentUser\My -Password $password + } finally { + Remove-Item $pfx -ErrorAction SilentlyContinue + } $thumbprint = $cert.Thumbprint - Write-Host "Signing with $($cert.Subject) ($thumbprint), valid until $($cert.NotAfter)" + # Recorded before any other check, so the cleanup step removes the + # certificate from the store whichever way this step fails. "SIGNING_THUMBPRINT=$thumbprint" | Out-File $env:GITHUB_ENV -Append -Encoding utf8 + if (-not $cert.HasPrivateKey) { Write-Error "the PFX has no private key"; exit 1 } + Write-Host "Signing with $($cert.Subject) ($thumbprint), valid until $($cert.NotAfter)" # The thumbprint is public; the private key stays in the store. @" @@ -117,8 +144,27 @@ jobs: $files = @(Get-ChildItem target/packager/*.exe) + @(Get-Item target/release/waveflow-quickplay.exe) foreach ($file in $files) { $sig = Get-AuthenticodeSignature $file.FullName - # A self-signed certificate reads "UnknownError" (untrusted root): - # what matters is that it is ours and timestamped. + # A self-signed certificate reads "UnknownError" (untrusted root) + # or "NotTrusted"; any other status, HashMismatch first, means + # the signature does not cover the file as it is. + if ($sig.Status.ToString() -notin @('Valid', 'UnknownError', 'NotTrusted')) { + Write-Error "$($file.Name) has a bad signature ($($sig.Status)): $($sig.StatusMessage)" + exit 1 + } + # UnknownError covers any chain failure, so it passes only for the + # one this certificate has: a chain of itself alone whose only + # fault is an untrusted root. The chain is read, not the message, + # which is in the runner's language. + if ($sig.Status.ToString() -eq 'UnknownError') { + $chain = New-Object System.Security.Cryptography.X509Certificates.X509Chain + $chain.ChainPolicy.RevocationMode = [System.Security.Cryptography.X509Certificates.X509RevocationMode]::NoCheck + $null = $chain.Build($sig.SignerCertificate) + $faults = @($chain.ChainStatus | ForEach-Object { $_.Status.ToString() }) + if ($chain.ChainElements.Count -ne 1 -or $faults.Count -ne 1 -or $faults[0] -ne 'UntrustedRoot') { + Write-Error "$($file.Name): signature chain fails for another reason ($($faults -join ', ')): $($sig.StatusMessage)" + exit 1 + } + } if ($sig.SignerCertificate.Thumbprint -ne $env:SIGNING_THUMBPRINT) { Write-Error "$($file.Name) is not signed with the release certificate ($($sig.Status))" exit 1 @@ -165,7 +211,7 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ env.RELEASE_TAG }} + ref: ${{ needs.version.outputs.sha }} persist-credentials: false # The packaging definitions (desktop entries, rpm.toml) come from the # workflow's own ref: on a tag push that is the tag itself, on a @@ -184,15 +230,33 @@ jobs: - run: rustup show active-toolchain - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - run: cargo test --locked - - run: cargo install cargo-packager cargo-generate-rpm --locked + - run: cargo install cargo-packager --version "$CARGO_PACKAGER_VERSION" --locked + - run: cargo install cargo-generate-rpm --version "$CARGO_GENERATE_RPM_VERSION" --locked + # The .deb depends on the glibc the binary was linked against: the + # newest GLIBC_ symbol version it references. + - name: glibc floor + run: | + cargo build --release --locked + glibc=$(LC_ALL=C objdump -T target/release/waveflow-quickplay | grep -o 'GLIBC_[0-9.]*' | cut -d_ -f2 | sort -uV | tail -1) + echo "needs glibc $glibc" + sed -i "s/^depends = \[/depends = [\"libc6 (>= $glibc)\", /" Cargo.toml + grep -m1 '^depends' Cargo.toml - run: cargo packager --release --formats deb,appimage # The .rpm comes from cargo-generate-rpm (cargo-packager has no rpm - # format), from the binary cargo-packager just built; its contents + # format), from the release binary built above; its contents # and dependencies are the [package.metadata.generate-rpm] section # kept in packaging/rpm.toml, appended to Cargo.toml here. - name: RPM run: | cat packaging/rpm.toml >> Cargo.toml + # What the binary links, with its symbol versions, the way rpm's + # ELF scan would declare it (automatic detection is off, see + # packaging/rpm.toml). + LC_ALL=C objdump -p target/release/waveflow-quickplay | awk ' + $1 == "NEEDED" { print "\"" $2 "()(64bit)\" = \"*\"" } + /required from/ { lib = $NF; sub(/:$/, "", lib) } + lib && NF == 4 && $4 ~ /_/ { print "\"" lib "(" $4 ")(64bit)\" = \"*\"" } + ' | sort -u >> Cargo.toml cargo generate-rpm --payload-compress zstd rpm=$(ls target/generate-rpm/*.rpm) mv "$rpm" target/packager/ @@ -205,6 +269,7 @@ jobs: rpm -qpl "$rpm" | grep -qx /usr/bin/waveflow-quickplay rpm -qpl "$rpm" | grep -qx /usr/share/applications/app.waveflow.quickplay.desktop rpm -qpR "$rpm" | grep -q 'libasound.so.2' + rpm -qpR "$rpm" | grep -q 'libc.so.6(GLIBC_' - name: Checksums run: | cd target/packager @@ -232,7 +297,7 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ env.RELEASE_TAG }} + ref: ${{ needs.version.outputs.sha }} persist-credentials: false # The bundle definition comes from the workflow's own ref, as the # Linux packaging does, so a manual run can attach it to an older tag. @@ -289,7 +354,7 @@ jobs: publish: name: GitHub release - needs: [windows, linux, macos] + needs: [version, windows, linux, macos] # Runs when every job that ran succeeded; `windows` and `macos` may # have been skipped by `linux_only`. if: ${{ always() && needs.linux.result == 'success' && (needs.windows.result == 'success' || needs.windows.result == 'skipped') && (needs.macos.result == 'success' || needs.macos.result == 'skipped') }} @@ -300,13 +365,25 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ env.RELEASE_TAG }} + ref: ${{ needs.version.outputs.sha }} persist-credentials: false - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: path: dist merge-multiple: true - run: ls -la dist + # The tag must still point where the packages were built from, or the + # release would pair them with another commit's tag. + - name: Tag unchanged + env: + GH_TOKEN: ${{ github.token }} + BUILT_SHA: ${{ needs.version.outputs.sha }} + run: | + now=$(gh api "repos/$GITHUB_REPOSITORY/commits/refs/tags/$RELEASE_TAG" --jq .sha) + if [ "$now" != "$BUILT_SHA" ]; then + echo "::error::$RELEASE_TAG moved from $BUILT_SHA to $now during the run" + exit 1 + fi # On an existing release the files replace their namesakes, the # others are kept, and the body is rewritten from the notes at the tag. - uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3.0.3 diff --git a/.gitignore b/.gitignore index e8ef79f..3edfb86 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,8 @@ /target /.claude +# Agent skills installed locally (npx skills), not part of the project. +/.agents +/skills-lock.json *.json.tmp # Stray captures from local test scripts; real screenshots live in docs/screenshots. /*.png diff --git a/Cargo.lock b/Cargo.lock index cc84b4a..9baf9b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4785,6 +4785,7 @@ dependencies = [ "egui", "image", "interprocess", + "libc", "lofty", "objc2 0.5.3", "objc2-foundation 0.2.2", @@ -4793,6 +4794,7 @@ dependencies = [ "rtrb", "serde", "serde_json", + "skrifa", "sys-locale", "tracing", "tracing-subscriber", diff --git a/Cargo.toml b/Cargo.toml index 32d9ba4..e913fa7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,10 @@ eframe = { version = "0.36", default-features = false, features = [ "x11", ] } egui = "0.36" +# The font parser egui itself uses (same version, so one build): a system +# fallback font is checked with it before egui is given the file, since +# egui panics on one it cannot read. +skrifa = { version = "0.44", default-features = false, features = ["std"] } # ── Audio transport (the same crates and versions the desktop engine uses) ─ # Decoding, channel conversion and resampling come from `waveflow-core` @@ -91,8 +95,10 @@ sys-locale = "0.3" [target.'cfg(windows)'.dependencies] # Owner relationship between the playlist window and the player (one taskbar -# button, minimized together), the registry, and the version resource of an -# installed WaveFlow; same version winit already pulls. +# button, minimized together), the registry, the version resource of an +# installed WaveFlow, who runs the other end of the single-instance pipe and +# whether it holds data yet; +# same version winit already pulls. windows-sys = { version = "0.61", features = [ "Win32_Foundation", "Win32_System_Threading", @@ -101,8 +107,15 @@ windows-sys = { version = "0.61", features = [ "Win32_System_Console", "Win32_System_Registry", "Win32_Storage_FileSystem", + "Win32_Security", + "Win32_System_Pipes", ] } +[target.'cfg(unix)'.dependencies] +# `geteuid`: the single-instance socket checks who is at the other end on +# the Linux abstract namespace, and who owns the directory it lives in. +libc = "0.2" + [target.'cfg(target_os = "macos")'.dependencies] # Files opened from Finder arrive as `application:openURLs:` on the # application delegate (src/macos.rs): the event loop is built here so the @@ -142,6 +155,9 @@ panic = "unwind" # on Linux. The NSIS installer is per user, no administrator rights, and runs # `--register` / `--unregister` (packaging/installer.nsi); the Linux packages # carry the desktop entry with the audio MIME types (packaging/quickplay.desktop). +# It packages the binary already built: run `cargo build --release --locked` +# first. No build hook here, so the release workflow compiles everything +# before the signing certificate is imported and nothing compiles after. [package.metadata.packager] product-name = "WaveFlow QuickPlay" identifier = "app.waveflow.quickplay" @@ -149,7 +165,6 @@ publisher = "InstaZDLL" category = "Music" icons = ["assets/icon.ico", "assets/icon-128.png"] out-dir = "target/packager" -before-packaging-command = "cargo build --release" binaries = [{ path = "waveflow-quickplay", main = true }] license-file = "LICENSE" resources = [{ src = "LICENSE", target = "LICENSE.txt" }] @@ -164,5 +179,5 @@ display-language-selector = false [package.metadata.packager.deb] package-name = "waveflow-quickplay" desktop-template = "packaging/quickplay.desktop" -depends = ["libasound2 | libasound2t64", "libxkbcommon0", "libwayland-client0", "libgl1"] +depends = ["libgcc-s1", "libasound2 | libasound2t64", "libxkbcommon0", "libwayland-client0", "libegl1", "libgl1", "libx11-6", "libx11-xcb1", "libxcursor1", "libxi6", "libxkbcommon-x11-0"] section = "sound" diff --git a/LICENSE b/LICENSE index 6cd286a..d07ba91 100644 --- a/LICENSE +++ b/LICENSE @@ -633,8 +633,8 @@ to attach them to the start of each source file to most effectively state the exclusion of warranty; and each file should have at least the "copyright" line and a pointer to where the full notice is found. - TPI Flow - Outil de planification pour le TPI - Copyright (C) 2026 InstaZDLL + + Copyright (C) This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by diff --git a/README.md b/README.md index 68b8523..ae2b6f8 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,7 @@ cargo clippy --all-targets cargo test cargo install cargo-packager cargo-generate-rpm +# The packagers take the release build above; they do not build it. cargo packager --release --formats nsis # Windows: per-user installer cargo packager --release --formats deb,appimage # Linux cat packaging/rpm.toml >> Cargo.toml && cargo generate-rpm # Linux: .rpm → target/generate-rpm/ diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index dffff6c..4a34c63 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -157,7 +157,20 @@ socket (`src/ipc.rs`): a named pipe on Windows, a socket file in the per-user runtime directory (`$XDG_RUNTIME_DIR`, `$TMPDIR` on macOS) on Unix, the Linux abstract namespace only when there is no such directory. The protocol is a few UTF-8 lines (`play` or `enqueue`, one path per line, -a blank line, `ok` back); messages are capped in size and count. Explorer +a blank line, `ok` back); messages are capped in size and count, and one +that is cut short or over a cap gets no `ok`. A path that is not UTF-8 or +holds a line break is not sent: that launch opens a window of its own. The +server reads each connection on a thread of its own, eight at most, and +gives each message two seconds in all (a Unix socket is read nonblocking, a +pipe is peeked before each read, since it has no timeouts); the client +gives up after three seconds and, once it has, sends nothing even if its +connection gets through later. A client that never finishes, or an +instance that never answers, holds up nothing else for long. Pipe names and the abstract namespace are machine-wide, +so another user could take the name first: there each end checks that the +other runs as the same user (the token's user SID on Windows, the peer's +uid on Linux), and a launch that finds a stranger opens its own window. The +runtime directory is +only used when this user owns it and nobody else can enter it. Explorer starts one process per selected file, so `play` messages arriving within half a second of each other are merged into one queue replacement. diff --git a/packaging/macos/bundle.sh b/packaging/macos/bundle.sh index 9976d22..efd826f 100755 --- a/packaging/macos/bundle.sh +++ b/packaging/macos/bundle.sh @@ -14,9 +14,18 @@ # unsigned code at all, so the signature is not optional. set -euo pipefail +if [ $# -ne 3 ]; then + echo "usage: $0 " >&2 + exit 2 +fi binary=$1 version=$2 out=$3 +# The version goes into a sed replacement, the plist and file names. +if ! [[ $version =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "bundle.sh: not a version: $version" >&2 + exit 2 +fi here=$(cd "$(dirname "$0")" && pwd) root=$(cd "$here/../.." && pwd) @@ -34,15 +43,17 @@ codesign --force --sign - --timestamp=none "$app" codesign --verify --strict --verbose=2 "$app" codesign --display --verbose=2 "$app" 2>&1 | grep -E '^(Identifier|Format|Signature)=' -stage="$out/dmg-stage" -rm -rf "$stage" -mkdir -p "$stage" -cp -R "$app" "$stage/" -ln -s /Applications "$stage/Applications" +# The image is staged, built and verified in a folder of this run's own, +# then moved over the previous one: a failed build leaves that image alone +# and no half-written one behind. +stage=$(mktemp -d "$out/dmg-stage.XXXXXX") +trap 'rm -rf "$stage"' EXIT +mkdir "$stage/root" +cp -R "$app" "$stage/root/" +ln -s /Applications "$stage/root/Applications" dmg="$out/waveflow-quickplay_${version}_macos-universal.dmg" -rm -f "$dmg" -hdiutil create -volname "WaveFlow QuickPlay $version" -srcfolder "$stage" \ - -fs HFS+ -format UDZO -ov "$dmg" -rm -rf "$stage" -hdiutil verify "$dmg" +hdiutil create -volname "WaveFlow QuickPlay $version" -srcfolder "$stage/root" \ + -fs HFS+ -format UDZO -ov "$stage/image.dmg" +hdiutil verify "$stage/image.dmg" +mv -f "$stage/image.dmg" "$dmg" echo "$dmg" diff --git a/packaging/rpm.toml b/packaging/rpm.toml index 6c78166..f6246e1 100644 --- a/packaging/rpm.toml +++ b/packaging/rpm.toml @@ -7,10 +7,14 @@ # # That lets the release workflow take the packaging definitions from its own # ref while building the sources of an older tag. Dependencies are soname -# capabilities, as rpm itself would derive them: only libasound is linked; -# wayland, xkbcommon and EGL/GL are dlopen-ed at run time, so they are named +# capabilities. Automatic detection is off because it would miss what is +# dlopen-ed at run time: wayland, xkbcommon, EGL/GL and, for X11 sessions, +# libX11, libX11-xcb, libXcursor, libXi and xkbcommon-x11, so they are named # here the way they are provided (alsa-lib, libwayland-client, libxkbcommon, -# libglvnd-egl, libglvnd-glx on Fedora). File triggers of desktop-file-utils +# libglvnd-egl, libglvnd-glx, libX11, libX11-xcb, libXcursor, libXi, +# libxkbcommon-x11 on Fedora). What the binary links (libc with its GLIBC_ +# symbol versions, libm, libgcc_s, libasound) is read from the binary by the +# release workflow and appended below, as rpm's own ELF scan would derive it. File triggers of desktop-file-utils # and hicolor-icon-theme refresh the caches, so no scriptlet is needed. [package.metadata.generate-rpm] @@ -28,8 +32,12 @@ assets = [ ] [package.metadata.generate-rpm.requires] -"libasound.so.2()(64bit)" = "*" "libwayland-client.so.0()(64bit)" = "*" "libxkbcommon.so.0()(64bit)" = "*" "libEGL.so.1()(64bit)" = "*" "libGL.so.1()(64bit)" = "*" +"libX11.so.6()(64bit)" = "*" +"libX11-xcb.so.1()(64bit)" = "*" +"libXcursor.so.1()(64bit)" = "*" +"libXi.so.6()(64bit)" = "*" +"libxkbcommon-x11.so.0()(64bit)" = "*" diff --git a/src/app.rs b/src/app.rs index 046a2c0..4bb3de0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -269,13 +269,7 @@ impl QuickPlayApp { .map(|e| std::path::absolute(&e.path).unwrap_or_else(|_| e.path.clone())) .collect(); let index = self.queue.current_index().unwrap_or(0); - // Before the pending load's `Loaded`, the engine's clock is still the - // previous track's: the new one starts where it was asked to. - let position = if self.loaded_generation == self.generation { - self.engine.as_ref().map_or(0, |e| e.shared().position_ms()) - } else { - self.load_start_ms - }; + let position = self.position_ms(); // Stopped or at the end: WaveFlow gets the track, from its start, // and waits; playing or loading: it carries on. let (position_ms, playing) = match self.state { @@ -313,6 +307,18 @@ impl QuickPlayApp { fn set_status(&mut self, message: impl Into) { self.status = Some((message.into(), Instant::now())); + // Idle, nothing else would wake the loop to take the line down. + self.ctx.request_repaint_after(STATUS_TTL); + } + + /// Where the current track is. Until the pending load's `Loaded` + /// arrives, the engine's clock still counts the previous track: the + /// new one is where it was asked to start. + fn position_ms(&self) -> u64 { + if self.state == PlayerState::Loading && self.loaded_generation != self.generation { + return self.load_start_ms; + } + self.engine.as_ref().map_or(0, |e| e.shared().position_ms()) } // ── Queue ─────────────────────────────────────────────────────────── @@ -371,6 +377,16 @@ impl QuickPlayApp { /// track again), unwinding the shuffle history to it instead of /// recording it again. fn play_index_from(&mut self, index: usize, start_ms: u64, autoplay: bool, back: bool) { + if index >= self.queue.len() { + return; + } + // No engine, no load: the player stays as it was, with the reason + // in the status line, and the next Play tries the device again. + // Committed to Loading first, it would wait for a Loaded that no + // engine is there to send. + if self.ensure_engine().is_none() { + return; + } let Some(entry) = self.queue.get(index) else { return; }; @@ -433,7 +449,7 @@ impl QuickPlayApp { } fn prev(&mut self) { - let position = self.engine.as_ref().map_or(0, |e| e.shared().position_ms()); + let position = self.position_ms(); if position > PREV_RESTART_THRESHOLD_MS || self.queue.prev_index().is_none() { self.restart_current(0); } else if let Some(i) = self.queue.prev_index() { @@ -462,6 +478,10 @@ impl QuickPlayApp { let ms = (fraction.clamp(0.0, 1.0) as f64 * duration as f64) as u64; match self.state { PlayerState::Playing | PlayerState::Paused | PlayerState::Loading => { + if self.state == PlayerState::Loading && self.loaded_generation != self.generation { + // Folded into where the open starts. + self.load_start_ms = ms; + } self.send(AudioCmd::Seek(ms)); } // After a natural end the decoder is idle: reload at the offset. @@ -479,20 +499,21 @@ impl QuickPlayApp { fn remove(&mut self, index: usize) { let was_current = self.queue.remove(index); - if was_current && self.playing_id.is_some() { - // The playing row is gone: stop, and let Next move on. - self.send(AudioCmd::Stop); - self.playing_id = None; - self.state = PlayerState::Idle; - self.duration_ms = None; - self.cover = None; - self.cover_for = None; - // The cursor now points at the row that moved up — Play resumes - // from it, which is what a list does when you delete the - // current entry. - if let Some(c) = self.queue.current_index() { - self.queue.set_current(Some(c)); + if was_current { + if self.playing_id.is_some() { + // The playing row is gone: stop, and drop whatever its load + // still has on the way, an end or an error that would + // start another track. + self.send(AudioCmd::Stop); + self.generation += 1; + self.playing_id = None; + self.state = PlayerState::Idle; } + // The cursor now points at the row that moved up, which has not + // played: Play and Next start it, as a list does when you + // delete the current entry. Nothing of the deleted track stays + // on screen under its title. + self.forget_track(); } self.selected = match self.selected { Some(s) if s == index => (index < self.queue.len()).then_some(index), @@ -504,18 +525,27 @@ impl QuickPlayApp { fn clear(&mut self) { self.queue.clear(); self.scan_generation += 1; + self.generation += 1; self.selected = None; self.playing_id = None; - self.duration_ms = None; - self.codec = None; - self.cover = None; - self.cover_for = None; + self.forget_track(); self.state = PlayerState::Idle; if let Some(engine) = &self.engine { engine.send(AudioCmd::Stop); } } + /// Drop what belonged to the track that was current: its length, + /// codec, lyrics, cover and a seek being dragged. + fn forget_track(&mut self) { + self.duration_ms = None; + self.codec = None; + self.lyrics = None; + self.seek_preview = None; + self.cover = None; + self.cover_for = None; + } + fn open_dialog(&mut self) { let t = self.t(); let dialog = rfd::FileDialog::new() @@ -734,7 +764,7 @@ impl QuickPlayApp { fn shortcut_snapshot(&self) -> ShortcutSnapshot { ShortcutSnapshot { volume: self.settings.volume, - position_ms: self.engine.as_ref().map_or(0, |e| e.shared().position_ms()), + position_ms: self.position_ms(), duration_ms: self.duration_ms, } } @@ -880,6 +910,9 @@ impl QuickPlayApp { } Action::Stop => { self.send(AudioCmd::Stop); + // An end or an error the decoder sent before it took the + // Stop must not start the next track. + self.generation += 1; self.state = PlayerState::Idle; self.seek_preview = None; } @@ -918,7 +951,7 @@ impl QuickPlayApp { let position_ms = match self.state { PlayerState::Idle => 0, PlayerState::Ended => self.duration_ms.unwrap_or(0), - _ => self.engine.as_ref().map_or(0, |e| e.shared().position_ms()), + _ => self.position_ms(), }; let format_chips = current.map_or_else(Vec::new, |e| { let meta = e.meta.as_ref(); diff --git a/src/audio/decoder.rs b/src/audio/decoder.rs index cb3b823..761af9a 100644 --- a/src/audio/decoder.rs +++ b/src/audio/decoder.rs @@ -326,6 +326,11 @@ impl Session<'_> { } AudioCmd::Resume => { self.shared.paused_output.store(false, Ordering::Release); + // The tail waits for the output, which does not play while + // paused: its deadline counts from the resume. + if let Some(since) = self.eof.as_mut() { + *since = Instant::now(); + } set_state(self.shared, self.events, PlayerState::Playing); None } @@ -416,7 +421,7 @@ fn play_track( // when it is full come back to the command drain after a nap. if !session.out.is_empty() { let before = session.out.len(); - push_samples(session.producer, &mut session.out); + push_samples(session.producer, &mut session.out, dst_channels); let written = (before - session.out.len()) as u64; shared.samples_pushed.fetch_add(written, Ordering::AcqRel); if !session.out.is_empty() { @@ -471,14 +476,16 @@ fn play_track( } } -/// Push as much of `buf` into the ring as fits; what did not fit stays at -/// the front of `buf`. -fn push_samples(producer: &mut Producer, buf: &mut Vec) { - let slots = producer.slots(); - if slots == 0 || buf.is_empty() { +/// Push as many whole frames of `buf` into the ring as fit; what did not +/// fit stays at the front of `buf`. Whole frames only, as the callback +/// reads them: a ring holding half a frame would shift every later sample +/// one channel over. +fn push_samples(producer: &mut Producer, buf: &mut Vec, channels: usize) { + let n = producer.slots().min(buf.len()); + let n = n - n % channels.max(1); + if n == 0 { return; } - let n = slots.min(buf.len()); if let Ok(chunk) = producer.write_chunk_uninit(n) { let written = chunk.fill_from_iter(buf.iter().copied()); buf.drain(..written); @@ -497,7 +504,7 @@ fn drain_ring(producer: &Producer, shared: &SharedPlayback) { #[cfg(test)] mod tests { - use super::clamp_to_track; + use super::{clamp_to_track, push_samples}; #[test] fn a_position_stays_within_the_track() { @@ -506,4 +513,20 @@ mod tests { assert_eq!(clamp_to_track(5_000, Some(0)), 5_000, "no length known"); assert_eq!(clamp_to_track(5_000, None), 5_000); } + + #[test] + fn only_whole_frames_go_into_the_ring() { + let (mut producer, mut consumer) = rtrb::RingBuffer::::new(5); + let mut buf = vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0]; + push_samples(&mut producer, &mut buf, 2); + assert_eq!( + buf, + [4.0, 5.0, 6.0, 7.0], + "two stereo frames fit in five slots" + ); + assert_eq!(consumer.slots(), 4); + consumer.pop().unwrap(); + push_samples(&mut producer, &mut buf, 2); + assert_eq!(buf, [6.0, 7.0], "one frame more once two slots are free"); + } } diff --git a/src/audio/engine.rs b/src/audio/engine.rs index b01cbdb..55a4e79 100644 --- a/src/audio/engine.rs +++ b/src/audio/engine.rs @@ -78,8 +78,15 @@ impl AudioEngine { )?; let (cmd_tx, cmd_rx) = unbounded::(); - let decoder = spawn_decoder_thread(cmd_rx, producer, shared.clone(), sink) - .map_err(|e| format!("spawn decoder thread: {e}"))?; + let decoder = match spawn_decoder_thread(cmd_rx, producer, shared.clone(), sink) { + Ok(decoder) => decoder, + Err(e) => { + // Close the device and join its thread before reporting, so + // a retry does not race the stream still open. + output.stop(); + return Err(format!("spawn decoder thread: {e}")); + } + }; Ok(Self { cmd_tx, diff --git a/src/audio/output.rs b/src/audio/output.rs index 4d9e7cb..ba326b9 100644 --- a/src/audio/output.rs +++ b/src/audio/output.rs @@ -167,15 +167,23 @@ where T: cpal::SizedSample + cpal::FromSample + Send + 'static, { let counters = Arc::clone(&shared); + let channels = usize::from(config.channels).max(1); let err_fn = move |err: cpal::Error| { // An xrun is a glitch the stream recovers from by itself: CoreAudio // reports one whenever the process stalls, as on the first launch of // a new build while macOS scans it. Releasing the engine for it would // stop the track; only the other errors mean the stream is gone. // Counted, not logged: on ALSA this runs on the stream's thread. - if err.kind() == cpal::ErrorKind::Xrun { - counters.xruns.fetch_add(1, Ordering::Relaxed); - return; + // A changed device is not an end either: the stream follows the + // default output (CoreAudio reports it when headphones are plugged + // in or out) and keeps playing on the new one. + match err.kind() { + cpal::ErrorKind::Xrun => { + counters.xruns.fetch_add(1, Ordering::Relaxed); + return; + } + cpal::ErrorKind::DeviceChanged => return, + _ => {} } on_error(format!("audio device error: {err}")); }; @@ -200,18 +208,28 @@ where return; } let volume = shared.volume(); - let mut written: u64 = 0; - for slot in out.iter_mut() { - *slot = match consumer.pop() { - Ok(s) => { - written += 1; - T::from_sample(s * volume) + // Whole frames, counted once up front: the decoder pushes + // whole frames, but one landing while this buffer is filled + // must not start in the middle of a frame left silent here, + // or every later sample would sit one channel over. + let ready = consumer.slots().min(out.len()); + let ready = ready - ready % channels; + let mut filled = 0; + if ready > 0 { + if let Ok(chunk) = consumer.read_chunk(ready) { + let (first, second) = chunk.as_slices(); + for (slot, s) in out.iter_mut().zip(first.iter().chain(second)) { + *slot = T::from_sample(s * volume); } - Err(_) => T::from_sample(0.0_f32), - }; + chunk.commit_all(); + filled = ready; + shared + .samples_played + .fetch_add(ready as u64, Ordering::AcqRel); + } } - if written > 0 { - shared.samples_played.fetch_add(written, Ordering::AcqRel); + for slot in out.iter_mut().skip(filled) { + *slot = T::from_sample(0.0_f32); } }, err_fn, diff --git a/src/audio/spectrum.rs b/src/audio/spectrum.rs index 3d70e1e..0af54b1 100644 --- a/src/audio/spectrum.rs +++ b/src/audio/spectrum.rs @@ -37,9 +37,13 @@ const MIN_HZ: f32 = 30.0; const MAX_HZ: f32 = 16_000.0; /// Audio between two emitted frames (~30 Hz). const EMIT_INTERVAL: Duration = Duration::from_millis(33); -/// Frames kept waiting for their play position; a second of audio at -/// 30 Hz is 30 frames, and a seek clears the queue anyway. -const MAX_QUEUED_FRAMES: usize = 64; +/// Frames kept waiting for their play position. The decoder runs up to a +/// ring ahead of the output, and the ring is a fixed count of interleaved +/// samples: a second of stereo at 48 kHz, but 12 s of mono at 8 kHz, which +/// is 360 frames at 30 Hz. Dropping the oldest frames because the queue +/// is too short would drop the ones due soonest and freeze the bars; a +/// seek clears the queue anyway. +const MAX_QUEUED_FRAMES: usize = 512; /// One analysed window, due when the output clock reaches `due_samples` /// (interleaved samples, the unit of `SharedPlayback::samples_played`). @@ -94,8 +98,12 @@ pub struct SpectrumAnalyzer { bands: [f32; BAND_COUNT], raw: [f32; BAND_COUNT], level: f32, - /// Audio slid through the window since the last emitted frame. + /// Audio slid through the window since the last emitted frame, less + /// the emission interval's carry: when the next frame is due. since_emit: usize, + /// Audio slid through the window since the last analysed frame: the + /// time the level follower moves by. + since_frame: usize, } impl SpectrumAnalyzer { @@ -115,6 +123,7 @@ impl SpectrumAnalyzer { raw: [0.0; BAND_COUNT], level: MIN_LEVEL, since_emit: usize::MAX, + since_frame: 0, } } @@ -150,9 +159,10 @@ impl SpectrumAnalyzer { while self.pending.len() >= FFT_SIZE { if self.since_emit >= interval { let first = self.since_emit == usize::MAX; - let audio = if first { interval } else { self.since_emit }; + let audio = if first { interval } else { self.since_frame }; let elapsed = Duration::from_secs_f32((audio as f32 / rate).min(1.0)); self.run_one_frame(rate, elapsed); + self.since_frame = 0; // The window's middle sits this many mono frames before // the newest sample fed. let behind = (self.pending.len() as u64).saturating_sub(FFT_SIZE as u64 / 2); @@ -170,6 +180,7 @@ impl SpectrumAnalyzer { let slid = HOP.min(self.pending.len()); self.pending.drain(..slid); self.since_emit = self.since_emit.saturating_add(slid); + self.since_frame = self.since_frame.saturating_add(slid); } } @@ -318,6 +329,20 @@ mod tests { assert!((15..=19).contains(&peak), "peak band {peak}"); } + #[test] + fn a_full_ring_of_low_rate_mono_keeps_the_frames_due_first() { + // 96 000 samples of 8 kHz mono: 12 s ahead, 360 frames at 30 Hz. + let mut q = SpectrumQueue::default(); + for i in 0..360u64 { + q.push(SpectrumFrame { + due_samples: i * 266, + bands: [i as f32; BAND_COUNT], + }); + } + assert_eq!(q.take_due(0).map(|b| b[0]), Some(0.0)); + assert_eq!(q.take_due(266).map(|b| b[0]), Some(1.0)); + } + #[test] fn the_soft_limit_is_continuous_at_the_knee() { assert!((soft_limit(KNEE) - KNEE).abs() < 1e-6); diff --git a/src/capture.rs b/src/capture.rs index d055d7c..022c96f 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -75,6 +75,10 @@ impl Capture { tracing::warn!(w, h, "capture: unexpected buffer size"); continue; }; + if let Err(e) = std::fs::create_dir_all(&self.dir) { + tracing::warn!(dir = %self.dir.display(), error = %e, "capture: no directory"); + continue; + } match buf.save(&path) { Ok(()) => tracing::info!(path = %path.display(), w, h, "capture written"), Err(e) => tracing::warn!(path = %path.display(), error = %e, "capture failed"), diff --git a/src/integration.rs b/src/integration.rs index 979bcbc..884c3dc 100644 --- a/src/integration.rs +++ b/src/integration.rs @@ -22,9 +22,27 @@ use crate::i18n::{Lang, Strings}; fn exe_path() -> io::Result { let exe = std::env::current_exe()?; let exe = exe.canonicalize().unwrap_or(exe); - // `canonicalize` yields `\\?\C:\…` on Windows; the shell wants `C:\…`. - let s = exe.to_string_lossy(); - Ok(PathBuf::from(s.strip_prefix(r"\\?\").unwrap_or(&s))) + // The registry and the desktop entry are text: a path that is not + // Unicode cannot be written into them unchanged, so it is refused + // rather than registered as another, nonexistent one. + let s = exe.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("{} is not valid Unicode", exe.display()), + ) + })?; + Ok(PathBuf::from(strip_verbatim(s))) +} + +/// `canonicalize` yields `\\?\C:\…`, and `\\?\UNC\server\share\…` for a +/// network share or a mapped drive, on Windows; the shell wants `C:\…` and +/// `\\server\share\…`. +fn strip_verbatim(path: &str) -> String { + if let Some(rest) = path.strip_prefix(r"\\?\UNC\") { + format!(r"\\{rest}") + } else { + path.strip_prefix(r"\\?\").unwrap_or(path).to_string() + } } /// `lang` is the language of the menu labels: on Windows the registry holds @@ -228,9 +246,7 @@ mod platform { for unit in script.encode_utf16() { bytes.extend_from_slice(&unit.to_le_bytes()); } - let path = - std::env::temp_dir().join(format!("waveflow-quickplay-{}.reg", std::process::id())); - std::fs::write(&path, bytes)?; + let path = write_new_script(&bytes)?; // CREATE_NO_WINDOW: called from the GUI too, where a console // window flashing up for reg.exe would be a surprise. const CREATE_NO_WINDOW: u32 = 0x0800_0000; @@ -247,6 +263,38 @@ mod platform { } } + /// A script file of this call's own: two imports can run at once (two + /// quick language changes each relabel on a thread), and neither may + /// overwrite or delete the other's. Created new, never opened over a + /// file that is already there. + fn write_new_script(bytes: &[u8]) -> io::Result { + use std::io::Write; + use std::sync::atomic::{AtomicU32, Ordering}; + static NEXT: AtomicU32 = AtomicU32::new(0); + loop { + let n = NEXT.fetch_add(1, Ordering::Relaxed); + let path = std::env::temp_dir() + .join(format!("waveflow-quickplay-{}-{n}.reg", std::process::id())); + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(mut file) => { + if let Err(e) = file.write_all(bytes) { + drop(file); + let _ = std::fs::remove_file(&path); + return Err(e); + } + return Ok(path); + } + // Left by an earlier process with the same id: next name. + Err(e) if e.kind() == io::ErrorKind::AlreadyExists && n < 1000 => {} + Err(e) => return Err(e), + } + } + } + fn notify_shell() { use windows_sys::Win32::UI::Shell::{SHChangeNotify, SHCNE_ASSOCCHANGED, SHCNF_IDLIST}; // SAFETY: documented call with null item pointers for this event. @@ -348,19 +396,44 @@ mod platform { .join(format!("{DESKTOP_ID}.png"))) } - /// Desktop-entry quoting for the Exec key. + /// One argument of the Exec key, quoted: `"`, `` ` ``, `$` and `\` + /// escaped inside the quotes, and `%` doubled, since a single one + /// starts a field code. The whole value then goes through + /// [`desktop_string`]. fn exec_quote(s: &str) -> String { let mut out = String::from("\""); for c in s.chars() { - if matches!(c, '"' | '`' | '$' | '\\') { - out.push('\\'); + match c { + '"' | '`' | '$' | '\\' => { + out.push('\\'); + out.push(c); + } + '%' => out.push_str("%%"), + _ => out.push(c), } - out.push(c); } out.push('"'); out } + /// A desktop-entry string value: backslashes and the characters that + /// would end or break the line escaped. Exec and TryExec are string + /// values, so a literal backslash in a path is `\\\\` in Exec (quoted, + /// then escaped) and `\\` in TryExec. + fn desktop_string(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '\\' => out.push_str(r"\\"), + '\n' => out.push_str(r"\n"), + '\r' => out.push_str(r"\r"), + '\t' => out.push_str(r"\t"), + _ => out.push(c), + } + } + out + } + /// The desktop entry carries every language, so there is never /// anything to rewrite. pub fn is_registered_at(_exe: &std::path::Path) -> bool { @@ -390,14 +463,20 @@ mod platform { out } - pub fn register(exe: &std::path::Path, _t: &Strings) -> io::Result { - let exe_s = exe.to_string_lossy(); - let quoted = exec_quote(&exe_s); + fn desktop_entry(exe: &str) -> String { + let play = desktop_string(&format!("{} %F", exec_quote(exe))); + let enqueue_exec = desktop_string(&format!("{} --enqueue %F", exec_quote(exe))); + let try_exec = desktop_string(exe); let comment = localised("Comment", |t| t.desktop_comment); let enqueue = localised("Name", |t| t.action_add_to_playlist); - let entry = format!( - "[Desktop Entry]\nType=Application\nName=WaveFlow QuickPlay\n{comment}Exec={quoted} %F\nTryExec={exe_s}\nIcon={DESKTOP_ID}\nTerminal=false\nCategories=AudioVideo;Audio;Player;\nMimeType={MIME_TYPES}\nStartupWMClass={DESKTOP_ID}\nActions=Enqueue;\n\n[Desktop Action Enqueue]\n{enqueue}Exec={quoted} --enqueue %F\n" - ); + format!( + "[Desktop Entry]\nType=Application\nName=WaveFlow QuickPlay\n{comment}Exec={play}\nTryExec={try_exec}\nIcon={DESKTOP_ID}\nTerminal=false\nCategories=AudioVideo;Audio;Player;\nMimeType={MIME_TYPES}\nStartupWMClass={DESKTOP_ID}\nActions=Enqueue;\n\n[Desktop Action Enqueue]\n{enqueue}Exec={enqueue_exec}\n" + ) + } + + pub fn register(exe: &std::path::Path, _t: &Strings) -> io::Result { + // `exe_path` has made sure the path is Unicode. + let entry = desktop_entry(&exe.to_string_lossy()); let desktop = desktop_file()?; let icon = icon_file()?; for p in [&desktop, &icon] { @@ -416,15 +495,40 @@ mod platform { pub fn unregister() -> io::Result { for p in [desktop_file()?, icon_file()?] { - if let Err(e) = std::fs::remove_file(&p) { - if p.exists() { - return Err(e); - } + match std::fs::remove_file(&p) { + Err(e) if e.kind() != io::ErrorKind::NotFound => return Err(e), + _ => {} } } refresh(); Ok("removed the desktop entry and the icon.".into()) } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn exec_and_try_exec_carry_any_path_unchanged() { + let entry = desktop_entry("/opt/a\\b%c$d \"e\"/qp"); + assert!( + entry.contains("\nExec=\"/opt/a\\\\\\\\b%%c\\\\$d \\\\\"e\\\\\"/qp\" %F\n"), + "{entry}" + ); + assert!( + entry.contains("\nTryExec=/opt/a\\\\b%c$d \"e\"/qp\n"), + "{entry}" + ); + assert!(entry + .contains("\nExec=\"/opt/a\\\\\\\\b%%c\\\\$d \\\\\"e\\\\\"/qp\" --enqueue %F\n")); + let entry = desktop_entry("/opt/new\nline/qp"); + assert!( + entry.contains("\nExec=\"/opt/new\\nline/qp\" %F\n"), + "{entry}" + ); + assert!(!entry.contains("new\nline")); + } + } } #[cfg(not(any(windows, target_os = "linux")))] @@ -460,3 +564,18 @@ mod platform { Err(io::Error::other("nothing to remove on this platform.")) } } + +#[cfg(test)] +mod tests { + use super::strip_verbatim; + + #[test] + fn verbatim_prefixes_come_off_shares_included() { + assert_eq!(strip_verbatim(r"\\?\C:\QP\qp.exe"), r"C:\QP\qp.exe"); + assert_eq!( + strip_verbatim(r"\\?\UNC\server\share\qp.exe"), + r"\\server\share\qp.exe" + ); + assert_eq!(strip_verbatim("/usr/bin/qp"), "/usr/bin/qp"); + } +} diff --git a/src/ipc.rs b/src/ipc.rs index 0026db7..d21ab25 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -6,17 +6,29 @@ //! mode-0700 directory (`$XDG_RUNTIME_DIR`, or `$TMPDIR` on macOS) where //! there is one, so no other local user can reach or squat it; a named pipe //! on Windows, whose default DACL lets only the owner write; the Linux -//! abstract namespace only as a last resort. Messages are capped in size -//! and count. What crosses the socket is a list of file paths to play, -//! nothing secret. Protocol, UTF-8 lines: `play` or `enqueue`, one absolute -//! path per line, an empty line; the server answers `ok` once it has queued -//! the message, so the client can exit knowing the files were taken. +//! abstract namespace only as a last resort. Pipe and abstract names are +//! machine-wide, so another user could create ours first: there both ends +//! check that the other runs as the same user. Messages are capped in size +//! and count. What +//! crosses the socket is a list of file paths to play, nothing secret. +//! Protocol, UTF-8 lines: `play` or `enqueue`, one absolute path per line, +//! an empty line; the server answers `ok` once it has queued the message, +//! so the client can exit knowing the files were taken. A message that is +//! cut short, too large or unreadable gets no `ok`. +//! +//! Every connection is read on a thread of its own, bounded in number, +//! against one deadline for the whole message ([`MESSAGE_DEADLINE`]), and +//! the client gives up after [`CLIENT_TIMEOUT`]: a client that connects and +//! never finishes cannot hold up the launches after it, and a server that +//! never answers cannot hang one. use std::io::{self, BufRead, BufReader, Read, Write}; use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; -use crossbeam_channel::{Receiver, Sender}; +use crossbeam_channel::{Receiver, Sender, TrySendError}; use interprocess::local_socket::{ prelude::*, GenericFilePath, GenericNamespaced, ListenerOptions, Name, Stream, }; @@ -48,7 +60,27 @@ pub struct Message { /// the UI (`request_repaint`) when a message arrives. pub type CtxSlot = Arc>>; -fn socket_name() -> io::Result<(Name<'static>, Option)> { +/// Longest a hand-off may take, connection to answer. Named pipes have no +/// read timeout, so the exchange runs on a thread and is waited for. +const CLIENT_TIMEOUT: Duration = Duration::from_secs(3); +/// Longest the server waits for a client to finish its message, all reads +/// together. +const MESSAGE_DEADLINE: Duration = Duration::from_secs(2); +/// Connections read at the same time. One more is closed unread. +const MAX_CLIENTS: usize = 8; +/// Messages taken and not yet picked up by the UI. One more is refused. +const MAX_QUEUED: usize = 64; + +/// The socket's name, its file when it has one, and whether its name is +/// machine-wide (a Windows pipe, the Linux abstract namespace): anyone can +/// take such a name first, so each end checks who the other runs as. +struct Endpoint { + name: Name<'static>, + path: Option, + check_peer: bool, +} + +fn endpoint() -> io::Result { let user = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) .unwrap_or_else(|_| "default".into()); @@ -61,7 +93,11 @@ fn socket_name() -> io::Result<(Name<'static>, Option)> { let private_dir = private_runtime_dir(); let abstract_ns = cfg!(any(target_os = "linux", target_os = "android")); if cfg!(windows) || (private_dir.is_none() && abstract_ns) { - Ok((base.to_ns_name::()?, None)) + Ok(Endpoint { + name: base.to_ns_name::()?, + path: None, + check_peer: true, + }) } else { let dir = private_dir .or_else(dirs::cache_dir) @@ -71,16 +107,21 @@ fn socket_name() -> io::Result<(Name<'static>, Option)> { .clone() .into_os_string() .to_fs_name::()?; - Ok((name, Some(path))) + Ok(Endpoint { + name, + path: Some(path), + check_peer: false, + }) } } /// A directory only this user can enter: `$XDG_RUNTIME_DIR` (mode 0700 by /// the spec) where there is one; on macOS the per-user temporary directory /// launchd hands out in `$TMPDIR` (`/var/folders/…/T`, mode 0700), never -/// the shared `/tmp`. +/// the shared `/tmp`. Both come from the environment, so they are checked: +/// owned by this user, no access for anyone else. fn private_runtime_dir() -> Option { - if let Some(dir) = dirs::runtime_dir().filter(|d| d.is_dir()) { + if let Some(dir) = dirs::runtime_dir().filter(|d| is_private(d)) { return Some(dir); } if cfg!(target_os = "macos") { @@ -88,18 +129,120 @@ fn private_runtime_dir() -> Option { let shared = ["/tmp", "/private/tmp"] .iter() .any(|shared| dir.starts_with(shared)); - if !shared && dir.is_dir() { + if !shared && is_private(&dir) { return Some(dir); } } None } +#[cfg(unix)] +fn is_private(dir: &std::path::Path) -> bool { + use std::os::unix::fs::MetadataExt; + std::fs::metadata(dir).is_ok_and(|m| { + // SAFETY: geteuid has no preconditions and cannot fail. + m.is_dir() && m.uid() == unsafe { libc::geteuid() } && m.mode() & 0o077 == 0 + }) +} + +#[cfg(not(unix))] +fn is_private(dir: &std::path::Path) -> bool { + dir.is_dir() +} + +/// Whether the process at the other end of `conn` runs as this user. +#[cfg(unix)] +fn same_user(conn: &Stream) -> bool { + // SAFETY: geteuid has no preconditions and cannot fail. + let me = unsafe { libc::geteuid() }; + conn.peer_creds() + .ok() + .and_then(|c| c.euid()) + .is_some_and(|uid| uid == me) +} + +#[cfg(windows)] +fn same_user(conn: &Stream) -> bool { + conn.peer_creds() + .ok() + .and_then(|c| c.pid()) + .is_some_and(crate::platform::same_user_process) +} + +#[cfg(not(any(unix, windows)))] +fn same_user(_conn: &Stream) -> bool { + true +} + /// Hands `paths` to a running instance. `Ok(true)` when one took them, -/// `Ok(false)` when nobody is listening. +/// `Ok(false)` when nobody is listening or the one listening turned them +/// down. `InvalidInput` when a path cannot be carried (not UTF-8, or with +/// a line break in it), `TimedOut` when the instance did not answer. pub fn send(mode: Mode, paths: &[PathBuf]) -> io::Result { - let (name, _) = socket_name()?; - let mut conn = match Stream::connect(name) { + let body = encode(mode, paths)?; + let point = endpoint()?; + let (tx, rx) = crossbeam_channel::bounded(1); + // Set when this call gives up: an exchange still stuck in its connect + // (a busy pipe) then sends nothing once it gets through, or the files + // would land in the running player as well as in the window this + // launch opens instead. + let abandoned = Arc::new(AtomicBool::new(false)); + let worker_abandoned = abandoned.clone(); + std::thread::Builder::new() + .name("quickplay-ipc-send".into()) + .spawn(move || { + let _ = tx.send(exchange(point, &body, &worker_abandoned)); + })?; + match rx.recv_timeout(CLIENT_TIMEOUT) { + Ok(result) => result, + Err(_) => { + abandoned.store(true, Ordering::Release); + Err(io::Error::new( + io::ErrorKind::TimedOut, + "the running instance did not answer", + )) + } + } +} + +fn encode(mode: Mode, paths: &[PathBuf]) -> io::Result { + if paths.len() > MAX_PATHS { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("more than {MAX_PATHS} files"), + )); + } + let mut body = String::new(); + body.push_str(mode.as_str()); + body.push('\n'); + for p in paths { + let abs = std::path::absolute(p).unwrap_or_else(|_| p.clone()); + // One path per line, as UTF-8: a path that cannot be written that + // way is not changed into another one, the hand-off is refused. + let line = abs + .to_str() + .filter(|s| !s.contains(['\n', '\r'])) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} cannot be handed over", abs.display()), + ) + })?; + body.push_str(line); + body.push('\n'); + } + body.push('\n'); + if body.len() as u64 > MAX_MESSAGE_BYTES { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "too many files to hand over", + )); + } + Ok(body) +} + +fn exchange(point: Endpoint, body: &str, abandoned: &AtomicBool) -> io::Result { + let mut conn = match Stream::connect(point.name) { Ok(c) => c, Err(e) if matches!( @@ -111,15 +254,19 @@ pub fn send(mode: Mode, paths: &[PathBuf]) -> io::Result { } Err(e) => return Err(e), }; - let mut body = String::new(); - body.push_str(mode.as_str()); - body.push('\n'); - for p in paths { - let abs = std::path::absolute(p).unwrap_or_else(|_| p.clone()); - body.push_str(&abs.to_string_lossy()); - body.push('\n'); + if point.check_peer && !same_user(&conn) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "the socket is held by another user", + )); } - body.push('\n'); + if abandoned.load(Ordering::Acquire) { + return Err(io::Error::new(io::ErrorKind::TimedOut, "given up")); + } + // Where the transport has timeouts (Unix sockets), the worker ends + // with the wait instead of outliving it. + let _ = conn.set_send_timeout(Some(CLIENT_TIMEOUT)); + let _ = conn.set_recv_timeout(Some(CLIENT_TIMEOUT)); conn.write_all(body.as_bytes())?; conn.flush()?; let mut reply = Vec::new(); @@ -140,27 +287,32 @@ pub struct Server { impl Server { /// Starts listening; `AddrInUse` means another instance won the race. pub fn start() -> io::Result { - let (name, socket_path) = socket_name()?; - let listener = match ListenerOptions::new().name(name.clone()).create_sync() { + let point = endpoint()?; + let socket_path = point.path.clone(); + let listener = match ListenerOptions::new() + .name(point.name.clone()) + .create_sync() + { Err(e) if e.kind() == io::ErrorKind::AddrInUse && socket_path.is_some() => { // Either a file left by a crashed instance, or an instance // that came up between our connect attempt and now. Only a // socket nobody answers is stale; a live one is handed to. - if Stream::connect(name.clone()).is_ok() { + if Stream::connect(point.name.clone()).is_ok() { return Err(e); } let path = socket_path.as_deref().expect("checked"); std::fs::remove_file(path)?; - ListenerOptions::new().name(name).create_sync()? + ListenerOptions::new().name(point.name).create_sync()? } other => other?, }; - let (tx, rx) = crossbeam_channel::unbounded(); + let (tx, rx) = crossbeam_channel::bounded(MAX_QUEUED); let ctx: CtxSlot = Arc::default(); let thread_ctx = ctx.clone(); + let check_peer = point.check_peer; std::thread::Builder::new() .name("quickplay-ipc".into()) - .spawn(move || serve(listener, tx, thread_ctx))?; + .spawn(move || serve(listener, tx, thread_ctx, check_peer))?; tracing::debug!("single-instance socket ready"); Ok(Self { rx, @@ -191,7 +343,13 @@ impl Drop for Server { } } -fn serve(listener: interprocess::local_socket::Listener, tx: Sender, ctx: CtxSlot) { +fn serve( + listener: interprocess::local_socket::Listener, + tx: Sender, + ctx: CtxSlot, + check_peer: bool, +) { + let active = Arc::new(AtomicUsize::new(0)); for conn in listener.incoming() { let conn = match conn { Ok(c) => c, @@ -200,20 +358,112 @@ fn serve(listener: interprocess::local_socket::Listener, tx: Sender, ct continue; } }; - let mut reader = BufReader::new(conn); - let Some(message) = read_message(&mut reader) else { + if check_peer && !same_user(&conn) { + tracing::warn!("ipc: connection from another user refused"); continue; - }; - tracing::info!(mode = ?message.mode, paths = message.paths.len(), "ipc: files received"); - if tx.send(message).is_err() { + } + if active.load(Ordering::Acquire) >= MAX_CLIENTS { + tracing::warn!("ipc: too many connections at once, one refused"); + continue; + } + active.fetch_add(1, Ordering::AcqRel); + let (tx, ctx, done) = (tx.clone(), ctx.clone(), active.clone()); + let spawned = std::thread::Builder::new() + .name("quickplay-ipc-client".into()) + .spawn(move || { + handle(conn, &tx, &ctx); + done.fetch_sub(1, Ordering::AcqRel); + }); + if let Err(e) = spawned { + active.fetch_sub(1, Ordering::AcqRel); + tracing::warn!(error = %e, "ipc: no thread for a connection"); + } + } +} + +fn handle(conn: Stream, tx: &Sender, ctx: &CtxSlot) { + // One deadline for the whole message: named pipes have no read + // timeout, and a per-read timeout on a Unix socket lets a client that + // sends a byte now and then hold its slot forever. Unix sockets are + // read nonblocking; a pipe read nonblocking reports "no data yet" as + // the end of the stream, so it is polled instead (see `Deadline`). + #[cfg(not(windows))] + if let Err(e) = conn.set_nonblocking(true) { + tracing::warn!(error = %e, "ipc: connection cannot be read with a deadline"); + return; + } + let mut reader = BufReader::new(Deadline { + inner: conn, + until: Instant::now() + MESSAGE_DEADLINE, + }); + let Some(message) = read_message(&mut reader) else { + return; + }; + tracing::info!(mode = ?message.mode, paths = message.paths.len(), "ipc: files received"); + match tx.try_send(message) { + Ok(()) => {} + Err(TrySendError::Full(_)) => { + tracing::warn!("ipc: the player is not taking messages, one refused"); return; } - if let Ok(slot) = ctx.lock() { - if let Some(ctx) = slot.as_ref() { - ctx.request_repaint(); + Err(TrySendError::Disconnected(_)) => return, + } + if let Ok(slot) = ctx.lock() { + if let Some(ctx) = slot.as_ref() { + ctx.request_repaint(); + } + } + let conn = &mut reader.get_mut().inner; + #[cfg(not(windows))] + let _ = conn.set_nonblocking(false); + let _ = conn.write_all(b"ok\n"); +} + +/// Reads a connection until `until`; past it the read fails, which ends +/// the message and frees the connection's slot. A Unix socket is read +/// nonblocking, and a read that would block waits a little and tries +/// again; a pipe stays blocking and is only read once it holds data. +struct Deadline { + inner: Stream, + until: Instant, +} + +impl Deadline { + fn wait(&self) -> io::Result<()> { + if Instant::now() >= self.until { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "message not finished in time", + )); + } + std::thread::sleep(Duration::from_millis(10)); + Ok(()) + } +} + +impl Read for Deadline { + #[cfg(windows)] + fn read(&mut self, buf: &mut [u8]) -> io::Result { + use std::os::windows::io::{AsHandle, AsRawHandle}; + let Stream::NamedPipe(pipe) = &self.inner; + // Nothing waiting yet: wait. A peek that fails (the client went + // away) falls through to the read, which reports it. + while crate::platform::pipe_bytes_available(pipe.as_handle().as_raw_handle()) + .is_ok_and(|n| n == 0) + { + self.wait()?; + } + self.inner.read(buf) + } + + #[cfg(not(windows))] + fn read(&mut self, buf: &mut [u8]) -> io::Result { + loop { + match self.inner.read(buf) { + Err(e) if e.kind() == io::ErrorKind::WouldBlock => self.wait()?, + other => return other, } } - let _ = reader.get_mut().write_all(b"ok\n"); } } @@ -221,6 +471,10 @@ fn serve(listener: interprocess::local_socket::Listener, tx: Sender, ct const MAX_MESSAGE_BYTES: u64 = 4 << 20; const MAX_PATHS: usize = 10_000; +/// One whole message, or `None`: a read error, a message cut short (no +/// empty line, or a last line without its line break, as when the size +/// limit cuts it), too many paths. A bare mode line followed by the end of +/// the stream is a focus request. fn read_message(reader: &mut R) -> Option { let mut reader = reader.take(MAX_MESSAGE_BYTES); let mut line = String::new(); @@ -238,11 +492,31 @@ fn read_message(reader: &mut R) -> Option { let mut paths = Vec::new(); loop { line.clear(); - let n = reader.read_line(&mut line).unwrap_or(0); - let path = line.trim_end_matches(['\r', '\n']); - if n == 0 || path.is_empty() || paths.len() >= MAX_PATHS { + let n = match reader.read_line(&mut line) { + Ok(n) => n, + Err(e) => { + tracing::warn!(error = %e, "ipc: message unreadable"); + return None; + } + }; + if n == 0 { + if paths.is_empty() { + break; + } + tracing::warn!("ipc: message cut short"); + return None; + } + let Some(path) = line.strip_suffix('\n') else { + tracing::warn!("ipc: message cut short"); + return None; + }; + if path.is_empty() { break; } + if paths.len() >= MAX_PATHS { + tracing::warn!(max = MAX_PATHS, "ipc: too many paths"); + return None; + } paths.push(PathBuf::from(path)); } Some(Message { mode, paths }) @@ -269,4 +543,26 @@ mod tests { let m = read_message(&mut Cursor::new("play\n")).unwrap(); assert!(m.paths.is_empty(), "a bare play is a focus request"); } + + #[test] + fn a_message_cut_short_is_refused() { + assert!(read_message(&mut Cursor::new("play\n/a.flac\n")).is_none()); + assert!(read_message(&mut Cursor::new("play\n/a.flac\n/b.fl")).is_none()); + let many = format!("play\n{}\n", "/a.flac\n".repeat(MAX_PATHS + 1)); + assert!(read_message(&mut Cursor::new(many)).is_none()); + } + + #[test] + fn a_message_round_trips_and_odd_paths_are_refused() { + let paths = [PathBuf::from("/music/a b.flac"), PathBuf::from("/x/y.mp3")]; + let body = encode(Mode::Play, &paths).unwrap(); + let m = read_message(&mut Cursor::new(body)).unwrap(); + assert_eq!(m.mode, Mode::Play); + assert_eq!(m.paths.len(), 2); + assert!(m.paths[0].ends_with("a b.flac")); + let err = encode(Mode::Play, &[PathBuf::from("/a\nb.flac")]).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + let err = encode(Mode::Play, &[PathBuf::from("/a.flac\r")]).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + } } diff --git a/src/lyrics.rs b/src/lyrics.rs index c978b4e..c56d3b2 100644 --- a/src/lyrics.rs +++ b/src/lyrics.rs @@ -5,12 +5,20 @@ //! deliberate "nothing to show" marker, which the UI turns back into the //! track title, as PotPlayer does. +use std::io::Read; use std::path::Path; +use std::sync::Arc; + +/// The largest sidecar read. A song's lyrics are a few kilobytes; a file +/// far bigger is not lyrics, and is left alone rather than parsed on the +/// UI thread. +const MAX_SIDECAR_BYTES: u64 = 1 << 20; #[derive(Debug, Clone, PartialEq, Eq)] struct Line { at_ms: u64, - text: String, + /// Shared by every timestamp of one LRC line (`[00:02][00:07]chorus`). + text: Arc, } #[derive(Debug, Clone, Default)] @@ -25,17 +33,35 @@ impl Lyrics { audio.file_stem()?; // `with_extension` on the full path: a stem with dots in it // (`01. Intro.flac`) must become `01. Intro.lrc`, not `01.lrc`. - let candidate = ["lrc", "LRC", "Lrc"] - .iter() - .map(|ext| audio.with_extension(ext)) + // Every casing of the extension, for case-sensitive file systems; + // on the others the first one already finds any. + let candidate = (0..8u8) + .map(|mask| { + let ext: String = "lrc" + .chars() + .enumerate() + .map(|(i, c)| { + if mask & (1 << i) != 0 { + c.to_ascii_uppercase() + } else { + c + } + }) + .collect(); + audio.with_extension(ext) + }) .find(|p| p.is_file())?; - let bytes = match std::fs::read(&candidate) { - Ok(b) => b, - Err(e) => { - tracing::warn!(path = %candidate.display(), error = %e, "lrc unreadable"); - return None; - } - }; + let mut bytes = Vec::new(); + let read = std::fs::File::open(&candidate) + .and_then(|f| f.take(MAX_SIDECAR_BYTES + 1).read_to_end(&mut bytes)); + if let Err(e) = read { + tracing::warn!(path = %candidate.display(), error = %e, "lrc unreadable"); + return None; + } + if bytes.len() as u64 > MAX_SIDECAR_BYTES { + tracing::warn!(path = %candidate.display(), max = MAX_SIDECAR_BYTES, "lrc too large, ignored"); + return None; + } let text = String::from_utf8_lossy(bytes.strip_prefix(b"\xEF\xBB\xBF").unwrap_or(&bytes)); let parsed = Self::parse(&text); if let Some(l) = &parsed { @@ -60,30 +86,36 @@ impl Lyrics { if let Some(ms) = parse_stamp(tag) { stamps.push(ms); } else if let Some(v) = tag.strip_prefix("offset:") { - offset_ms = v.trim().parse().unwrap_or(0); + // A malformed one leaves the offset as it was. + match v.trim().parse() { + Ok(ms) => offset_ms = ms, + Err(_) => tracing::debug!(tag, "lrc offset unreadable, ignored"), + } } // Any other `[id:value]` tag is metadata; ignored. } if stamps.is_empty() { continue; } - let text = strip_word_stamps(rest); + let text: Arc = strip_word_stamps(rest).into(); for at_ms in stamps { raw.push(Line { at_ms, - text: text.clone(), + text: Arc::clone(&text), }); } } if raw.is_empty() { return None; } + // In file time first: the offset can bring several lines down to + // zero, and among those the last one in time must stay last. + raw.sort_by_key(|l| l.at_ms); // A positive offset shifts the lyrics earlier (LRC convention). for l in &mut raw { let shifted = i128::from(l.at_ms) - i128::from(offset_ms); l.at_ms = shifted.clamp(0, i128::from(u64::MAX)) as u64; } - raw.sort_by_key(|l| l.at_ms); Some(Self { lines: raw }) } @@ -98,7 +130,7 @@ impl Lyrics { pub fn current(&self, pos_ms: u64) -> Option<(u64, &str)> { let idx = self.lines.partition_point(|l| l.at_ms <= pos_ms); let line = self.lines.get(idx.checked_sub(1)?)?; - (!line.text.is_empty()).then_some((line.at_ms, line.text.as_str())) + (!line.text.is_empty()).then_some((line.at_ms, &*line.text)) } /// When the displayed line next changes after `pos_ms`, if ever. @@ -140,19 +172,28 @@ fn parse_stamp(tag: &str) -> Option { } /// Removes enhanced-LRC `` word stamps and collapses the spaces. +/// One pass: a `<` that opens no stamp is text, and the scan goes on from +/// the next `<` or `>` after it, so stray brackets are never searched past +/// more than once. fn strip_word_stamps(s: &str) -> String { let mut out = String::with_capacity(s.len()); let mut rest = s; while let Some(start) = rest.find('<') { - let (before, after) = rest.split_at(start); - out.push_str(before); - match after[1..].find('>') { - Some(end) if parse_stamp(&after[1..1 + end]).is_some() => { - rest = &after[1 + end + 1..]; + out.push_str(&rest[..start]); + let after = &rest[start + 1..]; + match after.find(['<', '>']) { + Some(end) if after.as_bytes()[end] == b'>' && parse_stamp(&after[..end]).is_some() => { + rest = &after[end + 1..]; } - _ => { + Some(end) => { out.push('<'); - rest = &after[1..]; + out.push_str(&after[..end]); + rest = &after[end..]; + } + None => { + out.push('<'); + rest = after; + break; } } } @@ -179,8 +220,8 @@ mod tests { assert_eq!(l.lines.len(), 5); assert_eq!(l.lines[0].at_ms, 1000); assert_eq!(l.lines[1].at_ms, 2000); - assert_eq!(l.lines[1].text, "repeated"); - assert_eq!(l.lines[3].text, "third line"); + assert_eq!(&*l.lines[1].text, "repeated"); + assert_eq!(&*l.lines[3].text, "third line"); assert_eq!(l.lines[4].at_ms, 7000); } @@ -244,9 +285,50 @@ mod tests { .unwrap(); let l = Lyrics::load_for(&audio).expect("sidecar found"); assert_eq!(l.line_at(1000), Some("hello")); + // Any casing of the extension, on case-sensitive file systems too. + let other = dir.join("02. Outro.flac"); + std::fs::write(&other, b"").unwrap(); + std::fs::write(dir.join("02. Outro.lRc"), "[00:02.00]bye\n").unwrap(); + let l = Lyrics::load_for(&other).expect("mixed-case sidecar found"); + assert_eq!(l.line_at(2000), Some("bye")); let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn a_bad_offset_keeps_the_good_one() { + let l = Lyrics::parse("[offset:500]\n[offset:bad]\n[00:01]a\n").unwrap(); + assert_eq!(l.lines[0].at_ms, 500); + } + + #[test] + fn lines_brought_to_zero_keep_their_order() { + // Out of order in the file; the offset brings both to zero, and the + // later one (b, at 3 s) is the one in force there. + let l = Lyrics::parse("[offset:5000]\n[00:03]b\n[00:01]a\n").unwrap(); + assert_eq!(l.line_at(0), Some("b")); + } + + #[test] + fn stray_brackets_are_text_and_cheap() { + assert_eq!(strip_word_stamps("a < b > c"), "a < b > c"); + assert_eq!(strip_word_stamps("<<00:01.00>x"), " Option { .ok() } +/// What the command line asks for. +#[derive(Debug, Default)] +struct Args { + version: bool, + help: bool, + register: bool, + unregister: bool, + enqueue: bool, + /// Files and folders, kept as the OS gave them: a file name need not + /// be valid Unicode. + files: Vec, + /// Options this version does not know; logged and otherwise ignored. + unknown: Vec, +} + +/// Options are read up to `--`, after which everything is a file, so a +/// file named `--help` can still be opened. Anything else that starts with +/// `--` is an unknown option, not a file. +fn parse_args(args: impl IntoIterator) -> Args { + let mut parsed = Args::default(); + let mut options_done = false; + for arg in args { + if options_done { + parsed.files.push(PathBuf::from(arg)); + continue; + } + match arg.to_str() { + Some("--") => options_done = true, + Some("--version" | "-V") => parsed.version = true, + Some("--help" | "-h") => parsed.help = true, + Some("--register") => parsed.register = true, + Some("--unregister") => parsed.unregister = true, + Some("--enqueue") => parsed.enqueue = true, + Some(other) if other.starts_with("--") => parsed.unknown.push(other.to_string()), + _ => parsed.files.push(PathBuf::from(arg)), + } + } + parsed +} + fn main() -> eframe::Result { init_tracing(); let started = std::time::Instant::now(); - let args: Vec = std::env::args().skip(1).collect(); - if args.iter().any(|a| { - matches!( - a.as_str(), - "--version" | "-V" | "--help" | "-h" | "--register" | "--unregister" - ) - }) { + let args = parse_args(std::env::args_os().skip(1)); + if args.version || args.help || args.register || args.unregister { platform::attach_parent_console(); } - if args.iter().any(|a| a == "--version" || a == "-V") { + for option in &args.unknown { + tracing::warn!(%option, "unknown option ignored"); + } + if args.version { println!("waveflow-quickplay {}", env!("CARGO_PKG_VERSION")); return Ok(()); } // The interface language also names the file-manager verbs. let lang = || i18n::Lang::resolve(&Settings::load().language); - if args.iter().any(|a| a == "--help" || a == "-h") { + if args.help { let t = lang().strings(); - println!("usage: waveflow-quickplay [--enqueue] [FILE|FOLDER]..."); + println!("usage: waveflow-quickplay [--enqueue] [--] [FILE|FOLDER]..."); println!(); println!("A second launch hands its files to the running player and exits."); println!(" --enqueue append to its queue instead of replacing it"); @@ -86,11 +125,13 @@ fn main() -> eframe::Result { println!(" --unregister remove them"); return Ok(()); } - let integration_flag = args - .iter() - .find(|a| *a == "--register" || *a == "--unregister"); - if let Some(flag) = integration_flag { - let run = if flag == "--register" { + if args.register || args.unregister { + let flag = if args.register { + "--register" + } else { + "--unregister" + }; + let run = if args.register { integration::register(lang()) } else { integration::unregister() @@ -106,16 +147,12 @@ fn main() -> eframe::Result { } }; } - let mode = if args.iter().any(|a| a == "--enqueue") { + let mode = if args.enqueue { ipc::Mode::Enqueue } else { ipc::Mode::Play }; - let files: Vec = args - .iter() - .filter(|a| !a.starts_with("--")) - .map(PathBuf::from) - .collect(); + let files = args.files; // Single instance: hand the files to a running player if there is one; // otherwise become the one that listens. Losing the creation race means @@ -134,9 +171,18 @@ fn main() -> eframe::Result { let mut handed = false; for _ in 0..20 { std::thread::sleep(std::time::Duration::from_millis(50)); - if ipc::send(mode, &files).unwrap_or(false) { - handed = true; - break; + match ipc::send(mode, &files) { + Ok(true) => { + handed = true; + break; + } + Ok(false) => {} + // Paths the socket cannot carry, or an instance that + // does not answer: waiting longer does not help. + Err(e) => { + tracing::warn!(error = %e, "hand-off failed"); + break; + } } } if handed { @@ -206,3 +252,38 @@ fn main() -> eframe::Result { }), ) } + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(args: &[&str]) -> Args { + parse_args(args.iter().map(OsString::from)) + } + + #[test] + fn options_end_at_a_double_dash() { + let a = parse(&["--enqueue", "a.flac", "--", "--help", "-V"]); + assert!(a.enqueue && !a.help && !a.version); + assert_eq!( + a.files, + [PathBuf::from("a.flac"), "--help".into(), "-V".into()] + ); + } + + #[test] + fn an_unknown_option_is_not_a_file() { + let a = parse(&["--shuffle", "-x.mp3"]); + assert_eq!(a.unknown, ["--shuffle"]); + assert_eq!(a.files, [PathBuf::from("-x.mp3")]); + } + + #[cfg(unix)] + #[test] + fn a_file_name_need_not_be_unicode() { + use std::os::unix::ffi::OsStringExt; + let name = OsString::from_vec(b"caf\xe9.flac".to_vec()); + let a = parse_args([name.clone()]); + assert_eq!(a.files, [PathBuf::from(name)]); + } +} diff --git a/src/platform.rs b/src/platform.rs index 6ee3b95..a9ae512 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -114,3 +114,94 @@ pub fn attach_parent_console() { #[cfg(not(windows))] pub fn attach_parent_console() {} + +/// Whether process `pid` runs as the same user as this one: the user SIDs +/// of the two process tokens are equal. `false` when the other token +/// cannot be read, as for another user's process, which is the case this +/// tells apart. +#[cfg(windows)] +pub fn same_user_process(pid: u32) -> bool { + use windows_sys::Win32::Foundation::{CloseHandle, HANDLE}; + use windows_sys::Win32::Security::{ + EqualSid, GetTokenInformation, TokenUser, TOKEN_QUERY, TOKEN_USER, + }; + use windows_sys::Win32::System::Threading::{ + GetCurrentProcess, OpenProcess, OpenProcessToken, PROCESS_QUERY_LIMITED_INFORMATION, + }; + + /// The `TOKEN_USER` of `process`, in a buffer aligned for it. + /// + /// SAFETY: `process` is a valid process handle. + unsafe fn token_user(process: HANDLE) -> Option> { + let mut token: HANDLE = std::ptr::null_mut(); + // SAFETY: documented calls; `token` is closed below, the buffer is + // sized from the length the first call reports. + unsafe { + if OpenProcessToken(process, TOKEN_QUERY, &mut token) == 0 { + return None; + } + let mut len = 0u32; + GetTokenInformation(token, TokenUser, std::ptr::null_mut(), 0, &mut len); + let mut buf = vec![0u64; (len as usize).div_ceil(8)]; + let ok = len > 0 + && GetTokenInformation(token, TokenUser, buf.as_mut_ptr().cast(), len, &mut len) + != 0; + CloseHandle(token); + ok.then_some(buf) + } + } + + // SAFETY: the pseudo-handle of this process is always valid; the other + // handle is checked and closed; both buffers start with a TOKEN_USER + // written by GetTokenInformation, whose SIDs point into the buffers. + unsafe { + let Some(mine_buf) = token_user(GetCurrentProcess()) else { + return false; + }; + let process = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, 0, pid); + if process.is_null() { + return false; + } + let theirs = token_user(process); + CloseHandle(process); + let Some(theirs_buf) = theirs else { + return false; + }; + let mine = &*mine_buf.as_ptr().cast::(); + let theirs = &*theirs_buf.as_ptr().cast::(); + EqualSid(mine.User.Sid, theirs.User.Sid) != 0 + } +} + +#[cfg(all(test, windows))] +mod tests { + #[test] + fn this_process_is_ours_the_system_one_is_not() { + assert!(super::same_user_process(std::process::id())); + // PID 4 is the System process, which no user runs. + assert!(!super::same_user_process(4)); + } +} + +/// Bytes waiting to be read in the pipe `handle`, without reading them. +#[cfg(windows)] +pub fn pipe_bytes_available(handle: std::os::windows::io::RawHandle) -> std::io::Result { + use windows_sys::Win32::System::Pipes::PeekNamedPipe; + let mut available = 0u32; + // SAFETY: documented call on a pipe handle the caller holds open; no + // buffer is read into, only the count is written. + let ok = unsafe { + PeekNamedPipe( + handle, + std::ptr::null_mut(), + 0, + std::ptr::null_mut(), + &mut available, + std::ptr::null_mut(), + ) + }; + if ok == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(available) +} diff --git a/src/queue.rs b/src/queue.rs index 5e09c76..f733768 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -3,6 +3,7 @@ //! shuffle). use std::cell::Cell; +use std::collections::HashSet; use std::hash::{BuildHasher, Hasher}; use std::path::{Path, PathBuf}; @@ -50,12 +51,16 @@ fn file_stem(path: &Path) -> &str { pub struct Queue { entries: Vec, current: Option, + /// The current row was removed and the cursor sits on the row that + /// moved up into its place, which has not played: Next plays that row + /// rather than the one after it. + cursor_pending: bool, next_id: u64, repeat: Repeat, shuffle: bool, /// Ids played in the current shuffle round, so every track comes once /// before any comes again. Emptied when the round is complete. - played: Vec, + played: HashSet, /// Ids in the order they were played, for Previous under shuffle. history: Vec, /// xorshift state for the shuffle draws, in a `Cell` so `next_index` @@ -72,10 +77,11 @@ impl Default for Queue { Self { entries: Vec::new(), current: None, + cursor_pending: false, next_id: 0, repeat: Repeat::Off, shuffle: false, - played: Vec::new(), + played: HashSet::new(), history: Vec::new(), rng: Cell::new(seed | 1), } @@ -101,8 +107,14 @@ impl Queue { self.shuffle = shuffle; self.played.clear(); self.history.clear(); - if let Some(id) = self.current().map(|e| e.id) { - self.played.push(id); + // A row only waiting under the cursor has not played: it stays + // free for the round's first draw. + if let Some(id) = self + .current() + .filter(|_| !self.cursor_pending) + .map(|e| e.id) + { + self.played.insert(id); self.history.push(id); } } @@ -119,9 +131,13 @@ impl Queue { .all(|e| self.played.contains(&e.id)) } - /// Playable rows other than the current one. + /// Playable rows other than the current one; a row only waiting under + /// the cursor ([`Self::cursor_pending`]) counts among them. fn others(&self) -> impl Iterator + '_ { - let current_id = self.current().map(|e| e.id); + let current_id = self + .current() + .filter(|_| !self.cursor_pending) + .map(|e| e.id); (0..self.entries.len()) .filter(move |i| self.playable(*i) && Some(self.entries[*i].id) != current_id) } @@ -203,6 +219,7 @@ impl Queue { fn move_cursor(&mut self, index: Option, back: bool) { self.current = index.filter(|i| *i < self.entries.len()); + self.cursor_pending = false; let Some(id) = self.current().map(|e| e.id) else { return; }; @@ -211,17 +228,13 @@ impl Queue { Some(k) => self.history.truncate(k + 1), None => self.history.push(id), } - if !self.played.contains(&id) { - self.played.push(id); - } + self.played.insert(id); return; } if self.round_complete() { self.played.clear(); } - if !self.played.contains(&id) { - self.played.push(id); - } + self.played.insert(id); if self.history.last() != Some(&id) { self.history.push(id); } @@ -255,12 +268,20 @@ impl Queue { self.played.retain(|p| *p != id); match self.current { Some(c) if c == index => { - // The cursor keeps its position so Next plays what moved - // up into it; clamp when the last row went. + // The cursor keeps its position, on the row that moved up + // into it: Play plays that row, and so does Next, unless + // shuffle already played it in this round, in which case + // Next draws among the rows still unplayed. When the last + // row went, nothing moved up: it clamps to the new last + // row, which Play plays again and Next goes past. if self.entries.is_empty() { self.current = None; + self.cursor_pending = false; } else if c >= self.entries.len() { self.current = Some(self.entries.len() - 1); + self.cursor_pending = false; + } else { + self.cursor_pending = true; } true } @@ -275,6 +296,7 @@ impl Queue { pub fn clear(&mut self) { self.entries.clear(); self.current = None; + self.cursor_pending = false; self.played.clear(); self.history.clear(); } @@ -301,7 +323,11 @@ impl Queue { } fn sequential_next(&self) -> Option { - let start = self.current.map_or(0, |c| c + 1); + let start = match self.current { + Some(c) if self.cursor_pending => c, + Some(c) => c + 1, + None => 0, + }; let after = (start..self.entries.len()).find(|i| self.playable(*i)); match (after, self.repeat) { (Some(i), _) => Some(i), @@ -315,6 +341,22 @@ impl Queue { /// shuffle played before. pub fn prev_index(&self) -> Option { let end = self.current?; + if self.shuffle && self.cursor_pending { + // The row under the cursor is not the one that last played: + // back is the last track that did, other than that row, which + // the history holds too when shuffle played it earlier. + let under = self.entries[end].id; + return self + .history + .iter() + .rev() + .filter(|h| **h != under) + .find_map(|h| { + self.entries + .iter() + .position(|e| e.id == *h && e.failed.is_none()) + }); + } if self.shuffle { let id = self.entries[end].id; let at = self.history.iter().rposition(|h| *h == id)?; @@ -327,7 +369,7 @@ impl Queue { let before = (0..end).rev().find(|i| self.playable(*i)); match (before, self.repeat) { (Some(i), _) => Some(i), - (None, Repeat::All) => (end + 1..self.entries.len()) + (None, Repeat::All) => (end + usize::from(!self.cursor_pending)..self.entries.len()) .rev() .find(|i| self.playable(*i)), (None, _) => None, @@ -371,6 +413,61 @@ mod tests { assert_eq!(q.current_index(), None); } + #[test] + fn next_after_removing_the_current_row_plays_what_moved_up() { + let mut q = queue_of(3); + q.set_current(Some(0)); + assert!(q.remove(0)); + assert_eq!(q.next_index(), Some(0), "\"1\" moved up and comes next"); + assert_eq!(q.get(0).map(|e| e.title()), Some("1")); + let mut two = queue_of(2); + two.set_current(Some(0)); + two.remove(0); + assert!(two.has_next()); + assert_eq!(two.next_index(), Some(0)); + // Once it plays, the cursor is an ordinary one again. + q.set_current(Some(0)); + assert_eq!(q.next_index(), Some(1)); + // A track that survives keeps the usual next. + let mut q = queue_of(3); + q.set_current(Some(1)); + q.remove(0); + assert_eq!(q.next_index(), Some(1)); + } + + #[test] + fn under_shuffle_the_row_that_moved_up_can_be_drawn() { + let mut q = queue_of(2); + q.set_current(Some(0)); + q.set_shuffle(true); + q.remove(0); + assert!(q.has_next()); + assert_eq!(q.next_index(), Some(0)); + assert_eq!(q.prev_index(), None, "nothing that played is left"); + // Shuffle switched on after the removal: the row is still unplayed. + let mut q = queue_of(2); + q.set_current(Some(0)); + q.remove(0); + q.set_shuffle(true); + assert_eq!(q.next_index(), Some(0)); + assert_eq!(q.prev_index(), None); + } + + /// The row that moved up had already played in the round: Previous + /// goes to the track before, not to that row itself. + #[test] + fn previous_after_removal_skips_the_row_under_the_cursor() { + let mut q = queue_of(3); + q.set_current(Some(0)); + q.set_shuffle(true); + q.set_current(Some(2)); + q.set_current(Some(1)); + // History 0, 2, 1; removing 1 brings 2 under the cursor. + assert!(q.remove(1)); + assert_eq!(q.current().map(|e| e.title()), Some("2")); + assert_eq!(q.prev_index(), Some(0)); + } + #[test] fn next_and_prev_skip_failed_rows() { let mut q = queue_of(4); diff --git a/src/ui/playlist.rs b/src/ui/playlist.rs index d6f7981..0c8c52e 100644 --- a/src/ui/playlist.rs +++ b/src/ui/playlist.rs @@ -134,12 +134,16 @@ pub fn show( return; } let current = queue.current_index(); + // Before `show_rows`, which works out the visible range and the + // scroll height from the spacing in force when it is called: + // set inside, rows were laid out 30 points apart while it + // counted 34. + ui.spacing_mut().item_spacing = vec2(0.0, 0.0); egui::ScrollArea::vertical().auto_shrink(false).show_rows( ui, ROW_HEIGHT, queue.len(), |ui, range| { - ui.spacing_mut().item_spacing = vec2(0.0, 0.0); for index in range { let Some(entry) = queue.get(index) else { continue; diff --git a/src/ui/settings.rs b/src/ui/settings.rs index af85018..d9f06e4 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -31,8 +31,27 @@ pub fn show(ui: &mut Ui, t: &'static Strings, language: &str, actions: &mut Vec< actions.push(Action::ToggleSettings); } ui.add_space(4.0); - language_picker(ui, t, language, actions); - ui.label(RichText::new(t.language).size(12.0).color(theme::TEXT_DIM)); + // What is left between the title and the close button. The + // "Language" label goes first when the bar is narrow (320 + // points fit no more than the picker), then the picker narrows + // and truncates its text. + let label = RichText::new(t.language).size(12.0).color(theme::TEXT_DIM); + let label_width = ui + .painter() + .layout_no_wrap( + t.language.to_string(), + egui::FontId::proportional(12.0), + theme::TEXT_DIM, + ) + .size() + .x; + let room = ui.available_width() - ui.spacing().item_spacing.x; + let with_label = room >= PICKER_WIDTH + ui.spacing().item_spacing.x + label_width; + let width = room.clamp(PICKER_MIN_WIDTH, PICKER_WIDTH); + language_picker(ui, t, language, width, actions); + if with_label { + ui.label(label); + } }); }); @@ -90,25 +109,46 @@ pub fn show(ui: &mut Ui, t: &'static Strings, language: &str, actions: &mut Vec< }); } -/// One combo: the system entry first, then every language under its own -/// name. Picking one pushes [`Action::SetLanguage`]. -fn language_picker(ui: &mut Ui, t: &Strings, language: &str, actions: &mut Vec) { +/// The language picker's width when there is room, and the least it +/// narrows to. +const PICKER_WIDTH: f32 = 190.0; +const PICKER_MIN_WIDTH: f32 = 110.0; + +/// One combo, `width` points wide: the system entry first, then every +/// language under its own name. Picking one pushes [`Action::SetLanguage`]. +fn language_picker( + ui: &mut Ui, + t: &Strings, + language: &str, + width: f32, + actions: &mut Vec, +) { let selected = Lang::from_code(language); let system_label = fill(t.language_system, &[("l", Lang::system().native_name())]); let current = selected.map_or_else(|| system_label.clone(), |l| l.native_name().to_string()); - egui::ComboBox::from_id_salt("language") - .selected_text(RichText::new(current).size(12.0)) - .width(190.0) - .height(96.0) - .show_ui(ui, |ui| { - let mut pick = |ui: &mut Ui, code: &str, label: &str, is_selected: bool| { - if ui.selectable_label(is_selected, label).clicked() && !is_selected { - actions.push(Action::SetLanguage(code.to_string())); - } - }; - pick(ui, "", &system_label, selected.is_none()); - for lang in Lang::ALL { - pick(ui, lang.code(), lang.native_name(), selected == Some(lang)); - } - }); + // In a box of its own `width`, against which the selected text + // truncates: a long system entry does not widen the combo. + let height = ui.spacing().interact_size.y; + ui.allocate_ui_with_layout( + vec2(width, height), + Layout::right_to_left(Align::Center), + |ui| { + egui::ComboBox::from_id_salt("language") + .selected_text(RichText::new(current).size(12.0)) + .width(width) + .wrap_mode(egui::TextWrapMode::Truncate) + .height(96.0) + .show_ui(ui, |ui| { + let mut pick = |ui: &mut Ui, code: &str, label: &str, is_selected: bool| { + if ui.selectable_label(is_selected, label).clicked() && !is_selected { + actions.push(Action::SetLanguage(code.to_string())); + } + }; + pick(ui, "", &system_label, selected.is_none()); + for lang in Lang::ALL { + pick(ui, lang.code(), lang.native_name(), selected == Some(lang)); + } + }); + }, + ); } diff --git a/src/ui/theme.rs b/src/ui/theme.rs index d18329f..1530e2c 100644 --- a/src/ui/theme.rs +++ b/src/ui/theme.rs @@ -74,10 +74,19 @@ pub fn install_fonts(ctx: &egui::Context) { BOLD_FAMILY.into(), Arc::new(FontData::from_static(INTER_SEMIBOLD)), ); - if let Some((path, bytes)) = SYSTEM_FALLBACK - .iter() - .find_map(|p| std::fs::read(p).ok().map(|b| (p.to_string(), b))) - { + // The first candidate that reads *and* parses: egui panics at start-up + // on font data it cannot parse, so a damaged or empty file is passed + // over for the next one, and the bundled fonts carry on without any. + if let Some((path, bytes)) = SYSTEM_FALLBACK.iter().find_map(|p| { + let bytes = std::fs::read(p).ok()?; + match skrifa::FontRef::from_index(&bytes, 0) { + Ok(_) => Some((p.to_string(), bytes)), + Err(err) => { + tracing::warn!(path = %p, %err, "system fallback font unreadable, skipped"); + None + } + } + }) { tracing::debug!(%path, "system fallback font"); fonts.font_data.insert( "system-fallback".into(), diff --git a/src/waveflow.rs b/src/waveflow.rs index 4ff9aba..5ec9f4a 100644 --- a/src/waveflow.rs +++ b/src/waveflow.rs @@ -173,9 +173,21 @@ impl Version { } else { rest.split(['.', '-']) .filter(|p| !p.is_empty()) - .map(|p| match p.parse() { - Ok(n) => Identifier::Number(n), - Err(_) => Identifier::Word(p.to_ascii_lowercase()), + .flat_map(|p| { + // A label run into its number (`beta1`, `rc10`, the + // compact AUR spelling) is the label then the number, + // so `beta0` stays before `beta.1` and `rc10` after + // `rc2`. + let digits = p.len() - p.trim_end_matches(|c: char| c.is_ascii_digit()).len(); + let (word, number) = p.split_at(p.len() - digits); + let compact = !word.is_empty() + && digits > 0 + && word.bytes().all(|b| b.is_ascii_alphabetic()); + let parts = if compact { vec![word, number] } else { vec![p] }; + parts.into_iter().map(|part| match part.parse() { + Ok(n) => Identifier::Number(n), + Err(_) => Identifier::Word(part.to_ascii_lowercase()), + }) }) .collect() }; @@ -425,6 +437,7 @@ mod platform { #[cfg(all(unix, not(target_os = "macos")))] mod platform { + use std::path::Path; use std::process::{Command, Stdio}; use super::Install; @@ -433,8 +446,9 @@ mod platform { /// put it. A Flatpak install exports `app.waveflow.WaveFlow` instead, /// so it is not found, which is what we want: it cannot read the /// handoff file. A binary on `PATH` carries no readable version, so it - /// comes from the package manager that installed it; a binary none of - /// them knows (an AppImage put there by hand) has none. + /// comes from the package manager that installed *that binary*; one no + /// package owns (an AppImage or an old build put earlier on `PATH` by + /// hand) has none, even when a package is installed elsewhere. pub fn locate() -> Option { use std::os::unix::fs::PermissionsExt; let path = std::env::var_os("PATH")?; @@ -444,25 +458,52 @@ mod platform { std::fs::metadata(exe) .is_ok_and(|m| m.is_file() && m.permissions().mode() & 0o111 != 0) })?; - Some(Install { - exe, - version: package_version(), - }) + let version = package_version(&exe); + Some(Install { exe, version }) } - fn package_version() -> Option { + fn package_version(exe: &Path) -> Option { + // The path as found and the file it resolves to: `/bin` is a link + // to `/usr/bin` on merged-/usr systems, and a package may install + // the binary elsewhere with a link on PATH. + let mut paths = vec![exe.to_path_buf()]; + if let Ok(real) = exe.canonicalize() { + if real != exe { + paths.push(real); + } + } + let owned_by = |program: &str, args: &[&str], package: &str| { + paths.iter().any(|p| { + let Some(p) = p.to_str() else { + return false; + }; + let mut all = args.to_vec(); + all.push(p); + // dpkg answers `wave-flow: /usr/bin/waveflow` (with an + // `:arch` on multi-arch), rpm and pacman the bare name. + run(program, &all).is_some_and(|out| { + out.lines() + .any(|line| line.split(':').next().map(str::trim) == Some(package)) + }) + }) + }; // Debian and apt: the package is `wave-flow`. A Debian revision // (`2.0.0-1`) would otherwise read as a pre-release. - run("dpkg-query", &["-W", "-f=${Version}", "wave-flow"]) - .map(|v| super::strip_package_release(&v).to_string()) - // Fedora and COPR: `%{VERSION}` carries no release. - .or_else(|| run("rpm", &["-q", "--qf", "%{VERSION}", "waveflow"])) - // AUR: `waveflow-bin 2.0.0-1`, the version then the package - // release. - .or_else(|| { - let line = run("pacman", &["-Q", "waveflow-bin"])?; - Some(super::strip_package_release(line.split_whitespace().nth(1)?).to_string()) - }) + if owned_by("dpkg-query", &["-S"], "wave-flow") { + return run("dpkg-query", &["-W", "-f=${Version}", "wave-flow"]) + .map(|v| super::strip_package_release(&v).to_string()); + } + // Fedora and COPR: `%{VERSION}` carries no release. + if owned_by("rpm", &["-qf", "--qf", "%{NAME}\n"], "waveflow") { + return run("rpm", &["-q", "--qf", "%{VERSION}", "waveflow"]); + } + // AUR: `waveflow-bin 2.0.0-1`, the version then the package + // release. + if owned_by("pacman", &["-Qqo"], "waveflow-bin") { + let line = run("pacman", &["-Q", "waveflow-bin"])?; + return Some(super::strip_package_release(line.split_whitespace().nth(1)?).to_string()); + } + None } /// The trimmed output of a command that succeeded with some. @@ -483,9 +524,15 @@ mod platform { /// the last dash. A revision starts with a digit (`1`, `1+b1`, /// `0ubuntu1`), a pre-release label with a letter (`beta.1`), so /// `2.0.0-beta.1` is left whole and `2.0.0-alpha.1` does not become a -/// release. +/// release. A Debian epoch (`1:2.0.0-1`) comes off the front. #[cfg_attr(not(all(unix, not(target_os = "macos"))), allow(dead_code))] fn strip_package_release(version: &str) -> &str { + let version = match version.split_once(':') { + Some((epoch, rest)) if !epoch.is_empty() && epoch.bytes().all(|b| b.is_ascii_digit()) => { + rest + } + _ => version, + }; match version.rsplit_once('-') { Some((head, release)) if release.starts_with(|c: char| c.is_ascii_digit()) => head, _ => version, @@ -523,6 +570,16 @@ mod tests { assert!(takes_handoff("2.0.0~beta.1")); assert!(!takes_handoff("2.0.0~alpha.1")); assert!(takes_handoff("2.0.0beta.1")); + // The compact spelling, label and number run together. + assert!(!takes_handoff("2.0.0beta0")); + assert!(!takes_handoff("2.0.0-beta0")); + assert!(takes_handoff("2.0.0beta1")); + assert!(takes_handoff("2.0.0beta10")); + assert!(Version::parse("2.0.0rc10") > Version::parse("2.0.0rc2")); + // A Debian epoch. + assert_eq!(strip_package_release("1:2.0.0-1"), "2.0.0"); + assert!(takes_handoff(strip_package_release("1:2.0.0-1"))); + assert!(!takes_handoff(strip_package_release("1:1.9.7-1"))); assert!(takes_handoff("v2.0.0")); assert!(takes_handoff("2.0.0-beta.1+build.5")); assert_eq!(strip_package_release("2.0.0-1"), "2.0.0");