Additional cleanup and first pass according to review

libmamba/src/core/curl.cpp

@@ -234,7 +234,6 @@ namespace mamba
 
     CURLHandle::CURLHandle()  //(const Context& ctx)
         : m_handle(curl_easy_init())
-        , m_result(CURLE_OK)
     {
         if (m_handle == nullptr)
         {
@@ -253,15 +252,13 @@ namespace mamba
         rhs.m_handle = nullptr;
         rhs.p_headers = nullptr;
         std::swap(m_errorbuffer, rhs.m_errorbuffer);
-        std::swap(m_result, rhs.m_result);
         set_opt(CURLOPT_ERRORBUFFER, m_errorbuffer.data());
     }
 
     CURLHandle& CURLHandle::operator=(CURLHandle&& rhs)
     {
         using std::swap;
         swap(m_handle, rhs.m_handle);
-        swap(m_result, rhs.m_result);
         swap(p_headers, rhs.p_headers);
         swap(m_errorbuffer, rhs.m_errorbuffer);
         set_opt(CURLOPT_ERRORBUFFER, m_errorbuffer.data());
@@ -459,34 +456,9 @@ namespace mamba
         return get_info<std::string>(CURLINFO_EFFECTIVE_URL).value();
     }
 
-    std::size_t CURLHandle::get_result() const
+    CURLcode CURLHandle::perform()
     {
-        return static_cast<std::size_t>(m_result);
-    }
-
-    bool CURLHandle::is_curl_res_ok() const
-    {
-        return is_curl_res_ok(m_result);
-    }
-
-    void CURLHandle::set_result(CURLcode res)
-    {
-        m_result = res;
-    }
-
-    std::string CURLHandle::get_res_error() const
-    {
-        return get_res_error(m_result);
-    }
-
-    bool CURLHandle::can_proceed()
-    {
-        return can_retry(m_result);
-    }
-
-    void CURLHandle::perform()
-    {
-        m_result = curl_easy_perform(m_handle);
+        return curl_easy_perform(m_handle);
     }
 
     CURLId CURLHandle::get_id() const

libmamba/src/core/curl.hpp

@@ -140,16 +140,7 @@ namespace mamba
         const char* get_error_buffer() const;
         std::string get_curl_effective_url() const;
 
-        [[deprecated]] std::size_t get_result() const;
-        [[deprecated]] bool is_curl_res_ok() const;
-
-        [[deprecated]] void set_result(CURLcode res);
-
-        [[deprecated]] std::string get_res_error() const;
-
-        // Side-effect programming, to remove
-        [[deprecated]] bool can_proceed();
-        void perform();
+        CURLcode perform();
 
         CURLId get_id() const;
 
@@ -161,7 +152,6 @@ namespace mamba
     private:
 
         CURL* m_handle;
-        CURLcode m_result;  // Enum range from 0 to 99
         curl_slist* p_headers = nullptr;
         std::array<char, CURL_ERROR_SIZE> m_errorbuffer;
 

libmamba/src/core/download.cpp

@@ -91,7 +91,13 @@ namespace mamba
             }
         }
 
