Merge pull request #670 from wolfv/fix_win_chcp

when compiling pyc, properly chcp

include/mamba/thread_utils.hpp

@@ -50,7 +50,7 @@ namespace mamba
     // Must be called by the cleaning thread to ensure
     // it won't free ressources that could be required
     // by threads still active.
-    void wait_before_cleaning();
+    void wait_for_all_threads();
     void notify_cleanup();
 
     // Should be called by the main thread to ensure
@@ -145,7 +145,7 @@ namespace mamba
         m_cleanup_function = std::bind(std::forward<Function>(func), std::forward<Args>(args)...);
         std::signal(SIGINT, [](int) {
             set_sig_interrupted();
-            wait_before_cleaning();
+            wait_for_all_threads();
             m_cleanup_function();
         });
     }
@@ -159,11 +159,15 @@ namespace mamba
         block_signals();
         auto f = std::bind(std::forward<Function>(func), std::forward<Args>(args)...);
         std::thread victor_the_cleaner([f, this]() {
+            pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, nullptr);
+            pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, nullptr);
+
             wait_for_signal();
+
             if (m_interrupt.load())
             {
                 set_sig_interrupted();
-                wait_before_cleaning();
+                wait_for_all_threads();
                 f();
                 notify_cleanup();
             }

include/mamba/transaction.hpp

@@ -50,8 +50,23 @@ namespace mamba
         bool finalize_callback();
         bool finished();
         bool validate_extract();
+        auto validation_result() const;
+        void clear_cache() const;
+
         DownloadTarget* target(const fs::path& cache_path, MultiPackageCache& cache);
 
+        enum VALIDATION_RESULT
+        {
+            UNDEFINED = 0,
+            VALID = 1,
+            SHA256_ERROR,
+            MD5SUM_ERROR,
+            SIZE_ERROR,
+            EXTRACT_ERROR
+        };
+
+        std::exception m_decompress_exception;
+
     private:
         bool m_finished;
         PackageInfo m_package_info;
@@ -67,6 +82,7 @@ namespace mamba
 
         std::future<bool> m_extract_future;
 
+        VALIDATION_RESULT m_validation_result = VALIDATION_RESULT::UNDEFINED;
         static std::mutex extract_mutex;
     };
 

src/link.cpp

@@ -463,6 +463,43 @@ namespace mamba
         return tf;
     }
 
