From 95eed176fb259ee75c252c0cfc1640fd92cd1269 Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Fri, 19 Jun 2026 08:42:21 +0200 Subject: [PATCH 1/7] fix: Clear stale negative cache entries Signed-off-by: Julien Jerphanion --- .../include/mamba/core/package_fetcher.hpp | 1 + libmamba/src/core/package_cache.cpp | 3 + libmamba/src/core/package_fetcher.cpp | 2 + libmamba/src/core/transaction.cpp | 55 ++++++++++++++++++- .../tests/src/core/test_package_cache.cpp | 16 ++++++ 5 files changed, 76 insertions(+), 1 deletion(-) diff --git a/libmamba/include/mamba/core/package_fetcher.hpp b/libmamba/include/mamba/core/package_fetcher.hpp index bf326b59b2..3d1578ada3 100644 --- a/libmamba/include/mamba/core/package_fetcher.hpp +++ b/libmamba/include/mamba/core/package_fetcher.hpp @@ -116,6 +116,7 @@ namespace mamba void update_monitor(progress_callback_t* cb, PackageExtractEvent event) const; specs::PackageInfo m_package_info; + MultiPackageCache* m_caches = nullptr; fs::u8path m_tarball_path; fs::u8path m_cache_path; diff --git a/libmamba/src/core/package_cache.cpp b/libmamba/src/core/package_cache.cpp index fc8d023104..abf428e926 100644 --- a/libmamba/src/core/package_cache.cpp +++ b/libmamba/src/core/package_cache.cpp @@ -656,6 +656,9 @@ namespace mamba void MultiPackageCache::clear_query_cache(const specs::PackageInfo& s) { + const std::string pkg = s.long_str(); + m_cached_tarballs.erase(pkg); + m_cached_extracted_dirs.erase(pkg); for (auto& c : m_caches) { c.clear_query_cache(s); diff --git a/libmamba/src/core/package_fetcher.cpp b/libmamba/src/core/package_fetcher.cpp index 353914aac0..7f9d4fcfe5 100644 --- a/libmamba/src/core/package_fetcher.cpp +++ b/libmamba/src/core/package_fetcher.cpp @@ -140,6 +140,7 @@ namespace mamba PackageFetcher::PackageFetcher(const specs::PackageInfo& pkg_info, MultiPackageCache& caches) : m_package_info(pkg_info) + , m_caches(&caches) { const fs::u8path extracted_cache = caches.get_extracted_dir_path(m_package_info); if (extracted_cache.empty()) @@ -360,6 +361,7 @@ namespace mamba LOG_DEBUG << "Extracted to '" << extract_path.string() << "'"; write_repodata_record(extract_path); update_urls_txt(); + m_caches->clear_query_cache(m_package_info); update_monitor(cb, PackageExtractEvent::extract_success); } catch (const std::logic_error&) diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp index 3a4d58dc94..d66032a9e8 100644 --- a/libmamba/src/core/transaction.cpp +++ b/libmamba/src/core/transaction.cpp @@ -26,6 +26,7 @@ #include "mamba/core/execution.hpp" #include "mamba/core/output.hpp" #include "mamba/core/package_fetcher.hpp" +#include "mamba/core/package_handling.hpp" #include "mamba/core/repo_checker_store.hpp" #include "mamba/core/thread_utils.hpp" #include "mamba/core/transaction.hpp" @@ -57,6 +58,58 @@ namespace mamba && caches.get_tarball_path(pkg_info).empty(); } + /** + * Resolve the extracted package cache directory for linking. + * + * Clears stale negative cache entries (e.g. from before fetch/extract in the same + * transaction) and, if needed, re-extracts from a cached tarball at link time. + */ + fs::u8path resolve_extracted_cache_path( + const specs::PackageInfo& pkg, + MultiPackageCache& caches, + const Context& ctx + ) + { + auto lookup = [&]() { return caches.get_extracted_dir_path(pkg); }; + + if (auto path = lookup(); !path.empty()) + { + return path; + } + + caches.clear_query_cache(pkg); + if (auto path = lookup(); !path.empty()) + { + return path; + } + + PackageFetcher fetcher(pkg, caches); + if (fetcher.needs_download()) + { + LOG_ERROR << "Cannot find a valid extracted directory cache for '" << pkg.filename + << "'"; + throw std::runtime_error("Package cache error."); + } + + if (fetcher.needs_extract()) + { + const auto extract_options = ExtractOptions::from_context(ctx); + if (!fetcher.extract(extract_options)) + { + LOG_ERROR << "Failed to extract package '" << pkg.filename << "' for linking"; + throw std::runtime_error("Package cache error."); + } + caches.clear_query_cache(pkg); + if (auto path = lookup(); !path.empty()) + { + return path; + } + } + + LOG_ERROR << "Cannot find a valid extracted directory cache for '" << pkg.filename << "'"; + throw std::runtime_error("Package cache error."); + } + auto explicit_spec(const specs::PackageInfo& pkg) -> specs::MatchSpec { auto out = specs::MatchSpec(); @@ -764,7 +817,7 @@ namespace mamba } Console::stream() << "Linking " << pkg.str(); - const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg, false)); + const fs::u8path cache_path(resolve_extracted_cache_path(pkg, m_multi_cache, ctx)); LinkPackage lp(pkg, cache_path, &transaction_context); try { diff --git a/libmamba/tests/src/core/test_package_cache.cpp b/libmamba/tests/src/core/test_package_cache.cpp index a2bbe76094..025e64670c 100644 --- a/libmamba/tests/src/core/test_package_cache.cpp +++ b/libmamba/tests/src/core/test_package_cache.cpp @@ -177,5 +177,21 @@ namespace mamba MultiPackageCache cache({ long_pkgs_dir }, params); REQUIRE(cache.get_extracted_dir_path(pkg_info) == long_pkgs_dir / rel_path); } + + SECTION("Stale negative cache is cleared and package is found") + { + MultiPackageCache cache({ pkgs_dir }, params); + + // Negative lookup is cached before the extracted directory exists + REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); + + write_repodata_record(hierarchical_dir / "info" / "repodata_record.json", pkg_info); + + // Without clearing the query cache, the negative result is reused + REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); + + cache.clear_query_cache(pkg_info); + REQUIRE(cache.get_extracted_dir_path(pkg_info) == pkgs_dir / rel_path); + } } } // namespace mamba From 94ba42982a48f1f7cbcc36c5eed823ba7a6b1001 Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Mon, 22 Jun 2026 15:34:34 +0200 Subject: [PATCH 2/7] Add non-regression C++ tests Signed-off-by: Julien Jerphanion --- libmamba/tests/CMakeLists.txt | 1 + .../tests/src/core/test_package_cache.cpp | 30 ++ libmamba/tests/src/core/test_transaction.cpp | 401 ++++++++++++++++++ 3 files changed, 432 insertions(+) create mode 100644 libmamba/tests/src/core/test_transaction.cpp diff --git a/libmamba/tests/CMakeLists.txt b/libmamba/tests/CMakeLists.txt index db0e38d240..9e823d7971 100644 --- a/libmamba/tests/CMakeLists.txt +++ b/libmamba/tests/CMakeLists.txt @@ -122,6 +122,7 @@ set( src/core/test_subdir_index.cpp src/core/test_tasksync.cpp src/core/test_thread_utils.cpp + src/core/test_transaction.cpp src/core/test_transaction_context.cpp src/core/test_util.cpp src/core/test_virtual_packages.cpp diff --git a/libmamba/tests/src/core/test_package_cache.cpp b/libmamba/tests/src/core/test_package_cache.cpp index 025e64670c..96b552c283 100644 --- a/libmamba/tests/src/core/test_package_cache.cpp +++ b/libmamba/tests/src/core/test_package_cache.cpp @@ -193,5 +193,35 @@ namespace mamba cache.clear_query_cache(pkg_info); REQUIRE(cache.get_extracted_dir_path(pkg_info) == pkgs_dir / rel_path); } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + SECTION("Invalid extracted cache with missing paths.json file is rejected #4322") + { + auto warn_params = ctx.validation_params; + warn_params.safety_checks = VerificationLevel::Warn; + + const fs::u8path extract_dir = flat_dir; + fs::create_directories(extract_dir / "info"); + write_repodata_record(extract_dir / "info" / "repodata_record.json", pkg_info); + + nlohmann::json paths_json; + paths_json["paths_version"] = 1; + paths_json["paths"] = nlohmann::json::array( + { + nlohmann::json{ + { "_path", "etc/conda/test-files/missing-file.txt" }, + { "path_type", "hardlink" }, + { "size_in_bytes", 42 }, + }, + } + ); + { + std::ofstream paths_out((extract_dir / "info" / "paths.json").std_path()); + paths_out << paths_json.dump(); + } + + MultiPackageCache cache({ pkgs_dir }, warn_params); + REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); + } } } // namespace mamba diff --git a/libmamba/tests/src/core/test_transaction.cpp b/libmamba/tests/src/core/test_transaction.cpp new file mode 100644 index 0000000000..132484ac38 --- /dev/null +++ b/libmamba/tests/src/core/test_transaction.cpp @@ -0,0 +1,401 @@ +// Copyright (c) 2026, QuantStack and Mamba Contributors +// +// Distributed under the terms of the BSD 3-Clause License. +// +// The full license is in the file LICENSE, distributed with this software. + +#include +#include +#include + +#include +#include +#include + +#include "mamba/core/channel_context.hpp" +#include "mamba/core/context.hpp" +#include "mamba/core/package_cache.hpp" +#include "mamba/core/package_handling.hpp" +#include "mamba/core/transaction.hpp" +#include "mamba/core/util.hpp" +#include "mamba/fs/filesystem.hpp" +#include "mamba/solver/libsolv/database.hpp" +#include "mamba/solver/solution.hpp" +#include "mamba/specs/archive.hpp" +#include "mamba/validation/tools.hpp" + +#include "core/link.hpp" +#include "core/transaction_context.hpp" + +#include "mambatests.hpp" + +namespace mamba +{ + namespace + { + struct ScopedContextOverride + { + ScopedContextOverride(Context& ctx, const fs::u8path& prefix, const fs::u8path& pkgs_dir) + { + m_ctx = &ctx; + m_saved_prefix_params = ctx.prefix_params; + m_saved_pkgs_dirs = ctx.pkgs_dirs; + m_saved_prefix_data_interoperability = ctx.prefix_data_interoperability; + m_saved_extract_threads = ctx.threads_params.extract_threads; + + ctx.prefix_params.target_prefix = prefix; + ctx.prefix_params.root_prefix = prefix; + ctx.prefix_params.conda_prefix = prefix; + ctx.prefix_params.relocate_prefix = prefix; + ctx.pkgs_dirs = { pkgs_dir }; + ctx.prefix_data_interoperability = false; + ctx.threads_params.extract_threads = 1; + } + + ~ScopedContextOverride() + { + m_ctx->prefix_params = m_saved_prefix_params; + m_ctx->pkgs_dirs = m_saved_pkgs_dirs; + m_ctx->prefix_data_interoperability = m_saved_prefix_data_interoperability; + m_ctx->threads_params.extract_threads = m_saved_extract_threads; + } + + Context* m_ctx = nullptr; + PrefixParams m_saved_prefix_params; + std::vector m_saved_pkgs_dirs; + bool m_saved_prefix_data_interoperability = false; + int m_saved_extract_threads = 0; + }; + + void write_text_file(const fs::u8path& path, std::string_view content) + { + fs::create_directories(path.parent_path()); + std::ofstream out(path.std_path()); + out << content; + } + + specs::PackageInfo make_test_package(const std::string& name) + { + specs::PackageInfo pkg(name); + pkg.version = "1.0.0"; + pkg.build_string = "h0_0"; + pkg.build_number = 0; + pkg.platform = "noarch"; + pkg.channel = "https://conda.anaconda.org/conda-forge/noarch"; + pkg.filename = fmt::format("{}-{}-{}.tar.bz2", name, pkg.version, pkg.build_string); + pkg.package_url = pkg.channel + "/" + pkg.filename; + return pkg; + } + + void write_repodata_record(const fs::u8path& extract_dir, const specs::PackageInfo& pkg) + { + nlohmann::json repodata{ + { "name", pkg.name }, { "version", pkg.version }, + { "build", pkg.build_string }, { "build_number", pkg.build_number }, + { "url", pkg.package_url }, { "channel", pkg.channel }, + { "size", pkg.size }, + }; + if (!pkg.md5.empty()) + { + repodata["md5"] = pkg.md5; + } + if (!pkg.sha256.empty()) + { + repodata["sha256"] = pkg.sha256; + } + write_text_file(extract_dir / "info" / "repodata_record.json", repodata.dump()); + } + + void create_extracted_package( + const fs::u8path& pkgs_dir, + const specs::PackageInfo& pkg, + const std::string& rel_file, + std::string_view content, + bool include_file = true + ) + { + const std::string pkg_dir_name = std::string(specs::strip_archive_extension(pkg.filename)); + const fs::u8path extract_dir = pkgs_dir / pkg_dir_name; + const fs::u8path file_path = extract_dir / rel_file; + + if (include_file) + { + write_text_file(file_path, content); + } + + nlohmann::json paths_json; + paths_json["paths_version"] = 1; + if (include_file) + { + paths_json["paths"] = nlohmann::json::array( + { + nlohmann::json{ + { "_path", rel_file }, + { "path_type", "hardlink" }, + { "sha256", validation::sha256sum(file_path) }, + { "size_in_bytes", fs::file_size(file_path) }, + }, + } + ); + } + else + { + paths_json["paths"] = nlohmann::json::array( + { + nlohmann::json{ + { "_path", rel_file }, + { "path_type", "hardlink" }, + { "sha256", std::string(64, '0') }, + { "size_in_bytes", 42 }, + }, + } + ); + } + + nlohmann::json index_json{ + { "name", pkg.name }, + { "version", pkg.version }, + { "build", pkg.build_string }, + { "build_number", pkg.build_number }, + }; + + write_text_file(extract_dir / "info" / "paths.json", paths_json.dump()); + write_text_file(extract_dir / "info" / "index.json", index_json.dump()); + write_repodata_record(extract_dir, pkg); + } + + TransactionContext make_transaction_context(const fs::u8path& prefix) + { + TransactionParams tx_params{ + .is_mamba_exe = false, + .json_output = false, + .verbosity = 0, + .shortcuts = false, + .envs_dirs = {}, + .platform = "noarch", + .prefix_params = + PrefixParams{ + .target_prefix = prefix, + .root_prefix = prefix, + .conda_prefix = prefix, + .relocate_prefix = prefix, + }, + .link_params = { .compile_pyc = false }, + .threads_params = {}, + }; + return TransactionContext(tx_params, { "", "" }, "", {}); + } + + void link_package_to_prefix( + const specs::PackageInfo& pkg, + const fs::u8path& pkgs_dir, + const fs::u8path& prefix + ) + { + fs::create_directories(prefix / "conda-meta"); + auto tx_context = make_transaction_context(prefix); + LinkPackage linker(pkg, pkgs_dir, &tx_context); + REQUIRE(linker.execute()); + } + + bool prefix_has_package_file( + const fs::u8path& prefix, + const specs::PackageInfo& pkg, + const std::string& rel_file + ) + { + return fs::exists(prefix / rel_file) + && fs::exists(prefix / "conda-meta" / (pkg.str() + ".json")); + } + + void rollback_recorded_link_and_unlink_operations( + std::stack& linked_packages, + std::stack& unlinked_packages + ) + { + while (!linked_packages.empty()) + { + linked_packages.top().undo(); + linked_packages.pop(); + } + + while (!unlinked_packages.empty()) + { + unlinked_packages.top().undo(); + unlinked_packages.pop(); + } + } + } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + // Invalid extracted caches (missing files listed in paths.json) must not be used for linking. + TEST_CASE( + "Invalid extracted cache is rejected when paths.json file is missing #4322", + "[mamba::core][transaction][regression][4322]" + ) + { + auto& ctx = mambatests::context(); + TemporaryDirectory temp_dir; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + + auto pkg = make_test_package("pkg-cache-a"); + const std::string rel_file = "share/pkg-cache-a/hello.txt"; + create_extracted_package(pkgs_dir, pkg, rel_file, "hello\n", /* include_file= */ false); + + MultiPackageCache cache({ pkgs_dir }, ctx.validation_params); + REQUIRE(cache.get_extracted_dir_path(pkg).empty()); + } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + // After extracting from a tarball, stale negative cache entries must not block linking. + TEST_CASE( + "Stale negative cache recovers after tarball extraction #4322", + "[mamba::core][transaction][regression][4322]" + ) + { + auto& ctx = mambatests::context(); + TemporaryDirectory temp_dir; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + + auto pkg = make_test_package("pkg-cache-b"); + const std::string rel_file = "share/pkg-cache-b/hello.txt"; + const std::string pkg_dir_name = std::string(specs::strip_archive_extension(pkg.filename)); + const fs::u8path extract_dir = pkgs_dir / pkg_dir_name; + + create_extracted_package(pkgs_dir, pkg, rel_file, "hello\n"); + create_archive( + extract_dir, + pkgs_dir / pkg.filename, + compression_algorithm::bzip2, + /* compression_level= */ 1, + /* compression_threads= */ 1, + /* filter= */ nullptr + ); + + // Corrupt the extracted directory while keeping a valid tarball, as in the issue report. + fs::remove(extract_dir / rel_file); + + MultiPackageCache cache({ pkgs_dir }, ctx.validation_params); + REQUIRE(cache.get_extracted_dir_path(pkg).empty()); + + ExtractOptions options; + options.sparse = false; + options.subproc_mode = extract_subproc_mode::mamba_package; + fs::remove_all(extract_dir); + mamba::extract(pkgs_dir / pkg.filename, extract_dir, options); + write_repodata_record(extract_dir, pkg); + cache.clear_query_cache(pkg); + + REQUIRE(cache.get_extracted_dir_path(pkg) == pkgs_dir); + REQUIRE(fs::exists(extract_dir / rel_file)); + } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + // A package cache error during linking must roll back earlier unlink/link operations. + TEST_CASE( + "Link and unlink rollback restores prefix after package cache error #4322", + "[mamba::core][transaction][regression][4322]" + ) + { + TemporaryDirectory temp_dir; + const fs::u8path prefix = temp_dir.path() / "prefix"; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + + auto pkg_a = make_test_package("pkg-a"); + auto pkg_b = make_test_package("pkg-b"); + const std::string rel_file = "share/pkg-a/hello.txt"; + create_extracted_package(pkgs_dir, pkg_a, rel_file, "hello from pkg-a\n"); + link_package_to_prefix(pkg_a, pkgs_dir, prefix); + REQUIRE(prefix_has_package_file(prefix, pkg_a, rel_file)); + + auto tx_context = make_transaction_context(prefix); + std::stack unlinked_packages; + std::stack linked_packages; + + UnlinkPackage unlink_pkg_a(pkg_a, pkgs_dir, &tx_context); + REQUIRE(unlink_pkg_a.execute()); + unlinked_packages.push(std::move(unlink_pkg_a)); + REQUIRE_FALSE(fs::exists(prefix / rel_file)); + + LinkPackage relink_pkg_a(pkg_a, pkgs_dir, &tx_context); + REQUIRE(relink_pkg_a.execute()); + linked_packages.push(std::move(relink_pkg_a)); + REQUIRE(prefix_has_package_file(prefix, pkg_a, rel_file)); + + LinkPackage link_pkg_b(pkg_b, pkgs_dir, &tx_context); + REQUIRE_THROWS_AS(link_pkg_b.execute(), std::runtime_error); + + rollback_recorded_link_and_unlink_operations(linked_packages, unlinked_packages); + REQUIRE(prefix_has_package_file(prefix, pkg_a, rel_file)); + REQUIRE_FALSE(fs::exists(prefix / "conda-meta" / (pkg_b.str() + ".json"))); + } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + // When a package cannot be fetched, the prefix must remain unchanged. + TEST_CASE( + "Transaction leaves prefix intact when fetch fails before linking #4322", + "[mamba::core][transaction][regression][4322]" + ) + { + auto& ctx = mambatests::context(); + TemporaryDirectory temp_dir; + const fs::u8path prefix = temp_dir.path() / "prefix"; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + ScopedContextOverride context_guard(ctx, prefix, pkgs_dir); + + auto pkg_a = make_test_package("pkg-a"); + auto pkg_b = make_test_package("pkg-b"); + const std::string rel_file = "share/pkg-a/hello.txt"; + create_extracted_package(pkgs_dir, pkg_a, rel_file, "hello from pkg-a\n"); + link_package_to_prefix(pkg_a, pkgs_dir, prefix); + REQUIRE(prefix_has_package_file(prefix, pkg_a, rel_file)); + + auto channel_context = ChannelContext::make_simple(ctx); + auto db = solver::libsolv::Database(channel_context.params()); + PrefixData prefix_data = PrefixData::create(prefix, channel_context, true).value(); + MultiPackageCache package_caches({ pkgs_dir }, ctx.validation_params); + + solver::Request request; + solver::Solution solution{ { + solver::Solution::Remove{ pkg_a }, + solver::Solution::Install{ pkg_a }, + solver::Solution::Install{ pkg_b }, + } }; + + MTransaction transaction(ctx, db, request, std::move(solution), package_caches); + + REQUIRE_THROWS_AS(transaction.execute(ctx, channel_context, prefix_data), std::exception); + REQUIRE(prefix_has_package_file(prefix, pkg_a, rel_file)); + REQUIRE_FALSE(fs::exists(prefix / "conda-meta" / (pkg_b.str() + ".json"))); + } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 + TEST_CASE( + "UnlinkPackage undo restores a previously linked package #4322", + "[mamba::core][transaction][regression][4322]" + ) + { + TemporaryDirectory temp_dir; + const fs::u8path prefix = temp_dir.path() / "prefix"; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + + auto pkg = make_test_package("pkg-undo"); + const std::string rel_file = "share/pkg-undo/hello.txt"; + create_extracted_package(pkgs_dir, pkg, rel_file, "undo me\n"); + link_package_to_prefix(pkg, pkgs_dir, prefix); + REQUIRE(prefix_has_package_file(prefix, pkg, rel_file)); + + auto tx_context = make_transaction_context(prefix); + UnlinkPackage unlinker(pkg, pkgs_dir, &tx_context); + REQUIRE(unlinker.execute()); + REQUIRE_FALSE(fs::exists(prefix / rel_file)); + + REQUIRE(unlinker.undo()); + REQUIRE(prefix_has_package_file(prefix, pkg, rel_file)); + } +} // namespace mamba From a41e13e888753253bc42c523906beac3e4d69a5b Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Mon, 22 Jun 2026 16:59:02 +0200 Subject: [PATCH 3/7] Store `MultiPackageCache` using a shared pointer Signed-off-by: Julien Jerphanion --- libmamba/include/mamba/core/package_fetcher.hpp | 3 ++- libmamba/src/core/package_fetcher.cpp | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libmamba/include/mamba/core/package_fetcher.hpp b/libmamba/include/mamba/core/package_fetcher.hpp index 3d1578ada3..3b6b293336 100644 --- a/libmamba/include/mamba/core/package_fetcher.hpp +++ b/libmamba/include/mamba/core/package_fetcher.hpp @@ -8,6 +8,7 @@ #define MAMBA_CORE_PACKAGE_FETCHER_HPP #include +#include #include "mamba/core/package_cache.hpp" #include "mamba/core/package_handling.hpp" @@ -116,7 +117,7 @@ namespace mamba void update_monitor(progress_callback_t* cb, PackageExtractEvent event) const; specs::PackageInfo m_package_info; - MultiPackageCache* m_caches = nullptr; + std::shared_ptr m_caches; fs::u8path m_tarball_path; fs::u8path m_cache_path; diff --git a/libmamba/src/core/package_fetcher.cpp b/libmamba/src/core/package_fetcher.cpp index 7f9d4fcfe5..216404de3e 100644 --- a/libmamba/src/core/package_fetcher.cpp +++ b/libmamba/src/core/package_fetcher.cpp @@ -140,13 +140,13 @@ namespace mamba PackageFetcher::PackageFetcher(const specs::PackageInfo& pkg_info, MultiPackageCache& caches) : m_package_info(pkg_info) - , m_caches(&caches) + , m_caches(&caches, [](MultiPackageCache*) {}) { - const fs::u8path extracted_cache = caches.get_extracted_dir_path(m_package_info); + const fs::u8path extracted_cache = m_caches->get_extracted_dir_path(m_package_info); if (extracted_cache.empty()) { - const fs::u8path tarball_cache = caches.get_tarball_path(m_package_info); - auto& cache = caches.first_writable_cache(true); + const fs::u8path tarball_cache = m_caches->get_tarball_path(m_package_info); + auto& cache = m_caches->first_writable_cache(true); m_cache_path = cache.path() / package_cache_folder_relative_path(m_package_info); fs::create_directories(m_cache_path); @@ -160,7 +160,7 @@ namespace mamba } else { - caches.clear_query_cache(m_package_info); + m_caches->clear_query_cache(m_package_info); // need to download this file const DownloadRequestComponents components = get_download_request_components( m_package_info From edb439b65a5c78baa6e5503c65c0048d2ff52028 Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Tue, 23 Jun 2026 11:37:47 +0200 Subject: [PATCH 4/7] fix: Purge invalid hierarchical package cache before re-extract Signed-off-by: Julien Jerphanion --- libmamba/include/mamba/core/package_cache.hpp | 2 + libmamba/src/core/package_cache.cpp | 20 +++++ libmamba/src/core/package_fetcher.cpp | 18 ++++- libmamba/src/core/transaction.cpp | 4 + .../tests/src/core/test_package_cache.cpp | 37 +++++++++ .../tests/src/core/test_package_fetcher.cpp | 76 +++++++++++++++++++ 6 files changed, 155 insertions(+), 2 deletions(-) diff --git a/libmamba/include/mamba/core/package_cache.hpp b/libmamba/include/mamba/core/package_cache.hpp index 57fc9f616f..106e57ef49 100644 --- a/libmamba/include/mamba/core/package_cache.hpp +++ b/libmamba/include/mamba/core/package_cache.hpp @@ -108,6 +108,7 @@ namespace mamba Writable is_writable(); fs::u8path path() const; void clear_query_cache(const specs::PackageInfo& s); + void remove_extracted_package(const specs::PackageInfo& s); bool has_valid_tarball(const specs::PackageInfo& s, const ValidationParams& params); bool has_valid_extracted_dir(const specs::PackageInfo& s, const ValidationParams& params); @@ -138,6 +139,7 @@ namespace mamba std::vector writable_caches(); void clear_query_cache(const specs::PackageInfo& s); + void remove_extracted_package(const specs::PackageInfo& s); private: diff --git a/libmamba/src/core/package_cache.cpp b/libmamba/src/core/package_cache.cpp index abf428e926..a022009ffb 100644 --- a/libmamba/src/core/package_cache.cpp +++ b/libmamba/src/core/package_cache.cpp @@ -148,6 +148,15 @@ namespace mamba m_valid_extracted_dir.erase(s.long_str()); } + void PackageCacheData::remove_extracted_package(const specs::PackageInfo& s) + { + const auto folder = package_cache_folder_relative_path(s); + const fs::u8path pkg_name = specs::strip_archive_extension(s.filename); + fs::remove_all(m_path / folder / pkg_name); + fs::remove_all(m_path / pkg_name); + clear_query_cache(s); + } + void PackageCacheData::check_writable() { fs::u8path magic_file = m_path / PACKAGE_CACHE_MAGIC_FILE; @@ -664,4 +673,15 @@ namespace mamba c.clear_query_cache(s); } } + + void MultiPackageCache::remove_extracted_package(const specs::PackageInfo& s) + { + for (auto& c : m_caches) + { + c.remove_extracted_package(s); + } + const std::string pkg = s.long_str(); + m_cached_tarballs.erase(pkg); + m_cached_extracted_dirs.erase(pkg); + } } // namespace mamba diff --git a/libmamba/src/core/package_fetcher.cpp b/libmamba/src/core/package_fetcher.cpp index 216404de3e..85db50b625 100644 --- a/libmamba/src/core/package_fetcher.cpp +++ b/libmamba/src/core/package_fetcher.cpp @@ -392,9 +392,23 @@ namespace mamba void PackageFetcher::clear_cache() const { + const auto remove_extracted_at = [&](const fs::u8path& cache_path) + { fs::remove_all(get_extract_path(filename(), cache_path)); }; + fs::remove_all(m_tarball_path); - const fs::u8path dest_dir = specs::strip_archive_extension(m_tarball_path.string()); - fs::remove_all(dest_dir); + remove_extracted_at(m_cache_path); + + // Tarballs may use the flat layout while extraction uses the hierarchical layout. + const fs::u8path tarball_parent = m_tarball_path.parent_path(); + if (tarball_parent != m_cache_path) + { + remove_extracted_at(tarball_parent); + } + + if (m_caches) + { + m_caches->clear_query_cache(m_package_info); + } } /******************* diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp index d66032a9e8..ee745259e5 100644 --- a/libmamba/src/core/transaction.cpp +++ b/libmamba/src/core/transaction.cpp @@ -83,6 +83,10 @@ namespace mamba return path; } + // Drop invalid or partial extracted directories so a cached tarball can be + // re-extracted (e.g. after a failed extraction left stale files behind). + caches.remove_extracted_package(pkg); + PackageFetcher fetcher(pkg, caches); if (fetcher.needs_download()) { diff --git a/libmamba/tests/src/core/test_package_cache.cpp b/libmamba/tests/src/core/test_package_cache.cpp index 96b552c283..f329b06b1e 100644 --- a/libmamba/tests/src/core/test_package_cache.cpp +++ b/libmamba/tests/src/core/test_package_cache.cpp @@ -223,5 +223,42 @@ namespace mamba MultiPackageCache cache({ pkgs_dir }, warn_params); REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4331#issuecomment-4771693736 + SECTION("remove_extracted_package clears invalid hierarchical cache #4331") + { + auto warn_params = ctx.validation_params; + warn_params.safety_checks = VerificationLevel::Warn; + + const fs::u8path invalid_hierarchical_dir = pkgs_dir / rel_path + / "test-pkg-1.0.0-h123456_0"; + fs::create_directories(invalid_hierarchical_dir / "info"); + write_repodata_record(invalid_hierarchical_dir / "info" / "repodata_record.json", pkg_info); + + nlohmann::json paths_json; + paths_json["paths_version"] = 1; + paths_json["paths"] = nlohmann::json::array( + { + nlohmann::json{ + { "_path", "etc/conda/test-files/missing-file.txt" }, + { "path_type", "hardlink" }, + { "size_in_bytes", 42 }, + }, + } + ); + { + std::ofstream paths_out((invalid_hierarchical_dir / "info" / "paths.json").std_path()); + paths_out << paths_json.dump(); + } + + MultiPackageCache cache({ pkgs_dir }, warn_params); + REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); + REQUIRE(fs::exists(invalid_hierarchical_dir)); + + cache.remove_extracted_package(pkg_info); + + REQUIRE_FALSE(fs::exists(invalid_hierarchical_dir)); + REQUIRE(cache.get_extracted_dir_path(pkg_info).empty()); + } } } // namespace mamba diff --git a/libmamba/tests/src/core/test_package_fetcher.cpp b/libmamba/tests/src/core/test_package_fetcher.cpp index 439781e4af..daff879d60 100644 --- a/libmamba/tests/src/core/test_package_fetcher.cpp +++ b/libmamba/tests/src/core/test_package_fetcher.cpp @@ -15,6 +15,7 @@ #include "mamba/core/package_handling.hpp" #include "mamba/core/util.hpp" #include "mamba/fs/filesystem.hpp" +#include "mamba/specs/archive.hpp" #include "mamba/util/url_manip.hpp" #include "mambatests.hpp" @@ -1300,4 +1301,79 @@ namespace REQUIRE(repodata_record.contains("size")); CHECK(repodata_record["size"] == tarball_size); } + + // Non-regression for https://github.com/mamba-org/mamba/issues/4322 and + // https://github.com/mamba-org/mamba/issues/4331#issuecomment-4771693736 + // When the tarball uses the flat layout but extraction targets the hierarchical layout, + // clear_cache() must remove the hierarchical extracted directory. + TEST_CASE("PackageFetcher::clear_cache removes hierarchical extracted directory", "[regression][4322][4331]") + { + auto& ctx = mambatests::context(); + TemporaryDirectory temp_dir; + const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; + fs::create_directories(pkgs_dir); + MultiPackageCache package_caches({ pkgs_dir }, ctx.validation_params); + + specs::PackageInfo pkg; + pkg.name = "cuda-nvvp"; + pkg.version = "12.9.79"; + pkg.build_string = "he0c23c2_0"; + pkg.platform = "win-64"; + pkg.channel = "conda-forge"; + pkg.package_url = "https://conda.anaconda.org/conda-forge/win-64/cuda-nvvp-12.9.79-he0c23c2_0.conda"; + pkg.filename = "cuda-nvvp-12.9.79-he0c23c2_0.conda"; + + const auto cache_subdir = package_cache_folder_relative_path(pkg); + const std::string pkg_basename = std::string(specs::strip_archive_extension(pkg.filename)); + const fs::u8path hierarchical_extract = pkgs_dir / cache_subdir / pkg_basename; + const fs::u8path flat_tarball = pkgs_dir / pkg.filename; + + // Build a valid tarball at the flat cache location. + const fs::u8path build_dir = temp_dir.path() / "build"; + const fs::u8path info_dir = build_dir / "info"; + fs::create_directories(info_dir); + { + std::ofstream index_file((info_dir / "index.json").std_path()); + index_file << R"({"name":"cuda-nvvp","version":"12.9.79","build":"he0c23c2_0"})"; + } + { + std::ofstream paths_file((info_dir / "paths.json").std_path()); + paths_file << R"({"paths": [], "paths_version": 1})"; + } + create_archive( + build_dir, + flat_tarball, + compression_algorithm::bzip2, + /* compression_level= */ 1, + /* compression_threads= */ 1, + /* filter= */ nullptr + ); + REQUIRE(fs::exists(flat_tarball)); + + // Simulate a stale partial extraction at the hierarchical location. + fs::create_directories(hierarchical_extract / "info"); + { + auto out = open_ofstream(hierarchical_extract / "info" / "repodata_record.json"); + out << R"({"name":"cuda-nvvp","version":"12.9.79","build":"he0c23c2_0","url":")" + << pkg.package_url << R"(","channel":")" << pkg.channel << R"("})"; + } + { + std::ofstream paths_file((hierarchical_extract / "info" / "paths.json").std_path()); + paths_file << R"({ + "paths_version": 1, + "paths": [ + {"_path": "Library/missing-file.txt", "path_type": "hardlink", "size_in_bytes": 42} + ] +})"; + } + + PackageFetcher fetcher(pkg, package_caches); + REQUIRE(fetcher.needs_extract()); + + fetcher.clear_cache(); + + REQUIRE_FALSE(fs::exists(hierarchical_extract)); + REQUIRE_FALSE(fs::exists(flat_tarball)); + REQUIRE(package_caches.get_extracted_dir_path(pkg).empty()); + } } From 59ca9f6333aa68d819495abae38a49537f6b1f10 Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Tue, 23 Jun 2026 13:35:46 +0200 Subject: [PATCH 5/7] Simply use raw pointer Signed-off-by: Julien Jerphanion Co-authored-by: Johan Mabille --- libmamba/include/mamba/core/package_fetcher.hpp | 3 +-- libmamba/src/core/package_fetcher.cpp | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libmamba/include/mamba/core/package_fetcher.hpp b/libmamba/include/mamba/core/package_fetcher.hpp index 3b6b293336..3d1578ada3 100644 --- a/libmamba/include/mamba/core/package_fetcher.hpp +++ b/libmamba/include/mamba/core/package_fetcher.hpp @@ -8,7 +8,6 @@ #define MAMBA_CORE_PACKAGE_FETCHER_HPP #include -#include #include "mamba/core/package_cache.hpp" #include "mamba/core/package_handling.hpp" @@ -117,7 +116,7 @@ namespace mamba void update_monitor(progress_callback_t* cb, PackageExtractEvent event) const; specs::PackageInfo m_package_info; - std::shared_ptr m_caches; + MultiPackageCache* m_caches = nullptr; fs::u8path m_tarball_path; fs::u8path m_cache_path; diff --git a/libmamba/src/core/package_fetcher.cpp b/libmamba/src/core/package_fetcher.cpp index 85db50b625..0d492bb11d 100644 --- a/libmamba/src/core/package_fetcher.cpp +++ b/libmamba/src/core/package_fetcher.cpp @@ -140,7 +140,7 @@ namespace mamba PackageFetcher::PackageFetcher(const specs::PackageInfo& pkg_info, MultiPackageCache& caches) : m_package_info(pkg_info) - , m_caches(&caches, [](MultiPackageCache*) {}) + , m_caches(&caches) { const fs::u8path extracted_cache = m_caches->get_extracted_dir_path(m_package_info); if (extracted_cache.empty()) From d3ed4bbb99392c94eef6069fffdd36f22120b8fc Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Tue, 4 Aug 2026 15:56:35 +0200 Subject: [PATCH 6/7] Trigger CI Signed-off-by: Julien Jerphanion From ff808f1ca48ade287a1cdc435b7fa472bd64895b Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Wed, 5 Aug 2026 11:18:48 +0200 Subject: [PATCH 7/7] Use the existing `ScopedContextChange` Signed-off-by: Julien Jerphanion Co-authored-by: Hind Montassif --- libmamba/tests/src/core/test_transaction.cpp | 44 ++++---------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/libmamba/tests/src/core/test_transaction.cpp b/libmamba/tests/src/core/test_transaction.cpp index 132484ac38..9f1a13791c 100644 --- a/libmamba/tests/src/core/test_transaction.cpp +++ b/libmamba/tests/src/core/test_transaction.cpp @@ -33,40 +33,6 @@ namespace mamba { namespace { - struct ScopedContextOverride - { - ScopedContextOverride(Context& ctx, const fs::u8path& prefix, const fs::u8path& pkgs_dir) - { - m_ctx = &ctx; - m_saved_prefix_params = ctx.prefix_params; - m_saved_pkgs_dirs = ctx.pkgs_dirs; - m_saved_prefix_data_interoperability = ctx.prefix_data_interoperability; - m_saved_extract_threads = ctx.threads_params.extract_threads; - - ctx.prefix_params.target_prefix = prefix; - ctx.prefix_params.root_prefix = prefix; - ctx.prefix_params.conda_prefix = prefix; - ctx.prefix_params.relocate_prefix = prefix; - ctx.pkgs_dirs = { pkgs_dir }; - ctx.prefix_data_interoperability = false; - ctx.threads_params.extract_threads = 1; - } - - ~ScopedContextOverride() - { - m_ctx->prefix_params = m_saved_prefix_params; - m_ctx->pkgs_dirs = m_saved_pkgs_dirs; - m_ctx->prefix_data_interoperability = m_saved_prefix_data_interoperability; - m_ctx->threads_params.extract_threads = m_saved_extract_threads; - } - - Context* m_ctx = nullptr; - PrefixParams m_saved_prefix_params; - std::vector m_saved_pkgs_dirs; - bool m_saved_prefix_data_interoperability = false; - int m_saved_extract_threads = 0; - }; - void write_text_file(const fs::u8path& path, std::string_view content) { fs::create_directories(path.parent_path()); @@ -345,7 +311,15 @@ namespace mamba const fs::u8path prefix = temp_dir.path() / "prefix"; const fs::u8path pkgs_dir = temp_dir.path() / "pkgs"; fs::create_directories(pkgs_dir); - ScopedContextOverride context_guard(ctx, prefix, pkgs_dir); + mambatests::ScopedContextChange context_change{ ctx }; + context_change.set_target_prefix(prefix) + .set_root_prefix(prefix) + .set_pkgs_dirs({ pkgs_dir }) + .set_prefix_data_interoperability(false) + .preserve(ctx.threads_params); + ctx.prefix_params.conda_prefix = prefix; + ctx.prefix_params.relocate_prefix = prefix; + ctx.threads_params.extract_threads = 1; auto pkg_a = make_test_package("pkg-a"); auto pkg_b = make_test_package("pkg-b");