fix: Clear stale negative cache entries

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<PackageCacheData*> writable_caches();
 
         void clear_query_cache(const specs::PackageInfo& s);
+        void remove_extracted_package(const specs::PackageInfo& s);
 
     private:
 

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;

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;
@@ -656,9 +665,23 @@ 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);
         }
     }
+
+    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

libmamba/src/core/package_fetcher.cpp

@@ -139,12 +139,13 @@ 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);
+        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);
 
@@ -158,7 +159,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
@@ -359,6 +360,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&)
@@ -386,9 +388,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);
+        }
     }
 
     /*******************

libmamba/src/core/transaction.cpp

@@ -27,6 +27,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,62 @@ 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;
+            }
+
+            // 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())
+            {
+                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.");
+        }
+
         // TODO duplicated function, consider moving it to Pool
         auto database_has_package(solver::libsolv::Database& database, const specs::MatchSpec& spec)
             -> bool
@@ -771,7 +828,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
             {

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

libmamba/tests/src/core/test_package_cache.cpp

@@ -153,5 +153,88 @@ namespace mamba
 
             REQUIRE(cache.get_tarball_path(pkg_no_val) == pkgs_dir);
         }
+
+        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);
+        }
+
+        // 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());
+        }
+
+        // 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

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());
+    }
 }