+    auto prepare_wrapped_call(const fs::path& prefix, const std::vector<std::string>& cmd)
+    {
+        std::vector<std::string> command_args;
+        std::unique_ptr<TemporaryFile> script_file;
+
+        if (on_win)
+        {
+            ensure_comspec_set();
+            std::string comspec = env::get("COMSPEC");
+            if (comspec.size() == 0)
+            {
+                throw std::runtime_error(
+                    concat("Failed to run script: COMSPEC not set in env vars."));
+            }
+
+            script_file = wrap_call(
+                Context::instance().root_prefix, prefix, Context::instance().dev, false, cmd);
+
+            command_args = { comspec, "/d", "/c", script_file->path() };
+        }
+        else
+        {
+            // shell_path = 'sh' if 'bsd' in sys.platform else 'bash'
+            fs::path shell_path = env::which("bash");
+            if (shell_path.empty())
+            {
+                shell_path = env::which("sh");
+            }
+
+            script_file = wrap_call(
+                Context::instance().root_prefix, prefix, Context::instance().dev, false, cmd);
+            command_args.push_back(shell_path);
+            command_args.push_back(script_file->path());
+        }
+        return std::make_tuple(command_args, std::move(script_file));
+    }
+
     /*
        call the post-link or pre-unlink script and return true / false on success /
        failure
@@ -473,13 +510,17 @@ namespace mamba
                     const std::string& env_prefix = "",
                     bool activate = false)
     {
-#ifdef _WIN32
+        fs::path path;
-        auto path = prefix / get_bin_directory_short_path()
+        if (on_win)
-                    / concat(".", pkg_info.name, "-", action, ".bat");
+        {
-#else
+            path = prefix / get_bin_directory_short_path()
-        auto path = prefix / get_bin_directory_short_path()
+                   / concat(".", pkg_info.name, "-", action, ".bat");
-                    / concat(".", pkg_info.name, "-", action, ".sh");
+        }
-#endif
+        else
+        {
+            path = prefix / get_bin_directory_short_path()
+                   / concat(".", pkg_info.name, "-", action, ".sh");
+        }
 
         if (!fs::exists(path))
         {
@@ -500,56 +541,60 @@ namespace mamba
         std::vector<std::string> command_args;
         std::unique_ptr<TemporaryFile> script_file;
 
-#ifdef _WIN32
+        if (on_win)
-        ensure_comspec_set();
-        std::string comspec = env::get("COMSPEC");
-        if (comspec.size() == 0)
         {
-            LOG_ERROR << "Failed to run " << action << " for " << pkg_info.name
+            ensure_comspec_set();
-                      << " due to COMSPEC not set in env vars.";
+            std::string comspec = env::get("COMSPEC");
-            return false;
+            if (comspec.size() == 0)
+            {
+                LOG_ERROR << "Failed to run " << action << " for " << pkg_info.name
+                          << " due to COMSPEC not set in env vars.";
+                return false;
+            }
+
+            if (activate)
+            {
+                script_file = wrap_call(Context::instance().root_prefix,
+                                        prefix,
+                                        Context::instance().dev,
+                                        false,
+                                        { "@CALL", path });
+
+                command_args = { comspec, "/d", "/c", script_file->path() };
+            }
+            else
+            {
+                command_args = { comspec, "/d", "/c", path };
+            }
         }
 
-        if (activate)
-        {
-            script_file = wrap_call(Context::instance().root_prefix,
-                                    prefix,
-                                    Context::instance().dev,
-                                    false,
-                                    { "@CALL", path });
-
-            command_args = { comspec, "/d", "/c", script_file->path() };
-        }
         else
         {
-            command_args = { comspec, "/d", "/c", path };
+            // shell_path = 'sh' if 'bsd' in sys.platform else 'bash'
-        }
+            fs::path shell_path = env::which("bash");
-#else
+            if (shell_path.empty())
-        // shell_path = 'sh' if 'bsd' in sys.platform else 'bash'
+            {
-        fs::path shell_path = env::which("bash");
+                shell_path = env::which("sh");
-        if (shell_path.empty())
+            }
-        {
-            shell_path = env::which("sh");
-        }
 
-        if (activate)
+            if (activate)
-        {
+            {
-            // std::string caller
+                // std::string caller
-            script_file = wrap_call(Context::instance().root_prefix,
+                script_file = wrap_call(Context::instance().root_prefix,
-                                    prefix,
+                                        prefix,
-                                    Context::instance().dev,
+                                        Context::instance().dev,
-                                    false,
+                                        false,
-                                    { ".", path });
+                                        { ".", path });
-            command_args.push_back(shell_path);
+                command_args.push_back(shell_path);
-            command_args.push_back(script_file->path());
+                command_args.push_back(script_file->path());
+            }
+            else
+            {
+                command_args.push_back(shell_path);
+                command_args.push_back("-x");
+                command_args.push_back(path);
+            }
         }
-        else
-        {
-            command_args.push_back(shell_path);
-            command_args.push_back("-x");
-            command_args.push_back(path);
-        }
-#endif
 
         envmap["ROOT_PREFIX"] = Context::instance().root_prefix;
         envmap["PREFIX"] = env_prefix.size() ? env_prefix : std::string(prefix);
@@ -875,9 +920,6 @@ namespace mamba
         }
         all_py_files_f.close();
 
-        // TODO use the real python file here?!
-        LOG_DEBUG << "Running " << m_context->target_prefix / m_context->python_path
-                  << " -Wi -m compileall -q -l -i " << all_py_files.path() << std::endl;
         std::vector<std::string> command = { m_context->target_prefix / m_context->python_path,
                                              "-Wi",
                                              "-m",
@@ -901,18 +943,10 @@ namespace mamba
         std::string cwd = m_context->target_prefix;
         options.working_directory = cwd.c_str();
 
-        std::map<std::string, std::string> envmap = env::copy();
+        auto [wrapped_command, script_file]
-#ifdef _WIN32
+            = prepare_wrapped_call(m_context->target_prefix, command);
-        CmdExeActivator activator;
+        LOG_INFO << "Running wrapped python compilation command " << join(" ", command);
-#else
+        auto [_, ec] = reproc::run(wrapped_command, options);
-        PosixActivator activator;
-#endif
-        std::string current_env_path = activator.add_prefix_to_path(m_context->target_prefix, 0);
-        envmap["PATH"] = current_env_path;
-        options.env.behavior = reproc::env::empty;
-        options.env.extra = envmap;
-
-        auto [_, ec] = reproc::run(command, options);
 
         if (ec)
         {
@@ -934,11 +968,11 @@ namespace mamba
     {
         LOG_INFO << "Executing install for " << m_source;
         nlohmann::json index_json, out_json;
-        LOG_WARNING << "Opening: " << m_source / "info" / "paths.json";
+        LOG_INFO << "Opening: " << m_source / "info" / "paths.json";
 
         auto paths_data = read_paths(m_source);
 
-        LOG_WARNING << "Opening: " << m_source / "info" / "repodata_record.json";
+        LOG_INFO << "Opening: " << m_source / "info" / "repodata_record.json";
         std::ifstream repodata_f(m_source / "info" / "repodata_record.json");
 
         repodata_f >> index_json;

src/micromamba/main.cpp

@@ -792,7 +792,6 @@ download_explicit(const std::vector<PackageInfo>& pkgs)
     std::vector<std::unique_ptr<PackageDownloadExtractTarget>> targets;
     MultiDownloadTarget multi_dl;
     MultiPackageCache pkg_cache({ Context::instance().root_prefix / "pkgs" });
-    Console::instance().init_multi_progress();
 
     for (auto& pkg : pkgs)
     {
@@ -1240,9 +1239,11 @@ main(int argc, char** argv)
     {
         CLI11_PARSE(app, argc, argv);
     }
-    catch (std::exception& e)
+    catch (const std::exception& e)
     {
         LOG_ERROR << e.what();
+        std::raise(SIGTERM);
+        std::cout << "Exiting." << std::endl;
         exit(1);
     }
 

src/package_handling.cpp

@@ -371,22 +371,24 @@ namespace mamba
     fs::path extract(const fs::path& file)
     {
         std::string dest_dir = file;
+        TemporaryDirectory temp_extract_dir;
+
         if (ends_with(dest_dir, ".tar.bz2"))
         {
             dest_dir = dest_dir.substr(0, dest_dir.size() - 8);
-            extract_archive(file, dest_dir);
+            extract_archive(file, temp_extract_dir.path());
-            return dest_dir;
         }
         else if (ends_with(dest_dir, ".conda"))
         {
             dest_dir = dest_dir.substr(0, dest_dir.size() - 6);
-            extract_conda(file, dest_dir);
+            extract_conda(file, temp_extract_dir.path());
-            return dest_dir;
         }
         else
         {
             throw std::runtime_error("Unknown package format (" + file.string() + ")");
         }
+        fs::rename(temp_extract_dir.path(), dest_dir);
+        return dest_dir;
     }
 
     bool transmute(const fs::path& pkg_file, const fs::path& target, int compression_level)

src/thread_utils.cpp

@@ -78,7 +78,7 @@ namespace mamba
         return thread_count;
     }
 
-    void wait_before_cleaning()
+    void wait_for_all_threads()
     {
         std::unique_lock<std::mutex> lk(clean_mutex);
         clean_var.wait(lk, []() { return thread_count == 0; });
@@ -98,11 +98,17 @@ namespace mamba
 
     namespace
     {
-        std::thread::native_handle_type cleanup_id;
+        std::thread::native_handle_type cleanup_id = 0;
     }
 
     void register_cleaning_thread_id(std::thread::native_handle_type id)
     {
+#ifndef _WIN32
+        if (cleanup_id)
+        {
+            pthread_cancel(cleanup_id);
+        }
+#endif
         cleanup_id = id;
     }
 
@@ -160,10 +166,13 @@ namespace mamba
         if (is_sig_interrupted())
         {
             wait_for_cleanup();
+            std::cout << "Cleanup done." << std::endl;
         }
         m_interrupt.store(false);
         pthread_sigmask(SIG_UNBLOCK, &sigset, nullptr);
         set_default_signal_handler();
+        pthread_cancel(get_cleaning_thread_id());
+        register_cleaning_thread_id(0);
     }
 
     void interruption_guard::block_signals() const

src/transaction.cpp

@@ -79,29 +79,39 @@ namespace mamba
     bool PackageDownloadExtractTarget::validate_extract()
     {
         // Validation
+        m_validation_result = VALIDATION_RESULT::VALID;
         if (m_expected_size && size_t(m_target->downloaded_size) != m_expected_size)
         {
-            LOG_ERROR << "File not valid: file size doesn't match expectation " << m_tarball_path;
+            LOG_ERROR << "File not valid: file size doesn't match expectation " << m_tarball_path
-            throw std::runtime_error("File not valid: file size doesn't match expectation ("
+                      << "\nExpected: " << m_expected_size << "\n";
-                                     + std::string(m_tarball_path) + ")");
+            m_progress_proxy.mark_as_completed("File size validation error.");
+            m_validation_result = SIZE_ERROR;
         }
         interruption_point();
 
         if (!m_sha256.empty() && !validate::sha256(m_tarball_path, m_sha256))
         {
-            LOG_ERROR << "File not valid: SHA256 sum doesn't match expectation " << m_tarball_path;
+            m_validation_result = SHA256_ERROR;
-            throw std::runtime_error("File not valid: SHA256 sum doesn't match expectation ("
+            m_progress_proxy.mark_as_completed("SHA256 sum validation error.");
-                                     + std::string(m_tarball_path) + ")");
+            LOG_ERROR << "File not valid: SHA256 sum doesn't match expectation " << m_tarball_path
+                      << "\nExpected: " << m_sha256 << "\n";
         }
         else
         {
             if (!m_md5.empty() && !validate::md5(m_tarball_path, m_md5))
             {
-                LOG_ERROR << "File not valid: MD5 sum doesn't match expectation " << m_tarball_path;
+                m_validation_result = MD5SUM_ERROR;
-                throw std::runtime_error("File not valid: MD5 sum doesn't match expectation ("
+                m_progress_proxy.mark_as_completed("MD5 sum validation error.");
-                                         + std::string(m_tarball_path) + ")");
+                LOG_ERROR << "File not valid: MD5 sum doesn't match expectation " << m_tarball_path
+                          << "\nExpected: " << m_md5 << "\n";
             }
         }
+        if (m_validation_result != VALIDATION_RESULT::VALID)
+        {
+            // abort here, but set finished to true
+            m_finished = true;
+            return true;
+        }
 
         interruption_point();
         LOG_INFO << "Waiting for decompression " << m_tarball_path;
@@ -112,10 +122,22 @@ namespace mamba
             interruption_point();
             m_progress_proxy.set_postfix("Decompressing...");
             LOG_INFO << "Decompressing " << m_tarball_path;
-            auto extract_path = extract(m_tarball_path);
+            try
-            LOG_INFO << "Extracted to " << extract_path;
+            {
-            write_repodata_record(extract_path);
+                auto extract_path = extract(m_tarball_path);
-            add_url();
+                LOG_INFO << "Extracted to " << extract_path;
+                write_repodata_record(extract_path);
+                add_url();
+            }
+            catch (std::exception& e)
+            {
+                LOG_ERROR << "Error when extracting package: " << e.what();
+                m_decompress_exception = e;
+                m_validation_result = VALIDATION_RESULT::EXTRACT_ERROR;
+                m_finished = true;
+                m_progress_proxy.mark_as_completed("Extraction error");
+                return m_finished;
+            }
         }
 
         interruption_point();
@@ -152,6 +174,21 @@ namespace mamba
         return m_target == nullptr ? true : m_finished;
     }
 
+    auto PackageDownloadExtractTarget::validation_result() const
+    {
+        return m_validation_result;
+    }
+
+    void PackageDownloadExtractTarget::clear_cache() const
+    {
+        fs::remove_all(m_tarball_path);
+        fs::path dest_dir = strip_package_extension(m_tarball_path);
+        if (fs::exists(dest_dir))
+        {
+            fs::remove_all(dest_dir);
+        }
+    }
+
     // todo remove cache from this interface
     DownloadTarget* PackageDownloadExtractTarget::target(const fs::path& cache_path,
                                                          MultiPackageCache& cache)
@@ -166,6 +203,7 @@ namespace mamba
         if (valid && !dest_dir_exists)
         {
             // TODO add extract job here
+            // finalize_callback();
         }
 
         // tarball can be removed, it's fine if only the correct dest dir exists
@@ -663,6 +701,7 @@ namespace mamba
         interruption_guard g([]() { Console::instance().init_multi_progress(); });
 
         bool downloaded = multi_dl.download(true);
+        bool all_valid = true;
 
         if (!downloaded)
         {
@@ -688,7 +727,19 @@ namespace mamba
             std::this_thread::sleep_for(std::chrono::milliseconds(100));
         }
 
-        return !is_sig_interrupted() && downloaded;
+        for (const auto& t : targets)
+        {
+            if (t->validation_result() != PackageDownloadExtractTarget::VALIDATION_RESULT::VALID
+                && t->validation_result()
+                       != PackageDownloadExtractTarget::VALIDATION_RESULT::UNDEFINED)
+            {
+                t->clear_cache();
+                all_valid = false;
+                throw std::runtime_error("Found incorrect download. Aborting");
+            }
+        }
+
+        return !is_sig_interrupted() && downloaded && all_valid;
     }
 
     bool MTransaction::empty()