Check for Wine and yabridge compatibility on sync

This will probably solve the last common setup issue. Starting Wine can
take a second so this check is only repeated when either Wine or
yabridge get updated.

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

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.
 

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",
 ]

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"

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(())
 }

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.
+/// Stores information about a combination of Wine and yabridge that works together properly.
-/// Whenever we encounter a new version of yabridge or Wine, we'll check whether `yabridge-host.exe`
+/// 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<PathBuf> {
+        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<BTreeMap<&Path, SearchResults>> {

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"),

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 <vst_plugin_dll> <unix_domain_socket>";
+
 /// Wrapper around [`std::fs::copy()`](std::fs::copy) with a human readable error message.
 pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<u64> {
     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(&current_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("<no_output>").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 {