diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb3e3c7..1fe07a3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed + +- Parsing failures for `yabridge.toml` files will no longer cause plugins to + fail to load. If that happens, you'll now get a desktop notification and the + plugin will use the default settings instead. + ### yabridgectl - Some outdated warning messages have been updated to make yabridge's current diff --git a/src/common/configuration.cpp b/src/common/configuration.cpp index cac37efb..6cfb18be 100644 --- a/src/common/configuration.cpp +++ b/src/common/configuration.cpp @@ -29,7 +29,7 @@ Configuration::Configuration() noexcept {} Configuration::Configuration(const fs::path& config_path, const fs::path& yabridge_path) : Configuration() { - // Will throw a `toml::parsing_error` if the file cannot be parsed. Better + // Will throw a `toml::parse_error` if the file cannot be parsed. Better // to throw here rather than failing silently since syntax errors would // otherwise be impossible to spot. We'll also have to sort all tables by // the location in the file since tomlplusplus internally uses ordered maps diff --git a/src/common/configuration.h b/src/common/configuration.h index d119742c..dd3cc18e 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -30,9 +30,9 @@ * plugins that will be hosted in the same process rather than individually so * they can share resources. Configuration file loading works as follows: * - * 1. `load_config_for(path)` from `src/plugin/utils.h` gets called with a path - * to the copy of or symlink to `libyabridge-{clap,vst2,vst3}.so` that the - * plugin host has tried to load. + * 1. `load_config_for(path, ...)` from `src/plugin/utils.h` gets called with a + * path to the copy of or symlink to `libyabridge-{clap,vst2,vst3}.so` that + * the plugin host has tried to load. * 2. We start looking for a file named `yabridge.toml` in the same directory as * that `.so` file, iteratively continuing to search one directory higher * until we either find the file or we reach the filesystem root. @@ -64,7 +64,7 @@ class Configuration { * configuration file. Will leave the object empty if the plugin cannot be * matched to any of the patterns. Not meant to be used directly. * - * @throw toml::parsing_error If the file could not be parsed. + * @throw toml::parse_error If the file could not be parsed. * * @see ../plugin/utils.h:load_config_for */ diff --git a/src/plugin/bridges/common.h b/src/plugin/bridges/common.h index acebf30d..578186f2 100644 --- a/src/plugin/bridges/common.h +++ b/src/plugin/bridges/common.h @@ -81,14 +81,14 @@ class PluginBridge { PluginBridge(PluginType plugin_type, const ghc::filesystem::path& plugin_path, F&& create_socket_instance) - // This is still correct for VST3 plugins because we can configure an - // entire directory (the module's bundle) at once - : config_(load_config_for(plugin_path)), - info_(plugin_type, plugin_path, config_.vst3_prefer_32bit), - io_context_(), + : io_context_(), sockets_(create_socket_instance(io_context_, info_)), generic_logger_(Logger::create_from_environment( create_logger_prefix(sockets_.base_dir_))), + // This works for both individual files (VST2 and CLAP) and entire + // directories (VST3) + config_(load_config_for(plugin_path, generic_logger_)), + info_(plugin_type, plugin_path, config_.vst3_prefer_32bit), plugin_host_( config_.group ? std::unique_ptr(std::make_unique( @@ -438,19 +438,6 @@ class PluginBridge { } } - /** - * The configuration for this instance of yabridge. Set based on the values - * from a `yabridge.toml`, if it exists. - * - * @see ../utils.h:load_config_for - */ - Configuration config_; - - /** - * Information about the plugin we're bridging. - */ - const PluginInfo info_; - asio::io_context io_context_; /** @@ -471,6 +458,19 @@ class PluginBridge { */ Logger generic_logger_; + /** + * The configuration for this instance of yabridge. Set based on the values + * from a `yabridge.toml`, if it exists. + * + * @see ../utils.h:load_config_for + */ + Configuration config_; + + /** + * Information about the plugin we're bridging. + */ + const PluginInfo info_; + /** * The Wine process hosting our plugins. In the case of group hosts a * `PluginBridge` instance doesn't actually own a process, but rather either diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 4a52b6c1..65f3189c 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -23,6 +23,8 @@ #include #include "../common/configuration.h" +#include "../common/notifications.h" +#include "../common/toml++.h" #include "../common/utils.h" namespace fs = ghc::filesystem; @@ -396,7 +398,7 @@ ghc::filesystem::path generate_group_endpoint( return get_temporary_directory() / socket_name.str(); } -Configuration load_config_for(const fs::path& yabridge_path) { +Configuration load_config_for(const fs::path& yabridge_path, Logger& logger) { // First find the closest `yabridge.tmol` file for the plugin, falling back // to default configuration settings if it doesn't exist const std::optional config_file = @@ -405,5 +407,25 @@ Configuration load_config_for(const fs::path& yabridge_path) { return Configuration(); } - return Configuration(*config_file, yabridge_path); + try { + return Configuration(*config_file, yabridge_path); + } catch (const toml::parse_error& error) { + // Parsing failures should be non-fatal since that leads to a pretty + // confusing user experience (see + // https://github.com/robbert-vdh/yabridge/issues/282). They should, + // however, still result in a visible error. + logger.log(""); + logger.log("The configuration file at '" + config_file->string() + + "' could not be parsed:"); + logger.log(error.what()); + logger.log(""); + + send_notification( + "Failed to parse yabridge.toml file", + "The configuration file at '" + config_file->string() + + "' could not be parsed: " + std::string(error.description()), + std::nullopt); + + return Configuration(); + } } diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 53ecc1aa..6bb09c55 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -19,6 +19,7 @@ #include #include "../common/configuration.h" +#include "../common/logging/common.h" #include "../common/plugins.h" #include "../common/process.h" #include "../common/utils.h" @@ -238,12 +239,17 @@ ghc::filesystem::path generate_group_endpoint( * See the docstrong on the `Configuration` class for more details on how to * choose the config file to load. * - * This function will take any optional compile-time features that have not been - * enabled into account. + * If the configuration file has syntax errors, then an error will be logged and + * the defaults will be used instead. + * + * This function will also take any optional compile-time features that have not + * been enabled into account. * * @param yabridge_path The path to the .so file that's being loaded.by the VST * host. This will be used both for the starting location of the search and to * determine which section in the config file to use. + * @param logger The logger used to log parsing errors to. Parsing errors are + * non-fatal, but they should still be very visible. * * @return Either a configuration object populated with values from matched glob * pattern within the found configuration file, or an empty configuration @@ -252,7 +258,8 @@ ghc::filesystem::path generate_group_endpoint( * * @see Configuration */ -Configuration load_config_for(const ghc::filesystem::path& yabridge_path); +Configuration load_config_for(const ghc::filesystem::path& yabridge_path, + Logger& logger); /** * Starting from the starting file or directory, go up in the directory