From f62d06e0855b6ba8ad6dd135cb249fa3aa9a25e7 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 16 Apr 2022 21:00:56 +0200 Subject: [PATCH] [yabridgectl] Remove symlink installation method --- CHANGELOG.md | 2 + tools/yabridgectl/src/actions.rs | 73 ++++++++------------------------ tools/yabridgectl/src/config.rs | 54 +---------------------- tools/yabridgectl/src/main.rs | 24 ----------- tools/yabridgectl/src/utils.rs | 2 +- 5 files changed, 21 insertions(+), 134 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a34150d..8415590e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,8 @@ Versioning](https://semver.org/spec/v2.0.0.html). once after updating to yabridge 4.0, yabridge can now be updated without needing to rerun `yabridgectl sync`. This is particularly useful when using a distro packaged version of yabridge. +- The previously deprecated symlink installation method has now been removed + from yabridgectl, along with the `yabridgectl set --method` option. - `yabridgectl status` now lists the architecture of `libyabridge-chainloader-vst2.so` just like it already did for the VST3 library. diff --git a/tools/yabridgectl/src/actions.rs b/tools/yabridgectl/src/actions.rs index 4c5e96e0..1d18d60f 100644 --- a/tools/yabridgectl/src/actions.rs +++ b/tools/yabridgectl/src/actions.rs @@ -23,7 +23,7 @@ use std::fs; use std::path::{Path, PathBuf}; use walkdir::WalkDir; -use crate::config::{yabridge_vst3_home, Config, InstallationMethod, YabridgeFiles}; +use crate::config::{yabridge_vst3_home, Config, YabridgeFiles}; use crate::files::{self, NativeFile, Plugin, Vst2Plugin}; use crate::utils::{self, get_file_type}; use crate::utils::{verify_path_setup, verify_wine_setup}; @@ -139,12 +139,6 @@ pub fn show_status(config: &Config) -> Result<()> { } } - // Since the symlink-based installation method is deprecated, don't even mention installation - // methods if it's currently set to copies - if config.method != InstallationMethod::Copy { - println!("\ninstallation method: {}", config.method) - } - for (path, search_results) in results { // Always print these paths with trailing slashes for consistency's sake because paths can // be added both with and without a trailing slash @@ -188,8 +182,7 @@ pub fn show_status(config: &Config) -> Result<()> { } /// Options passed to `yabridgectl set`, see `main()` for the definitions of these options. -pub struct SetOptions<'a> { - pub method: Option<&'a str>, +pub struct SetOptions { pub path: Option, pub path_auto: bool, pub no_verify: Option, @@ -197,13 +190,6 @@ pub struct SetOptions<'a> { /// Change configuration settings. The actual options are defined in the clap [app](clap::App). pub fn set_settings(config: &mut Config, options: &SetOptions) -> Result<()> { - match options.method { - Some("copy") => config.method = InstallationMethod::Copy, - Some("symlink") => config.method = InstallationMethod::Symlink, - Some(s) => unimplemented!("Unexpected installation method '{}'", s), - None => (), - } - if let Some(path) = &options.path { config.yabridge_home = Some(path.clone()); } @@ -237,20 +223,6 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { None => None, }; - if config.method == InstallationMethod::Symlink { - eprintln!( - "{}", - utils::wrap(&format!( - "{}: The symlink-based installation method is currently active. This \ - will likely result in unexpected behavior, and the feature will be removed \ - entirely in a later version of yabridgectl. You can revert back \ - to the copy-based installation method by running '{}'.\n", - "WARNING".yellow().bold(), - "yabridgectl set --method=copy".bright_white() - )) - ); - } - if let Some((vst3_chainloader_path, _)) = &files.vst3_chainloader { println!("Setting up VST2 and VST3 plugins using:"); println!("- {}", files.vst2_chainloader.display()); @@ -307,19 +279,13 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { path: plugin_path, .. }) => { let target_path = plugin_path.with_extension("so"); - let normalized_target_path = if config.method == InstallationMethod::Symlink { - // We should probably remove the symlink option altogether, but the count - // will at least be soemwhat correct-ish this way - target_path.clone() - } else { - utils::normalize_path(&target_path) - }; + let normalized_target_path = utils::normalize_path(&target_path); // Since we skip some files, we'll also keep track of how many new file we've // actually set up if install_file( options.force, - config.method, + InstallationMethod::Copy, &files.vst2_chainloader, Some(vst2_chainloader_hash), &target_path, @@ -342,13 +308,7 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { let target_native_module_path = module.target_native_module_path(Some(&files)); let target_windows_module_path = module.target_windows_module_path(); let normalized_native_module_path = - if config.method == InstallationMethod::Symlink { - // We should probably remove the symlink option altogether, but the - // count will at least be soemwhat correct-ish this way - target_native_module_path.clone() - } else { - utils::normalize_path(&target_native_module_path) - }; + utils::normalize_path(&target_native_module_path); // 32-bit and 64-bit versions of the plugin can live inside of the same bundle), // but it's not possible to use the exact same plugin from multiple Wine @@ -379,7 +339,7 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { utils::create_dir_all(target_native_module_path.parent().unwrap())?; if install_file( options.force, - config.method, + InstallationMethod::Copy, &files.vst3_chainloader.as_ref().unwrap().0, vst3_chainloader_hash, &target_native_module_path, @@ -541,13 +501,8 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { // Don't mind the ugly format string, the existence of the symlink-based installation method // should be hidden as much as possible until it gets removed in yabridge 4.0 println!( - "Finished setting up {} plugins{} ({} new), skipped {} non-plugin .dll files", + "Finished setting up {} plugins ({} new), skipped {} non-plugin .dll files", managed_plugins.len(), - if config.method == InstallationMethod::Copy { - String::new() - } else { - format!(" using {}", config.method.plural_name()) - }, new_plugins.len(), num_skipped_files ); @@ -559,10 +514,9 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { } // The path setup is to make sure that the `libyabridge-chainloader-{vst2,vst3}.so` copies can - // find `yabridge-host.exe` - if config.method == InstallationMethod::Copy { - verify_path_setup(config)?; - } + // find `yabridge-host.exe` and by extension the plugin libraries. That last part should already + // be the case if we get to this point though. + verify_path_setup(config)?; // This check is only performed once per combination of Wine and yabridge versions verify_wine_setup(config)?; @@ -570,6 +524,13 @@ pub fn do_sync(config: &mut Config, options: &SyncOptions) -> Result<()> { Ok(()) } +// TODO: Clean this up, in the past this was part of a yabridgectl setting and the enum was simply +// reused here +enum InstallationMethod { + Copy, + Symlink, +} + /// Create a copy or symlink of `from` to `to`. Depending on `force`, we might not actually create a /// new copy or symlink if `to` matches `from_hash`. fn install_file( diff --git a/tools/yabridgectl/src/config.rs b/tools/yabridgectl/src/config.rs index d517e53f..92b88646 100644 --- a/tools/yabridgectl/src/config.rs +++ b/tools/yabridgectl/src/config.rs @@ -21,7 +21,6 @@ use rayon::prelude::*; use serde_derive::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::env; -use std::fmt::Display; use std::fs; use std::path::{Path, PathBuf}; use which::which; @@ -57,12 +56,9 @@ const YABRIDGE_VST3_HOME: &str = ".vst3/yabridge"; /// The configuration used for yabridgectl. This will be serialized to and deserialized from /// `$XDG_CONFIG_HOME/yabridge/config.toml`. -#[derive(Deserialize, Serialize, Debug)] +#[derive(Debug, Default, Deserialize, Serialize)] #[serde(default)] pub struct Config { - /// The installation method to use. We will default to creating copies since that works - /// everywhere. - pub method: InstallationMethod, /// The path to the directory containing `libyabridge-{chainloader,}-{vst2,vst3}.so`. If not /// set, then yabridgectl will look in `/usr/lib` and `$XDG_DATA_HOME/yabridge` since those are /// the expected locations for yabridge to be installed in. @@ -86,41 +82,6 @@ pub struct Config { pub last_known_config: Option, } -/// Specifies how yabridge will be set up for the found plugins. -#[derive(Deserialize, Serialize, Debug, PartialEq, Eq, Clone, Copy)] -#[serde(rename_all = "snake_case")] -pub enum InstallationMethod { - /// Create a copy of `libyabridge-chainloader-{vst2,vst3}.so` for every Windows VST2 plugin - /// `.dll` file or VST3 module found. After updating yabridge, the user will have to rerun - /// `yabridgectl sync` to copy over the new version. - Copy, - /// This will create a symlink to `libyabridge-chainloader-{vst2,vst3}.so` for every VST2 plugin - /// `.dll` file or VST3 module in the plugin directories. Now that yabridge also searches in - /// `~/.local/share/yabridge` since yabridge 2.1 this option is not really needed anymore. - /// - /// TODO: This feature has been deprecated, remove it in yabridge 4.0 - Symlink, -} - -impl InstallationMethod { - /// The plural term for this installation methodd, using in string formatting. - pub fn plural_name(&self) -> &str { - match &self { - InstallationMethod::Copy => "copies", - InstallationMethod::Symlink => "symlinks", - } - } -} - -impl Display for InstallationMethod { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self { - InstallationMethod::Copy => write!(f, "copy"), - InstallationMethod::Symlink => write!(f, "symlink"), - } - } -} - /// 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 @@ -168,19 +129,6 @@ pub struct YabridgeFiles { pub yabridge_host_32_exe_so: Option, } -impl Default for Config { - fn default() -> Self { - Config { - method: InstallationMethod::Copy, - yabridge_home: None, - plugin_dirs: BTreeSet::new(), - no_verify: false, - blacklist: BTreeSet::new(), - last_known_config: None, - } - } -} - impl Config { /// Try to read the config file, creating a new default file if necessary. This will fail if the /// file could not be created or if it could not be parsed. diff --git a/tools/yabridgectl/src/main.rs b/tools/yabridgectl/src/main.rs index 9da08d89..37c7113a 100644 --- a/tools/yabridgectl/src/main.rs +++ b/tools/yabridgectl/src/main.rs @@ -122,29 +122,6 @@ fn main() -> Result<()> { .about("Change the yabridge path (advanced)") .display_order(200) .setting(AppSettings::ArgRequiredElseHelp) - .arg( - Arg::new("method") - .long("method") - .help("The installation method to use (deprecated)") - .long_help(format!( - "This feature has been deprecated in yabridgectl 3.8.0 and should \ - not be used anymore. \ - \n\n\ - The installation method to use. \ - '{}' works in every situation but it requires you to modify your PATH \ - environment variable so yabridge is able to find 'yabridge-host.exe'. \ - 'yabridgectl sync' whenever you update yabridge. You'll also have to \ - rerun 'yabridgectl sync' whenever you update yabridge. \ - '{}' only works for hosts that support individually sandboxed plugins \ - such as Bitwig Studio, but it does not require setting environment \ - variables or to manual updates.", - "copy".bright_white(), - "symlink".bright_white() - ).as_ref()) - .setting(clap::ArgSettings::NextLineHelp) - .possible_values(["copy", "symlink"]) - .takes_value(true), - ) .arg( Arg::new("path") .long("path") @@ -265,7 +242,6 @@ fn main() -> Result<()> { Some(("set", options)) => actions::set_settings( &mut config, &actions::SetOptions { - method: options.value_of("method"), // We've already verified that the path is valid, so we should only be getting // errors for missing arguments path: options diff --git a/tools/yabridgectl/src/utils.rs b/tools/yabridgectl/src/utils.rs index ab8d189e..38323a50 100644 --- a/tools/yabridgectl/src/utils.rs +++ b/tools/yabridgectl/src/utils.rs @@ -228,7 +228,7 @@ pub fn verify_path_setup(config: &Config) -> Result { let shell = Path::new(&shell_path) .file_name() .and_then(|os_str| os_str.to_str()) - .unwrap_or_else(|| shell_path.as_str()); + .unwrap_or(shell_path.as_str()); // We're using the `-l` flag present in most shells to start a login shell, but some // shells don't have this option. According the Bash's man page, another method some