Skip to content

Commit 7e5cb53

Browse files
committed
fix: Purge invalid hierarchical package cache before re-extract
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
1 parent 1485058 commit 7e5cb53

6 files changed

Lines changed: 155 additions & 2 deletions

File tree

libmamba/include/mamba/core/package_cache.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ namespace mamba
108108
Writable is_writable();
109109
fs::u8path path() const;
110110
void clear_query_cache(const specs::PackageInfo& s);
111+
void remove_extracted_package(const specs::PackageInfo& s);
111112

112113
bool has_valid_tarball(const specs::PackageInfo& s, const ValidationParams& params);
113114
bool has_valid_extracted_dir(const specs::PackageInfo& s, const ValidationParams& params);
@@ -138,6 +139,7 @@ namespace mamba
138139
std::vector<PackageCacheData*> writable_caches();
139140

140141
void clear_query_cache(const specs::PackageInfo& s);
142+
void remove_extracted_package(const specs::PackageInfo& s);
141143

142144
private:
143145

libmamba/src/core/package_cache.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,15 @@ namespace mamba
148148
m_valid_extracted_dir.erase(s.long_str());
149149
}
150150

151+
void PackageCacheData::remove_extracted_package(const specs::PackageInfo& s)
152+
{
153+
const auto folder = package_cache_folder_relative_path(s);
154+
const fs::u8path pkg_name = specs::strip_archive_extension(s.filename);
155+
fs::remove_all(m_path / folder / pkg_name);
156+
fs::remove_all(m_path / pkg_name);
157+
clear_query_cache(s);
158+
}
159+
151160
void PackageCacheData::check_writable()
152161
{
153162
fs::u8path magic_file = m_path / PACKAGE_CACHE_MAGIC_FILE;
@@ -664,4 +673,15 @@ namespace mamba
664673
c.clear_query_cache(s);
665674
}
666675
}
676+
677+
void MultiPackageCache::remove_extracted_package(const specs::PackageInfo& s)
678+
{
679+
for (auto& c : m_caches)
680+
{
681+
c.remove_extracted_package(s);
682+
}
683+
const std::string pkg = s.long_str();
684+
m_cached_tarballs.erase(pkg);
685+
m_cached_extracted_dirs.erase(pkg);
686+
}
667687
} // namespace mamba

libmamba/src/core/package_fetcher.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,9 +388,23 @@ namespace mamba
388388

389389
void PackageFetcher::clear_cache() const
390390
{
391+
const auto remove_extracted_at = [&](const fs::u8path& cache_path)
392+
{ fs::remove_all(get_extract_path(filename(), cache_path)); };
393+
391394
fs::remove_all(m_tarball_path);
392-
const fs::u8path dest_dir = specs::strip_archive_extension(m_tarball_path.string());
393-
fs::remove_all(dest_dir);
395+
remove_extracted_at(m_cache_path);
396+
397+
// Tarballs may use the flat layout while extraction uses the hierarchical layout.
398+
const fs::u8path tarball_parent = m_tarball_path.parent_path();
399+
if (tarball_parent != m_cache_path)
400+
{
401+
remove_extracted_at(tarball_parent);
402+
}
403+
404+
if (m_caches)
405+
{
406+
m_caches->clear_query_cache(m_package_info);
407+
}
394408
}
395409

