Remove mirror from default package cache directory cache key again

We added this initially to deal with pacman not having the mirror
in its cache key of repository metadata. The downside of this approach
is that we cannot cache packages across different mirrors. As an
aternative, let's simply not cache repository metadata for pacman in
the package cache directory.

While we're at it, remove the hack we did for zypper to ensure it had
the mirror in its cache key and also don't store its repository metadata
in the package cache directory. The reasoning here is while we can make
sure our own generated repository ids have the hashed mirror in them, we
cannot do so for any repositories added by users, which might end up causing
conflicts.

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index 5334e484c6c39599716eb1ae7fe493f7dd6b02f9..2f67bdd4be9a85b83dd6e714c1e09dec79d16ad0 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -88,6 +88,8 @@ from mkosi.context import Context
 from mkosi.distributions import Distribution, detect_distribution
 from mkosi.documentation import show_docs
 from mkosi.installer import clean_package_manager_metadata
+from mkosi.installer.pacman import Pacman
+from mkosi.installer.zypper import Zypper
 from mkosi.kmod import gen_required_kernel_modules, is_valid_kdir, loaded_modules, process_kernel_modules
 from mkosi.log import ARG_DEBUG, complete_step, die, log_notice, log_step
 from mkosi.manifest import Manifest
@@ -4867,8 +4869,23 @@ def sync_repository_metadata(
 
     subdir = last.distribution.package_manager(last).subdir(last)
 
+    # Pacman and Zypper are unable to cache repository metadata for multiple mirrors at the same time in the
+    # same cache directory as they both only use the repository id as the cache key which will trivially
+    # conflict across multiple mirrors. Apt and Fedora put the mirror in the cache key for the repository
+    # metadata in some form which avoids conflicts across different mirrors.
+    shared_repository_metadata_cache = last.distribution.package_manager(last) not in (Pacman, Zypper)
+
+    if shared_repository_metadata_cache:
+        dst = last.package_cache_dir_or_default()
+    elif last.is_incremental() and last.cacheonly != Cacheonly.never:
+        if not metadata_cache(last).exists():
+            make_tree(metadata_cache(last), use_subvolumes=last.use_subvolumes, sandbox=last.sandbox)
+        dst = metadata_cache(last)
+    else:
+        dst = metadata_dir
+
     for d in ("cache", "lib"):
-        (last.package_cache_dir_or_default() / d / subdir).mkdir(parents=True, exist_ok=True)
+        (dst / d / subdir).mkdir(parents=True, exist_ok=True)
 
     # Sync repository metadata unless explicitly disabled.
     if last.cacheonly not in (Cacheonly.always, Cacheonly.metadata):
@@ -4879,7 +4896,7 @@ def sync_repository_metadata(
                 workspace=workspace,
                 resources=resources,
                 keyring_dir=keyring_dir,
-                metadata_dir=last.package_cache_dir_or_default(),
+                metadata_dir=dst,
             )
             context.root.mkdir(mode=0o755)
 
@@ -4901,20 +4918,26 @@ def sync_repository_metadata(
     # If we're in incremental mode and caching metadata is not explicitly disabled, cache the keyring and the
     # synced repository metadata so we can reuse them later.
     if last.is_incremental() and last.cacheonly != Cacheonly.never:
-        rmtree(keyring_cache(last), metadata_cache(last), sandbox=last.sandbox)
-
-        for p in (keyring_cache(last), metadata_cache(last)):
-            make_tree(p, use_subvolumes=last.use_subvolumes, sandbox=last.sandbox)
+        rmtree(
+            keyring_cache(last),
+            *([metadata_cache(last)] if shared_repository_metadata_cache else []),
+            sandbox=last.sandbox,
+        )
 
+        make_tree(keyring_cache(last), use_subvolumes=last.use_subvolumes, sandbox=last.sandbox)
         copy_tree(keyring_dir, keyring_cache(last), use_subvolumes=last.use_subvolumes, sandbox=last.sandbox)
-        copy_repository_metadata(last, metadata_cache(last))
+
+        if shared_repository_metadata_cache:
+            make_tree(metadata_cache(last), use_subvolumes=last.use_subvolumes, sandbox=last.sandbox)
+            copy_repository_metadata(last, metadata_cache(last))
+
         copy_tree(
             metadata_cache(last),
             metadata_dir,
             use_subvolumes=last.use_subvolumes,
             sandbox=last.sandbox,
         )
-    else:
+    elif shared_repository_metadata_cache:
         copy_repository_metadata(last, metadata_dir)
 
 

diff --git a/mkosi/config.py b/mkosi/config.py
index 4e0465a8cb9375cb15217a4246a129d67cb4e7a2..8f14d78e2962d4272d2241604772001e6b523633 100644 (file)
--- a/mkosi/config.py
+++ b/mkosi/config.py
@@ -2195,8 +2195,6 @@ class Config:
 
     def package_cache_dir_or_default(self) -> Path:
         key = f"{self.distribution}~{self.release}~{self.architecture}"
-        if self.mirror:
-            key += f"-{self.mirror.replace('/', '-')}"
         return self.package_cache_dir or (INVOKING_USER.cache_dir() / "mkosi" / key)
 
     def tools(self) -> Path:

diff --git a/mkosi/installer/zypper.py b/mkosi/installer/zypper.py
index 78704815b8c442f685104771f9394ea23d96acdf..d5df6e9c66d57d5b821e988f559b5c39063c0d8f 100644 (file)
--- a/mkosi/installer/zypper.py
+++ b/mkosi/installer/zypper.py
@@ -67,15 +67,10 @@ class Zypper(PackageManager):
             repofile.parent.mkdir(exist_ok=True, parents=True)
             with repofile.open("w") as f:
                 for repo in repositories:
-                    # zypper uses the repo ID as its cache key which is unsafe so add a hash of the url used
-                    # to it to make sure a unique cache is used for each repository. We use roughly the same
-                    # algorithm here that dnf uses as well.
-                    key = hashlib.sha256(repo.url.encode()).hexdigest()[:16]
-
                     f.write(
                         textwrap.dedent(
                             f"""\
-                            [{repo.id}-{key}]
+                            [{repo.id}]
                             name={repo.id}
                             {repo.url}
                             gpgcheck=1