-        std::pair<bool, bool> get_env_remote_params(const Context& context)
+        struct EnvRemoteParams
+        {
+            bool set_low_speed_opt = false;
+            bool set_ssl_no_revoke = false;
+        };
+
+        EnvRemoteParams get_env_remote_params(const Context& context)
         {
             // TODO: we should probably store set_low_speed_limit and set_ssl_no_revoke in
             // RemoteFetchParams if the request is slower than 30b/s for 60 seconds, cancel.
@@ -249,7 +255,7 @@ namespace mamba
 
     void DownloadAttempt::configure_handle(const Context& context)
     {
-        auto [set_low_speed_opt, set_ssl_no_revoke] = get_env_remote_params(context);
+        const auto [set_low_speed_opt, set_ssl_no_revoke] = get_env_remote_params(context);
 
         m_handle.configure_handle(
             util::file_uri_unc2_to_unc4(p_request->url),
@@ -874,7 +880,7 @@ namespace mamba
             init_remote_fetch_params(ctx.remote_fetch_params);
         }
 
-        auto [set_low_speed_opt, set_ssl_no_revoke] = get_env_remote_params(context);
+        const auto [set_low_speed_opt, set_ssl_no_revoke] = get_env_remote_params(context);
 
         return curl::check_resource_exists(
             util::file_uri_unc2_to_unc4(url),

libmamba/src/core/download_progress_bar.cpp

@@ -18,7 +18,7 @@ namespace mamba
             const DownloadProgress& progress
         )
         {
-            auto now = std::chrono::steady_clock::now();
+            const auto now = std::chrono::steady_clock::now();
 
             const auto throttle_treshold = std::chrono::milliseconds(50);
             if (now - throttle_time < throttle_treshold)
@@ -283,7 +283,7 @@ namespace mamba
         init_aggregated_download();
         init_aggregated_extract();
 
-        auto& pbar_manager = dynamic_cast<AggregatedBarManager&>(
+        auto& pbar_manager = static_cast<AggregatedBarManager&>(
             Console::instance().progress_bar_manager()
         );
         pbar_manager.start();
@@ -340,7 +340,7 @@ namespace mamba
 
     void PackageDownloadMonitor::init_aggregated_extract()
     {
-        auto& pbar_manager = dynamic_cast<AggregatedBarManager&>(
+        auto& pbar_manager = static_cast<AggregatedBarManager&>(
             Console::instance().progress_bar_manager()
         );
         auto* extract_bar = pbar_manager.aggregated_bar("Extract");
@@ -383,7 +383,7 @@ namespace mamba
 
     void PackageDownloadMonitor::init_aggregated_download()
     {
-        auto& pbar_manager = dynamic_cast<AggregatedBarManager&>(
+        auto& pbar_manager = static_cast<AggregatedBarManager&>(
             Console::instance().progress_bar_manager()
         );
         auto* dl_bar = pbar_manager.aggregated_bar("Download");

libmamba/src/core/package_fetcher.cpp

@@ -81,7 +81,7 @@ namespace mamba
     )
         : m_package_info(pkg_info)
     {
-        // only do this for micromamba for now
+        // FIXME: only do this for micromamba for now
         if (channel_context.context().command_params.is_micromamba)
         {
             m_url = channel_context.make_channel(pkg_info.url).urls(true)[0];
@@ -91,10 +91,10 @@ namespace mamba
             m_url = pkg_info.url;
         }
 
-        fs::u8path extracted_cache = caches.get_extracted_dir_path(m_package_info);
+        const fs::u8path extracted_cache = caches.get_extracted_dir_path(m_package_info);
         if (extracted_cache.empty())
         {
-            fs::u8path tarball_cache = caches.get_tarball_path(m_package_info);
+            const fs::u8path tarball_cache = caches.get_tarball_path(m_package_info);
             auto& cache = caches.first_writable_cache(true);
             m_cache_path = cache.path();
 
@@ -304,10 +304,7 @@ namespace mamba
     {
         fs::remove_all(m_tarball_path);
         fs::u8path dest_dir = strip_package_extension(m_tarball_path.string());
-        if (fs::exists(dest_dir))
-        {
-            fs::remove_all(dest_dir);
-        }
+        fs::remove_all(dest_dir);
     }
 
     /*******************
@@ -349,7 +346,6 @@ namespace mamba
                       << "\nExpected: " << expected_size() << "\nActual: " << downloaded_size
                       << "\n";
             Console::instance().print(filename() + " tarball has incorrect size");
-            // TODO: terminate monitor
         }
         return res;
     }
@@ -371,8 +367,8 @@ namespace mamba
 
     void PackageFetcher::write_repodata_record(const fs::u8path& base_path) const
     {
-        fs::u8path repodata_record_path = base_path / "info" / "repodata_record.json";
-        fs::u8path index_path = base_path / "info" / "index.json";
+        const fs::u8path repodata_record_path = base_path / "info" / "repodata_record.json";
+        const fs::u8path index_path = base_path / "info" / "index.json";
 
         nlohmann::json index, solvable_json;
         std::ifstream index_file = open_ifstream(index_path);

libmamba/src/core/transaction.cpp

@@ -1051,7 +1051,7 @@ namespace mamba
             task.wait();
         }
 
-        bool all_valid = check_all_valid(fetchers, extract_requests);
+        const bool all_valid = check_all_valid(fetchers, extract_requests);
         return !is_sig_interrupted() && all_valid;
     }