From f6863599653083a7314f9ef620eeaffb97550a81 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Wed, 28 Jun 2023 19:25:21 +0200 Subject: [PATCH] Revert "Improvement: Support ZST in mamba and enable ZST by default (#2404)" This reverts commit 60cd358c396b179f80af99a59ee1d9632216afd1. --- libmamba/include/mamba/core/context.hpp | 1 + libmamba/src/api/configuration.cpp | 5 + libmamba/src/api/info.cpp | 31 ++--- libmamba/src/core/subdirdata.cpp | 147 ++++++++++++++++-------- libmamba/tests/src/core/test_cpp.cpp | 4 + libmambapy/libmambapy/__init__.pyi | 2 - libmambapy/src/main.cpp | 19 +-- mamba/mamba/utils.py | 17 +-- 8 files changed, 133 insertions(+), 93 deletions(-) diff --git a/libmamba/include/mamba/core/context.hpp b/libmamba/include/mamba/core/context.hpp index d5ef67e3e4..7757c64b81 100644 --- a/libmamba/include/mamba/core/context.hpp +++ b/libmamba/include/mamba/core/context.hpp @@ -279,6 +279,7 @@ namespace mamba bool use_only_tar_bz2 = false; + bool repodata_use_zst = false; std::vector repodata_has_zst = { "https://conda.anaconda.org/conda-forge" }; // usernames on anaconda.org can have a underscore, which influences the diff --git a/libmamba/src/api/configuration.cpp b/libmamba/src/api/configuration.cpp index 239600dc08..3d670e33a2 100644 --- a/libmamba/src/api/configuration.cpp +++ b/libmamba/src/api/configuration.cpp @@ -1232,6 +1232,11 @@ namespace mamba .set_env_var_names() .description("Permit use of the --overide-channels command-line flag")); + insert(Configurable("repodata_use_zst", &ctx.repodata_use_zst) + .group("Repodata") + .set_rc_configurable() + .description("Use zstd encoded repodata when fetching")); + insert(Configurable("repodata_has_zst", &ctx.repodata_has_zst) .group("Repodata") .set_rc_configurable() diff --git a/libmamba/src/api/info.cpp b/libmamba/src/api/info.cpp index 8e4867391a..7ab2da08d1 100644 --- a/libmamba/src/api/info.cpp +++ b/libmamba/src/api/info.cpp @@ -78,7 +78,7 @@ namespace mamba std::map items_map; for (auto& [key, val] : items) { - items_map.insert({key, val}); + items_map.insert({ key, val }); } Console::instance().json_write(items_map); @@ -89,18 +89,18 @@ namespace mamba auto& ctx = Context::instance(); std::vector> items; - items.push_back({"libmamba version", version()}); + items.push_back({ "libmamba version", version() }); if (ctx.command_params.is_micromamba && !ctx.command_params.caller_version.empty()) { - items.push_back({"micromamba version", ctx.command_params.caller_version}); + items.push_back({ "micromamba version", ctx.command_params.caller_version }); } - items.push_back({"curl version", curl_version()}); - items.push_back({"libarchive version", archive_version_details()}); + items.push_back({ "curl version", curl_version() }); + items.push_back({ "libarchive version", archive_version_details() }); - items.push_back({"envs directories", ctx.envs_dirs}); - items.push_back({"package cache", ctx.pkgs_dirs}); + items.push_back({ "envs directories", ctx.envs_dirs }); + items.push_back({ "package cache", ctx.pkgs_dirs }); std::string name, location; if (!ctx.prefix_params.target_prefix.empty()) @@ -132,25 +132,26 @@ namespace mamba name += " (not found)"; } - items.push_back({"environment", name}); - items.push_back({"env location", location}); + items.push_back({ "environment", name }); + items.push_back({ "env location", location }); // items.insert( { "shell level", { 1 } }); - items.push_back({"user config files", {(env::home_directory() / ".mambarc").string()}}); + items.push_back({ "user config files", + { (env::home_directory() / ".mambarc").string() } }); std::vector sources; for (auto s : config.valid_sources()) { sources.push_back(s.string()); }; - items.push_back({"populated config files", sources}); + items.push_back({ "populated config files", sources }); std::vector virtual_pkgs; for (auto pkg : get_virtual_packages()) { virtual_pkgs.push_back(concat(pkg.name, "=", pkg.version, "=", pkg.build_string)); } - items.push_back({"virtual packages", virtual_pkgs}); + items.push_back({ "virtual packages", virtual_pkgs }); std::vector channels = ctx.channels; // Always append context channels @@ -164,11 +165,11 @@ namespace mamba channel_urls.push_back(url); } } - items.push_back({"channels", channel_urls}); + items.push_back({ "channels", channel_urls }); - items.push_back({"base environment", ctx.prefix_params.root_prefix.string()}); + items.push_back({ "base environment", ctx.prefix_params.root_prefix.string() }); - items.push_back({"platform", ctx.platform}); + items.push_back({ "platform", ctx.platform }); info_json_print(items); info_pretty_print(items); diff --git a/libmamba/src/core/subdirdata.cpp b/libmamba/src/core/subdirdata.cpp index 2bea3603f6..54fd4ba2bd 100644 --- a/libmamba/src/core/subdirdata.cpp +++ b/libmamba/src/core/subdirdata.cpp @@ -598,30 +598,33 @@ namespace mamba auto& ctx = Context::instance(); if (!ctx.offline || forbid_cache()) { - bool has_value = m_metadata.has_zst.has_value(); - bool is_expired = m_metadata.has_zst.has_value() - && m_metadata.has_zst.value().has_expired(); - bool has_zst = m_metadata.check_zst(p_channel); - if (!has_zst && (is_expired || !has_value)) + if (ctx.repodata_use_zst) { - m_check_targets.push_back(std::make_unique( - m_name + " (check zst)", - m_repodata_url + ".zst", - "" - )); - m_check_targets.back()->set_head_only(true); - m_check_targets.back()->set_finalize_callback(&MSubdirData::finalize_check, this); - m_check_targets.back()->set_ignore_failure(true); - if (!(ctx.graphics_params.no_progress_bars || ctx.output_params.quiet - || ctx.output_params.json)) + bool has_value = m_metadata.has_zst.has_value(); + bool is_expired = m_metadata.has_zst.has_value() + && m_metadata.has_zst.value().has_expired(); + bool has_zst = m_metadata.check_zst(p_channel); + if (!has_zst && (is_expired || !has_value)) { - m_progress_bar_check = Console::instance().add_progress_bar( - m_name + " (check zst)" - ); - m_check_targets.back()->set_progress_bar(m_progress_bar_check); - m_progress_bar_check.repr().postfix.set_value("Checking"); + m_check_targets.push_back(std::make_unique( + m_name + " (check zst)", + m_repodata_url + ".zst", + "" + )); + m_check_targets.back()->set_head_only(true); + m_check_targets.back()->set_finalize_callback(&MSubdirData::finalize_check, this); + m_check_targets.back()->set_ignore_failure(true); + if (!(ctx.graphics_params.no_progress_bars || ctx.output_params.quiet + || ctx.output_params.json)) + { + m_progress_bar_check = Console::instance().add_progress_bar( + m_name + " (check zst)" + ); + m_check_targets.back()->set_progress_bar(m_progress_bar_check); + m_progress_bar_check.repr().postfix.set_value("Checking"); + } + return true; } - return true; } create_target(); } @@ -673,12 +676,15 @@ namespace mamba m_solv_cache_valid = true; } - auto state_file = json_file; - state_file.replace_extension(".state.json"); - auto lock = LockFile(state_file); - m_metadata.store_file_metadata(json_file); - auto outf = open_ofstream(state_file); - m_metadata.serialize_to_stream(outf); + if (Context::instance().repodata_use_zst) + { + auto state_file = json_file; + state_file.replace_extension(".state.json"); + auto lock = LockFile(state_file); + m_metadata.store_file_metadata(json_file); + auto outf = open_ofstream(state_file); + m_metadata.serialize_to_stream(outf); + } } bool MSubdirData::finalize_transfer(const DownloadTarget&) @@ -811,28 +817,79 @@ namespace mamba m_metadata.cache_control = m_target->get_cache_control(); m_metadata.stored_file_size = file_size; - fs::u8path state_file = json_file; - state_file.replace_extension(".state.json"); - std::error_code ec; - mamba_fs::rename_or_move(m_temp_file->path(), json_file, ec); - if (ec) + if (!Context::instance().repodata_use_zst) { - throw mamba_error( - fmt::format( - "Could not move repodata file from {} to {}: {}", - m_temp_file->path(), - json_file, - strerror(errno) - ), - mamba_error_code::subdirdata_not_loaded + LOG_DEBUG << "Opening '" << json_file.string() << "'"; + path::touch(json_file, true); + std::ofstream final_file = open_ofstream(json_file); + + if (!final_file.is_open()) + { + throw mamba_error( + fmt::format("Could not open file '{}'", json_file.string()), + mamba_error_code::subdirdata_not_loaded + ); + } + + if (m_progress_bar) + { + m_progress_bar.set_postfix("Finalizing"); + } + + std::ifstream temp_file = open_ifstream(m_temp_file->path()); + std::stringstream temp_json; + m_metadata.serialize_to_stream_tiny(temp_json); + + // replace `}` with `,` + temp_json.seekp(-1, temp_json.cur); + temp_json << ','; + final_file << temp_json.str(); + temp_file.seekg(1); + std::copy( + std::istreambuf_iterator(temp_file), + std::istreambuf_iterator(), + std::ostreambuf_iterator(final_file) ); - } - fs::last_write_time(json_file, fs::now()); - m_metadata.store_file_metadata(json_file); - std::ofstream state_file_stream = open_ofstream(state_file); - m_metadata.serialize_to_stream(state_file_stream); + if (!temp_file) + { + std::error_code ec; + fs::remove(json_file, ec); + if (ec) + { + LOG_ERROR << "Could not remove file " << json_file << ": " << ec.message(); + } + throw mamba_error( + fmt::format("Could not write out repodata file {}: {}", json_file, strerror(errno)), + mamba_error_code::subdirdata_not_loaded + ); + } + fs::last_write_time(json_file, fs::now()); + } + else + { + fs::u8path state_file = json_file; + state_file.replace_extension(".state.json"); + std::error_code ec; + mamba_fs::rename_or_move(m_temp_file->path(), json_file, ec); + if (ec) + { + throw mamba_error( + fmt::format( + "Could not move repodata file from {} to {}: {}", + m_temp_file->path(), + json_file, + strerror(errno) + ), + mamba_error_code::subdirdata_not_loaded + ); + } + fs::last_write_time(json_file, fs::now()); + m_metadata.store_file_metadata(json_file); + std::ofstream state_file_stream = open_ofstream(state_file); + m_metadata.serialize_to_stream(state_file_stream); + } if (m_progress_bar) { m_progress_bar.repr().postfix.set_value("Downloaded").deactivate(); diff --git a/libmamba/tests/src/core/test_cpp.cpp b/libmamba/tests/src/core/test_cpp.cpp index 27abaf2131..7e291f0d1a 100644 --- a/libmamba/tests/src/core/test_cpp.cpp +++ b/libmamba/tests/src/core/test_cpp.cpp @@ -587,6 +587,8 @@ namespace mamba { TEST_CASE("parse_mod_etag") { + bool old_value = Context::instance().repodata_use_zst; + Context::instance().repodata_use_zst = true; fs::u8path cache_folder = fs::u8path{ test_data_dir / "repodata_json_cache" }; auto mq = detail::read_metadata(cache_folder / "test_1.json"); CHECK(mq.has_value()); @@ -663,6 +665,8 @@ namespace mamba CHECK_EQ(j.url, "https://conda.anaconda.org/conda-forge/noarch/repodata.json.zst"); CHECK_EQ(j.has_zst.value().value, true); CHECK_EQ(j.has_zst.value().last_checked, parse_utc_timestamp("2023-01-06T16:33:06Z")); + + Context::instance().repodata_use_zst = old_value; } } } // namespace mamba diff --git a/libmambapy/libmambapy/__init__.pyi b/libmambapy/libmambapy/__init__.pyi index 648982ddb9..234b8e0b79 100644 --- a/libmambapy/libmambapy/__init__.pyi +++ b/libmambapy/libmambapy/__init__.pyi @@ -1533,8 +1533,6 @@ class SubdirData: ) -> None: ... def cache_path(self) -> str: ... def create_repo(self, arg0: Pool) -> Repo: ... - def download_and_check_targets(self, arg0: DownloadTargetList) -> bool: ... - def finalize_checks(self) -> None: ... def loaded(self) -> bool: ... pass diff --git a/libmambapy/src/main.cpp b/libmambapy/src/main.cpp index 8e99ae69ff..fc066a7c63 100644 --- a/libmambapy/src/main.cpp +++ b/libmambapy/src/main.cpp @@ -136,7 +136,6 @@ PYBIND11_MODULE(bindings, m) auto pyPackageInfo = py::class_(m, "PackageInfo"); auto pyPrefixData = py::class_(m, "PrefixData"); auto pySolver = py::class_(m, "Solver"); - auto pyMultiDownloadTarget = py::class_(m, "DownloadTargetList"); // only used in a return type; does it belong in the module? auto pyRootRole = py::class_(m, "RootRole"); @@ -474,25 +473,13 @@ PYBIND11_MODULE(bindings, m) .def( "cache_path", [](const MSubdirData& self) -> std::string { return extract(self.cache_path()); } - ) - .def( - "download_and_check_targets", - [](MSubdirData& self, MultiDownloadTarget& multi_download) -> bool - { - for (auto& check_target : self.check_targets()) - { - multi_download.add(check_target.get()); - } - multi_download.download(MAMBA_NO_CLEAR_PROGRESS_BARS); - return self.check_targets().size(); - } - ) - .def("finalize_checks", &MSubdirData::finalize_checks); + ); m.def("cache_fn_url", &cache_fn_url); m.def("create_cache_dir", &create_cache_dir); - pyMultiDownloadTarget.def(py::init<>()) + py::class_(m, "DownloadTargetList") + .def(py::init<>()) .def( "add", [](MultiDownloadTarget& self, MSubdirData& sub) -> void { self.add(sub.target()); } diff --git a/mamba/mamba/utils.py b/mamba/mamba/utils.py index 5ad1717be2..6d542bdbe5 100644 --- a/mamba/mamba/utils.py +++ b/mamba/mamba/utils.py @@ -102,23 +102,10 @@ def fixup_channel_spec(spec): channel, channel_platform, full_url, pkgs_dirs, repodata_fn ) - needs_finalising = sd.download_and_check_targets(dlist) index.append( - ( - sd, - { - "platform": channel_platform, - "url": url, - "channel": channel, - "needs_finalising": needs_finalising, - }, - ) + (sd, {"platform": channel_platform, "url": url, "channel": channel}) ) - - for (sd, info) in index: - if info["needs_finalising"]: - sd.finalize_checks() - dlist.add(sd) + dlist.add(sd) is_downloaded = dlist.download(api.MAMBA_DOWNLOAD_FAILFAST)