diff --git a/CHANGELOG.md b/CHANGELOG.md index dcc0c17f..8e468f8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### yabridgectl +- Added a check to `yabridgectl sync` to verify that the currently installed + versions of Wine and yabridge are compatible. This check will only be + performed once for a given combination of Wine and yabridge versions. + + TODO: Remove the 'starting from yabridge 1.3.1' from the note about this in + the troubleshooting section of the readme. + - Added a `--no-verify` option to `yabridgectl sync` to skip the post-installation setup checks. This option will skip both the login shell search path check when using the copy-based installation method as well as the diff --git a/README.md b/README.md index 3c4a7e88..b9205ea0 100644 --- a/README.md +++ b/README.md @@ -272,6 +272,10 @@ override the Wine prefix for all instances of yabridge. you'll have to upgrade your Wine version. Instructions for how to do this on Ubuntu can be found on the [WineHQ website](https://wiki.winehq.org/Ubuntu). + Note that starting from yabridge 1.3.1 this check is also performed + automatically whenever you run `yabridgectl sync` using a new version of Wine + or yabridge. + - Sometimes left over Wine processes can cause problems. Run `wineserver -k` to terminate Wine related in the current or default Wine prefix. diff --git a/tools/yabridgectl/Cargo.lock b/tools/yabridgectl/Cargo.lock index f1438c80..949447bb 100644 --- a/tools/yabridgectl/Cargo.lock +++ b/tools/yabridgectl/Cargo.lock @@ -549,6 +549,26 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "thiserror" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dfdd070ccd8ccb78f4ad66bf1982dc37f620ef696c6b5028fe2ed83dd3d0d08" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd80fc12f73063ac132ac92aceea36734f04a1d93c1240c6944e23a3b8841793" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "toml" version = "0.5.6" @@ -617,6 +637,16 @@ version = "0.9.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" +[[package]] +name = "which" +version = "4.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5fe1a9cb33fe7cf77d431070d0223e544b1e4e7f7764bad0a3e691a6678a131" +dependencies = [ + "libc", + "thiserror", +] + [[package]] name = "winapi" version = "0.2.8" @@ -682,5 +712,6 @@ dependencies = [ "textwrap", "toml", "walkdir", + "which", "xdg", ] diff --git a/tools/yabridgectl/Cargo.toml b/tools/yabridgectl/Cargo.toml index c9ed5ce4..d5f102f9 100644 --- a/tools/yabridgectl/Cargo.toml +++ b/tools/yabridgectl/Cargo.toml @@ -22,4 +22,5 @@ serde_derive = "1.0.114" textwrap = "*" toml = "0.5.6" walkdir = "2.3.1" +which = "4.0.1" xdg = "2.2.0" diff --git a/tools/yabridgectl/src/actions.rs b/tools/yabridgectl/src/actions.rs index 027e8c5f..b74aabe1 100644 --- a/tools/yabridgectl/src/actions.rs +++ b/tools/yabridgectl/src/actions.rs @@ -24,7 +24,7 @@ use crate::config::{Config, InstallationMethod}; use crate::files; use crate::files::FoundFile; use crate::utils; -use crate::utils::{verify_path_setup, wrap}; +use crate::utils::{verify_path_setup, verify_wine_setup, wrap}; /// Add a direcotry to the plugin locations. Duplicates get ignord because we're using ordered sets. pub fn add_directory(config: &mut Config, path: PathBuf) -> Result<()> { @@ -151,7 +151,7 @@ pub struct SyncOptions { /// Set up yabridge for all Windows VST2 plugins in the plugin directories. Will also remove orphan /// `.so` files if the prune option is set. -pub fn do_sync(config: &Config, options: &SyncOptions) -> Result<()> { +pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { let libyabridge_path = config.libyabridge()?; println!("Using '{}'\n", libyabridge_path.display()); @@ -243,8 +243,10 @@ pub fn do_sync(config: &Config, options: &SyncOptions) -> Result<()> { } if config.method == InstallationMethod::Copy { + // TODO: Move this warning to `verify_path_setup()` just like we're doign with + // `verify_wine_setup()` if let Err(shell_name) = verify_path_setup() { - println!( + eprintln!( "\n{}", wrap(&format!( "Warning: 'yabridge-host.exe' is not present in your login shell's search \ @@ -264,5 +266,8 @@ pub fn do_sync(config: &Config, options: &SyncOptions) -> Result<()> { } } + // This check is only performed once per combination of Wine and yabridge versions + verify_wine_setup(config)?; + Ok(()) } diff --git a/tools/yabridgectl/src/config.rs b/tools/yabridgectl/src/config.rs index f701df2f..8f56ed10 100644 --- a/tools/yabridgectl/src/config.rs +++ b/tools/yabridgectl/src/config.rs @@ -23,6 +23,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Display; use std::fs; use std::path::{Path, PathBuf}; +use which::which; use xdg::BaseDirectories; use crate::files::{self, SearchResults}; @@ -35,6 +36,8 @@ const YABRIDGECTL_PREFIX: &str = "yabridgectl"; /// The name of the library file we're searching for. const LIBYABRIDGE_NAME: &str = "libyabridge.so"; +/// The name of the script we're going to run to verify that everything's working correctly. +const YABRIDGE_HOST_EXE_NAME: &str = "yabridge-host.exe"; /// The name of the XDG base directory prefix for yabridge's own files, relative to /// `$XDG_CONFIG_HOME` and `$XDG_DATA_HOME`. const YABRIDGE_PREFIX: &str = "yabridge"; @@ -92,20 +95,20 @@ impl Display for InstallationMethod { } } -/// Stores information about a combination of yabridge and Wine that works together properly. -/// Whenever we encounter a new version of yabridge or Wine, we'll check whether `yabridge-host.exe` +/// Stores information about a combination of Wine and yabridge that works together properly. +/// Whenever we encounter a new version of Wine or yabridge, we'll check whether `yabridge-host.exe` /// can run without issues. This is needed because older versions of Wine won't be able to run newer /// winelibs, and Ubuntu ships with old versions of Wine. To prevent repeating unnecessarily /// repeating this check we'll keep track of the last combination of Wine and yabridge that would /// work together properly. -#[derive(Deserialize, Serialize, Debug)] +#[derive(Deserialize, Serialize, Debug, PartialEq, Eq)] pub struct KnownConfig { /// The output of `wine --version`, minus the trailing newline. - wine_version: String, + pub wine_version: String, /// The results from running the contents of `yabridge-host.exe.so` through /// [`DefaultHasher`](std::collections::hash_map::DefaultHasher). Hash collisions aren't really /// an issue here since we mostly care about the version of Wine. - yabridge_host_hash: u64, + pub yabridge_host_hash: u64, } impl Config { @@ -188,6 +191,21 @@ impl Config { } } + /// Return the path to `yabridge-host.exe`, or a descriptive error if it can't be found. This + /// will first search alongside `libyabridge.so` and then search through the search path. + pub fn yabridge_host_exe(&self) -> Result { + let libyabridge_path = self.libyabridge()?; + + let yabridge_host_exe_candidate = libyabridge_path.with_file_name(YABRIDGE_HOST_EXE_NAME); + if yabridge_host_exe_candidate.exists() { + return Ok(yabridge_host_exe_candidate); + } + + // Normally we wouldn't need the full absolute path to `yabridge-host.exe`, but it's useful + // for the error messages + Ok(which(YABRIDGE_HOST_EXE_NAME)?) + } + /// Search for VST2 plugins in all of the registered plugins directories. This will return an /// error if `winedump` could not be called. pub fn index_directories(&self) -> Result> { diff --git a/tools/yabridgectl/src/main.rs b/tools/yabridgectl/src/main.rs index f4b1449a..840fc4a0 100644 --- a/tools/yabridgectl/src/main.rs +++ b/tools/yabridgectl/src/main.rs @@ -140,7 +140,7 @@ fn main() -> Result<()> { }, ), ("sync", Some(options)) => actions::do_sync( - &config, + &mut config, &actions::SyncOptions { no_verify: options.is_present("no-verify"), prune: options.is_present("prune"), diff --git a/tools/yabridgectl/src/utils.rs b/tools/yabridgectl/src/utils.rs index 95219b10..8be0b26a 100644 --- a/tools/yabridgectl/src/utils.rs +++ b/tools/yabridgectl/src/utils.rs @@ -18,14 +18,23 @@ use anyhow::{Context, Result}; use colored::Colorize; +use std::collections::hash_map::DefaultHasher; use std::env; use std::fs; +use std::hash::Hasher; use std::os::unix::fs as unix_fs; use std::os::unix::process::CommandExt; use std::path::Path; use std::process::{Command, Stdio}; use textwrap::Wrapper; +use crate::config::{Config, KnownConfig}; + +/// (Part of) the expected output when running `yabridge-host.exe`. Used to verify that everything's +/// working correctly. +const YABRIDGE_HOST_EXPECTED_OUTPUT: &str = + "Usage: yabridge-host.exe "; + /// Wrapper around [`std::fs::copy()`](std::fs::copy) with a human readable error message. pub fn copy, Q: AsRef>(from: P, to: Q) -> Result { fs::copy(&from, &to).with_context(|| { @@ -139,6 +148,111 @@ pub fn verify_path_setup() -> Result<(), String> { } } +/// Verify that the installed versions of Wine and yabridge will work together properly. This check +/// is only performed once per combination of Wine and yabridge, and we'll update the config with +/// the versions we just tested if the check succeeds. Will return `Err` values if either Wine or +/// `yabridge-host.exe` can't be run. +pub fn verify_wine_setup(config: &mut Config) -> Result<()> { + // These winelib scripts respect `$WINELOADER`, so we'll do the same thing + let wine_binary = env::var("WINELOADER").unwrap_or_else(|_| String::from("wine")); + let wine_version_output = Command::new(&wine_binary) + .arg("--version") + .output() + .with_context(|| { + format!( + "Could not run '{}', make sure Wine is installed", + wine_binary + ) + })? + .stdout; + // Strip the trailing newline just to make the config file a bit neater + let mut wine_version = String::from_utf8(wine_version_output)?; + wine_version.pop().unwrap(); + + let yabridge_host_exe_path = config + .yabridge_host_exe() + .context("Could not find 'yabridge-host.exe")?; + + // Hash the contents of `yabridge-host.exe.so` since `yabridge-host.exe` is only a Wine + // generated shell script + let yabridge_host_exe_so_path = yabridge_host_exe_path.with_extension("exe.so"); + let mut hasher = DefaultHasher::new(); + hasher.write(&fs::read(&yabridge_host_exe_so_path).with_context(|| { + format!( + "Could not read contents of '{}'", + yabridge_host_exe_so_path.display() + ) + })?); + let yabridge_host_hash = hasher.finish(); + + // Since these checks can take over a second if wineserver isn't already running we'll only + // perform them when something has changed + let current_config = KnownConfig { + wine_version: wine_version.clone(), + yabridge_host_hash, + }; + if config.last_known_config.as_ref() == Some(¤t_config) { + return Ok(()); + } + + // If everything's + let output = Command::new(&yabridge_host_exe_path) + .output() + .with_context(|| format!("Could not run '{}'", yabridge_host_exe_path.display()))?; + let stderr = String::from_utf8(output.stderr)?; + + // There are three scenarios here: + // - Either everything is fine and we'll see the usage string being printed + // - Or the used version of Wine is too old and we'll see some line starting with + // `002b:err:module:__wine_process_init` + // - Or the used version of Wine is much newer than what was used to compile yabridge with + // + // I don't know if it's possible to differentiate between the second and the third case, so + // we'll always assume it's Wine that's outdated. + let mut success = false; + let mut last_error: Option<&str> = None; + for line in stderr.lines() { + if line == YABRIDGE_HOST_EXPECTED_OUTPUT { + success = true; + break; + } + + // Ignore fixme messages here, since those can be produced by wineserver even after the + // application has errored out + if &line[5..10] != "fixme" { + last_error = Some(line); + } + } + + if success { + config.last_known_config = Some(current_config); + config.write()?; + } else { + eprintln!( + "\n{}", + wrap(&format!( + "Warning: Could not run 'yabridge-host.exe'. Wine reported the following error: \n\ + \n\ + {}\n\ + \n\ + This can happen when using a version of Wine that is much older than the version \ + that has been used to compile yabridge with. Your current Wine version is '{}'. \ + See the troubleshooting section of the readme for more information on how to \ + upgrade your installation of Wine.\n\ + \n\ + https://github.com/robbert-vdh/yabridge#troubleshooting-common-issues", + last_error.unwrap_or("").bright_white(), + wine_version + .strip_prefix("wine-") + .unwrap_or(&wine_version) + .bright_white(), + )) + ) + } + + Ok(()) +} + /// Wrap a long paragraph of text to terminal width, or 80 characters if the width of the terminal /// can't be determined. Everything after the first line gets indented with four spaces. pub fn wrap(text: &str) -> String {