diff --git a/src/common/process.cpp b/src/common/process.cpp index 4094aa17..51904387 100644 --- a/src/common/process.cpp +++ b/src/common/process.cpp @@ -17,6 +17,7 @@ #include "process.h" #include +#include #include #include @@ -39,6 +40,63 @@ bool pid_running(pid_t pid) { return !err || err.value() == EACCES; } +std::vector get_augmented_search_path() { + // HACK: `std::locale("")` would return the current locale, but this + // overload is implementation specific, and libstdc++ returns an error + // when this happens and one of the locale variables (or `LANG`) is + // set to a locale that doesn't exist. Because of that, you should use + // the default constructor instead which does fall back gracefully + // when using an invalid locale. Boost.Process sadly doesn't seem to + // do this, so some intervention is required. We can remove this once + // the PR linked below is merged into Boost proper and included in + // most distro's copy of Boost (which will probably take a while): + // + // https://svn.boost.org/trac10/changeset/72855 + // + // https://github.com/boostorg/process/pull/179 + // FIXME: As mentioned above, we did this in the past to work around a + // Boost.Process bug. Since we no longer use Boost.Process, we can + // technically get rid of this, but we could also leave it in place + // since this may still cause other crashes for the user if we don't + // do it. + try { + std::locale(""); + } catch (const std::runtime_error&) { + // We normally avoid modifying the current process' environment and + // instead use `boost::process::environment` to only modify the + // environment of launched child processes, but in this case we do need + // to fix this + // TODO: We don't have access to the logger here, so we cannot yet + // properly print the message inform the user that their locale is + // broken when this happens + std::cerr << std::endl; + std::cerr << "WARNING: Your locale is broken. Yabridge was kind enough " + "to monkey patch it for you in this DAW session, but you " + "should probably take a look at it ;)" + << std::endl; + std::cerr << std::endl; + + setenv("LC_ALL", "C", true); // NOLINT(concurrency-mt-unsafe) + } + + // NOLINTNEXTLINE(concurrency-mt-unsafe) + const char* path_env = getenv("PATH"); + assert(path_env); + + std::vector search_path = split_path(path_env); + + // NOLINTNEXTLINE(concurrency-mt-unsafe) + if (const char* xdg_data_home = getenv("XDG_DATA_HOME")) { + search_path.push_back(fs::path(xdg_data_home) / "yabridge"); + // NOLINTNEXTLINE(concurrency-mt-unsafe) + } else if (const char* home_directory = getenv("HOME")) { + search_path.push_back(fs::path(home_directory) / ".local" / "share" / + "yabridge"); + } + + return search_path; +} + std::vector split_path( const std::string_view& path_env) { // C++ has these great split range adapters. That are completely usless. diff --git a/src/common/process.h b/src/common/process.h index 1d9a0726..ba9014a7 100644 --- a/src/common/process.h +++ b/src/common/process.h @@ -45,6 +45,17 @@ */ bool pid_running(pid_t pid); +/** + * Return the search path as defined in `$PATH`, with `~/.local/share/yabridge` + * appended to the end. Even though it likely won't be set, this does respect + * `$XDG_DATA_HOME`. I'd rather not do this since more magic makes things harder + * to comprehend, but I can understand that modifying your login shell's `PATH` + * environment variable can be a big hurdle if you've never done anything like + * that before. And since this is the recommended installation location, it + * makes sense to also search there by default. + */ +std::vector get_augmented_search_path(); + /** * Split a `PATH`-like environment variable on colons. These environment * variables don't support escaping, which makes this a lot simpler. diff --git a/src/plugin/utils.cpp b/src/plugin/utils.cpp index 14656827..28164f5e 100644 --- a/src/plugin/utils.cpp +++ b/src/plugin/utils.cpp @@ -16,8 +16,6 @@ #include "utils.h" -#include - #include #include @@ -354,63 +352,6 @@ ghc::filesystem::path generate_group_endpoint( return get_temporary_directory() / socket_name.str(); } -std::vector get_augmented_search_path() { - // HACK: `std::locale("")` would return the current locale, but this - // overload is implementation specific, and libstdc++ returns an error - // when this happens and one of the locale variables (or `LANG`) is - // set to a locale that doesn't exist. Because of that, you should use - // the default constructor instead which does fall back gracefully - // when using an invalid locale. Boost.Process sadly doesn't seem to - // do this, so some intervention is required. We can remove this once - // the PR linked below is merged into Boost proper and included in - // most distro's copy of Boost (which will probably take a while): - // - // https://svn.boost.org/trac10/changeset/72855 - // - // https://github.com/boostorg/process/pull/179 - // FIXME: As mentioned above, we did this in the past to work around a - // Boost.Process bug. Since we no longer use Boost.Process, we can - // technically get rid of this, but we could also leave it in place - // since this may still cause other crashes for the user if we don't - // do it. - try { - std::locale(""); - } catch (const std::runtime_error&) { - // We normally avoid modifying the current process' environment and - // instead use `boost::process::environment` to only modify the - // environment of launched child processes, but in this case we do need - // to fix this - // TODO: We don't have access to the logger here, so we cannot yet - // properly print the message inform the user that their locale is - // broken when this happens - std::cerr << std::endl; - std::cerr << "WARNING: Your locale is broken. Yabridge was kind enough " - "to monkey patch it for you in this DAW session, but you " - "should probably take a look at it ;)" - << std::endl; - std::cerr << std::endl; - - setenv("LC_ALL", "C", true); // NOLINT(concurrency-mt-unsafe) - } - - // NOLINTNEXTLINE(concurrency-mt-unsafe) - const char* path_env = getenv("PATH"); - assert(path_env); - - std::vector search_path = split_path(path_env); - - // NOLINTNEXTLINE(concurrency-mt-unsafe) - if (const char* xdg_data_home = getenv("XDG_DATA_HOME")) { - search_path.push_back(fs::path(xdg_data_home) / "yabridge"); - // NOLINTNEXTLINE(concurrency-mt-unsafe) - } else if (const char* home_directory = getenv("HOME")) { - search_path.push_back(fs::path(home_directory) / ".local" / "share" / - "yabridge"); - } - - return search_path; -} - Configuration load_config_for(const fs::path& yabridge_path) { // First find the closest `yabridge.tmol` file for the plugin, falling back // to default configuration settings if it doesn't exist diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 02babbe2..a1536c97 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -229,17 +229,6 @@ ghc::filesystem::path generate_group_endpoint( const ghc::filesystem::path& wine_prefix, const LibArchitecture architecture); -/** - * Return the search path as defined in `$PATH`, with `~/.local/share/yabridge` - * appended to the end. Even though it likely won't be set, this does respect - * `$XDG_DATA_HOME`. I'd rather not do this since more magic makes things harder - * to comprehend, but I can understand that modifying your login shell's `PATH` - * environment variable can be a big hurdle if you've never done anything like - * that before. And since this is the recommended installation location, it - * makes sense to also search there by default. - */ -std::vector get_augmented_search_path(); - /** * Load the configuration that belongs to a copy of or symlink to * `libyabridge-{vst2,vst3}.so`. If no configuration file could be found then