396410
/*******************

libmamba/src/core/transaction.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ namespace mamba
8383
return path;
8484
}
8585

86+
// Drop invalid or partial extracted directories so a cached tarball can be
87+
// re-extracted (e.g. after a failed extraction left stale files behind).
88+
caches.remove_extracted_package(pkg);
89+
8690
PackageFetcher fetcher(pkg, caches);
8791
if (fetcher.needs_download())
8892
{

libmamba/tests/src/core/test_package_cache.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,5 +199,42 @@ namespace mamba
199199
MultiPackageCache cache({ pkgs_dir }, warn_params);
200200
REQUIRE(cache.get_extracted_dir_path(pkg_info).empty());
201201
}
202+
203+
// Non-regression for https://github.com/mamba-org/mamba/issues/4331#issuecomment-4771693736
204+
SECTION("remove_extracted_package clears invalid hierarchical cache #4331")
205+
{
206+
auto warn_params = ctx.validation_params;
207+
warn_params.safety_checks = VerificationLevel::Warn;
208+
209+
const fs::u8path invalid_hierarchical_dir = pkgs_dir / rel_path
210+
/ "test-pkg-1.0.0-h123456_0";
211+
fs::create_directories(invalid_hierarchical_dir / "info");
212+
write_repodata_record(invalid_hierarchical_dir / "info" / "repodata_record.json", pkg_info);
213+
214+
nlohmann::json paths_json;
215+
paths_json["paths_version"] = 1;
216+
paths_json["paths"] = nlohmann::json::array(
217+
{
218+
nlohmann::json{
219+
{ "_path", "etc/conda/test-files/missing-file.txt" },
220+
{ "path_type", "hardlink" },
221+
{ "size_in_bytes", 42 },
222+
},
223+
}
224+
);
225+
{
226+
std::ofstream paths_out((invalid_hierarchical_dir / "info" / "paths.json").std_path());
227+
paths_out << paths_json.dump();
228+
}
229+
230+
MultiPackageCache cache({ pkgs_dir }, warn_params);
231+
REQUIRE(cache.get_extracted_dir_path(pkg_info).empty());
232+
REQUIRE(fs::exists(invalid_hierarchical_dir));
233+
234+
cache.remove_extracted_package(pkg_info);
235+
236+
REQUIRE_FALSE(fs::exists(invalid_hierarchical_dir));
237+
REQUIRE(cache.get_extracted_dir_path(pkg_info).empty());
238+
}
202239
}
203240
} // namespace mamba

libmamba/tests/src/core/test_package_fetcher.cpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "mamba/core/package_handling.hpp"
1616
#include "mamba/core/util.hpp"
1717
#include "mamba/fs/filesystem.hpp"
18+
#include "mamba/specs/archive.hpp"
1819
#include "mamba/util/url_manip.hpp"
1920

2021
#include "mambatests.hpp"
@@ -1300,4 +1301,79 @@ namespace
13001301
REQUIRE(repodata_record.contains("size"));
13011302
CHECK(repodata_record["size"] == tarball_size);
13021303
}
1304+
1305+
// Non-regression for https://github.com/mamba-org/mamba/issues/4322 and
1306+
// https://github.com/mamba-org/mamba/issues/4331#issuecomment-4771693736
1307+
// When the tarball uses the flat layout but extraction targets the hierarchical layout,
1308+
// clear_cache() must remove the hierarchical extracted directory.
1309+
TEST_CASE("PackageFetcher::clear_cache removes hierarchical extracted directory", "[regression][4322][4331]")
1310+
{
1311+
auto& ctx = mambatests::context();
1312+
TemporaryDirectory temp_dir;
1313+
const fs::u8path pkgs_dir = temp_dir.path() / "pkgs";
1314+
fs::create_directories(pkgs_dir);
1315+
MultiPackageCache package_caches({ pkgs_dir }, ctx.validation_params);
1316+
1317+
specs::PackageInfo pkg;
1318+
pkg.name = "cuda-nvvp";
1319+
pkg.version = "12.9.79";
1320+
pkg.build_string = "he0c23c2_0";
1321+
pkg.platform = "win-64";
1322+
pkg.channel = "conda-forge";
1323+
pkg.package_url = "https://conda.anaconda.org/conda-forge/win-64/cuda-nvvp-12.9.79-he0c23c2_0.conda";
1324+
pkg.filename = "cuda-nvvp-12.9.79-he0c23c2_0.conda";
1325+
1326+
const auto cache_subdir = package_cache_folder_relative_path(pkg);
1327+
const std::string pkg_basename = std::string(specs::strip_archive_extension(pkg.filename));
1328+
const fs::u8path hierarchical_extract = pkgs_dir / cache_subdir / pkg_basename;
1329+
const fs::u8path flat_tarball = pkgs_dir / pkg.filename;
1330+
1331+
// Build a valid tarball at the flat cache location.
1332+
const fs::u8path build_dir = temp_dir.path() / "build";
1333+
const fs::u8path info_dir = build_dir / "info";
1334+
fs::create_directories(info_dir);
1335+
{
1336+
std::ofstream index_file((info_dir / "index.json").std_path());
1337+
index_file << R"({"name":"cuda-nvvp","version":"12.9.79","build":"he0c23c2_0"})";
1338+
}
1339+
{
1340+
std::ofstream paths_file((info_dir / "paths.json").std_path());
1341+
paths_file << R"({"paths": [], "paths_version": 1})";
1342+
}
1343+
create_archive(
1344+
build_dir,
1345+
flat_tarball,
1346+
compression_algorithm::bzip2,
1347+
/* compression_level= */ 1,
1348+
/* compression_threads= */ 1,
1349+
/* filter= */ nullptr
1350+
);
1351+
REQUIRE(fs::exists(flat_tarball));
1352+
1353+
// Simulate a stale partial extraction at the hierarchical location.
1354+
fs::create_directories(hierarchical_extract / "info");
1355+
{
1356+
auto out = open_ofstream(hierarchical_extract / "info" / "repodata_record.json");
1357+
out << R"({"name":"cuda-nvvp","version":"12.9.79","build":"he0c23c2_0","url":")"
1358+
<< pkg.package_url << R"(","channel":")" << pkg.channel << R"("})";
1359+
}
1360+
{
1361+
std::ofstream paths_file((hierarchical_extract / "info" / "paths.json").std_path());
1362+
paths_file << R"({
1363+
"paths_version": 1,
1364+
"paths": [
1365+
{"_path": "Library/missing-file.txt", "path_type": "hardlink", "size_in_bytes": 42}
1366+
]
1367+
})";
1368+
}
1369+
1370+
PackageFetcher fetcher(pkg, package_caches);
1371+
REQUIRE(fetcher.needs_extract());
1372+
1373+
fetcher.clear_cache();
1374+
1375+
REQUIRE_FALSE(fs::exists(hierarchical_extract));
1376+
REQUIRE_FALSE(fs::exists(flat_tarball));
1377+
REQUIRE(package_caches.get_extracted_dir_path(pkg).empty());
1378+
}
13031379
}

0 commit comments

Comments
 (0)