From a9df9f9bb8c760d1ba7762397fed476a47bca515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Lamotte=20=28Klaim=29?= Date: Thu, 28 Jul 2022 18:52:49 +0200 Subject: [PATCH] Fixed libmamba crash when using `pip` Added an overload of `env::which` taking a list of search paths for the executable to find. --- libmamba/include/mamba/core/activation.hpp | 5 ++- libmamba/include/mamba/core/environment.hpp | 1 + libmamba/src/api/install.cpp | 39 ++++++++++++---- libmamba/src/core/activation.cpp | 2 +- libmamba/src/core/environment.cpp | 50 ++++++++++++++------- 5 files changed, 72 insertions(+), 25 deletions(-) diff --git a/libmamba/include/mamba/core/activation.hpp b/libmamba/include/mamba/core/activation.hpp index d10e22e015..038d8f31ce 100644 --- a/libmamba/include/mamba/core/activation.hpp +++ b/libmamba/include/mamba/core/activation.hpp @@ -63,7 +63,6 @@ namespace mamba const std::string& conda_default_env, int old_conda_shlvl); - std::vector get_path_dirs(const fs::path& prefix); std::vector get_clean_dirs(); std::string add_prefix_to_path(const fs::path& prefix, int old_conda_shlvl); @@ -181,6 +180,10 @@ namespace mamba std::string hook_postamble() override; fs::path hook_source_path() override; }; + + + std::vector get_path_dirs(const fs::path& prefix); + } // namespace mamba #endif diff --git a/libmamba/include/mamba/core/environment.hpp b/libmamba/include/mamba/core/environment.hpp index b840af14f2..d9d2f41e4b 100644 --- a/libmamba/include/mamba/core/environment.hpp +++ b/libmamba/include/mamba/core/environment.hpp @@ -50,6 +50,7 @@ namespace mamba void unset(const std::string& key); fs::path which(const std::string& exe, const std::string& override_path = ""); + fs::path which(const std::string& exe, const std::vector& search_paths); std::map copy(); std::string platform(); fs::path home_directory(); diff --git a/libmamba/src/api/install.cpp b/libmamba/src/api/install.cpp index fc5a82bdd8..8b6e0ff200 100644 --- a/libmamba/src/api/install.cpp +++ b/libmamba/src/api/install.cpp @@ -20,6 +20,7 @@ #include "mamba/core/util.hpp" #include "mamba/core/virtual_packages.hpp" #include "mamba/core/env_lockfile.hpp" +#include "mamba/core/activation.hpp" #include "termcolor/termcolor.hpp" @@ -27,8 +28,25 @@ namespace mamba { namespace { - std::map other_pkg_mgr_install_instructions - = { { "pip", "python -m pip install -r {0} --no-input" } }; + tl::expected get_other_pkg_mgr_install_instructions( + const std::string& name, const std::string& target_prefix) + { + const auto get_python_path + = [&] { return env::which("python", get_path_dirs(target_prefix)).u8string(); }; + + const std::unordered_map other_pkg_mgr_install_instructions{ + { "pip", + fmt::format("{} {}", get_python_path(), "-m pip install -r {0} --no-input") } + }; + + auto found_it = other_pkg_mgr_install_instructions.find(name); + if (found_it != other_pkg_mgr_install_instructions.end()) + return found_it->second; + else + return tl::unexpected(std::runtime_error( + fmt::format("no install instruction found for package manager '{}'", name))); + } + } bool reproc_killed(int status) @@ -70,7 +88,17 @@ namespace mamba const auto& deps = other_spec.deps; const auto& cwd = other_spec.cwd; - std::string install_instructions = other_pkg_mgr_install_instructions[pkg_mgr]; + const auto& ctx = Context::instance(); + + std::string install_instructions = [&] + { + const auto maybe_instructions + = get_other_pkg_mgr_install_instructions(pkg_mgr, ctx.target_prefix); + if (maybe_instructions) + return maybe_instructions.value(); + else + throw maybe_instructions.error(); + }(); TemporaryFile specs; std::ofstream specs_f = open_ofstream(specs.path()); @@ -80,12 +108,7 @@ namespace mamba replace_all(install_instructions, "{0}", specs.path()); - const auto& ctx = Context::instance(); - std::vector install_args = split(install_instructions, " "); - - install_args[0] - = (ctx.target_prefix / get_bin_directory_short_path() / install_args[0]).string(); auto [wrapped_command, tmpfile] = prepare_wrapped_call(ctx.target_prefix, install_args); reproc::options options; diff --git a/libmamba/src/core/activation.cpp b/libmamba/src/core/activation.cpp index ca40079b29..2837d5be6e 100644 --- a/libmamba/src/core/activation.cpp +++ b/libmamba/src/core/activation.cpp @@ -172,7 +172,7 @@ namespace mamba } } - std::vector Activator::get_path_dirs(const fs::path& prefix) + std::vector get_path_dirs(const fs::path& prefix) { if (on_win) { diff --git a/libmamba/src/core/environment.cpp b/libmamba/src/core/environment.cpp index 9c75bee7dc..d1a9775746 100644 --- a/libmamba/src/core/environment.cpp +++ b/libmamba/src/core/environment.cpp @@ -81,23 +81,12 @@ namespace mamba if (env_path) { std::string path = env_path.value(); - auto parts = mamba::split(path, pathsep()); - for (auto& p : parts) - { - if (!fs::exists(p) || !fs::is_directory(p)) - { - continue; - } - for (const auto& entry : fs::directory_iterator(p)) - { - if (entry.path().filename() == exe) - { - return entry.path(); - } - } - } + const auto parts = mamba::split(path, pathsep()); + const std::vector search_paths(parts.begin(), parts.end()); + return which(exe, search_paths); } + #ifndef _WIN32 if (override_path == "") { @@ -115,6 +104,37 @@ namespace mamba return ""; // empty path } + fs::path which(const std::string& exe, const std::vector& search_paths) + { + for (auto& p : search_paths) + { + if (!fs::exists(p) || !fs::is_directory(p)) + { + continue; + } + +#ifdef _WIN32 + const auto exe_with_extension = exe + ".exe"; +#endif + for (const auto& entry : fs::directory_iterator(p)) + { + const auto filename = entry.path().filename(); + if (filename == exe + +#ifdef _WIN32 + || filename == exe_with_extension +#endif + ) + { + return entry.path(); + } + } + } + + + return ""; // empty path + } + std::map copy() { std::map m;