yarn v1: Drop offline mirror collision exception for registry packages

closes #788 Signed-off-by: Michal Šoltis <msoltis@redhat.com>
containerbuildsystem · Jan 20, 2025 · af5c616 · af5c616
1 parent b4a413d
commit af5c616
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 41 deletions.
diff --git a/cachi2/core/package_managers/yarn_classic/main.py b/cachi2/core/package_managers/yarn_classic/main.py
@@ -203,20 +203,36 @@ def _verify_corepack_yarn_version(source_dir: RootedPath, env: dict[str, str]) -
 
 def _verify_no_offline_mirror_collisions(packages: Iterable[YarnClassicPackage]) -> None:
     """Verify that there are no duplicate tarballs in the offline mirror."""
-    tarballs = []
+    all_tarballs_counter: Counter[str] = Counter()
+    registry_tarballs_counter: Counter[str] = Counter()
+
     for p in packages:
         if isinstance(p, (RegistryPackage, UrlPackage)):
             tarball_name = get_tarball_mirror_name(p.url)
-            tarballs.append(tarball_name)
+            all_tarballs_counter[tarball_name] += 1
+            if isinstance(p, RegistryPackage):
+                registry_tarballs_counter[tarball_name] += 1
+
         elif isinstance(p, GitPackage):
             tarball_name = get_git_tarball_mirror_name(p.url)
-            tarballs.append(tarball_name)
+            all_tarballs_counter[tarball_name] += 1
         else:
             # file, link, and workspace packages are not copied to the offline mirror
             continue
 
-    c = Counter(tarballs)
-    duplicate_tarballs = [f"{name} ({count}x)" for name, count in c.most_common() if count > 1]
+    # check for duplicates, considering cross-type collisions
+    duplicate_tarballs = []
+    for name, count in all_tarballs_counter.items():
+        if count <= 1:
+            continue
+
+        registry_count = registry_tarballs_counter.get(name, 0)
+        non_registry_count = count - registry_count
+
+        # flag if there is at least 1 non-registry package OR a cross-type collision
+        if non_registry_count >= 1:
+            duplicate_tarballs.append(f"{name} ({count}x)")
+
     if len(duplicate_tarballs) > 0:
         raise PackageManagerError(f"Duplicate tarballs detected: {', '.join(duplicate_tarballs)}")
 

diff --git a/tests/unit/package_managers/yarn_classic/test_main.py b/tests/unit/package_managers/yarn_classic/test_main.py
@@ -22,8 +22,11 @@
 )
 from cachi2.core.package_managers.yarn_classic.project import Project
 from cachi2.core.package_managers.yarn_classic.resolver import (
+    FilePackage,
     GitPackage,
+    LinkPackage,
     RegistryPackage,
+    UrlPackage,
     YarnClassicPackage,
 )
 from cachi2.core.rooted_path import RootedPath
@@ -247,47 +250,83 @@ def test_verify_corepack_yarn_version_invalid_version(
         _verify_corepack_yarn_version(RootedPath(tmp_path), env={"foo": "bar"})
 
 
-def test_verify_offline_mirror_collisions_registry_packages() -> None:
-    packages: Iterable[YarnClassicPackage] = [
-        RegistryPackage(
-            name="foo",
-            version="1.0.0",
-            url="https://registry.yarnpkg.com/same/-/same-1.0.0.tgz",
+@pytest.mark.parametrize(
+    "packages",
+    [
+        pytest.param(
+            [
+                RegistryPackage(
+                    name="foo",
+                    version="1.0.0",
+                    url="https://registry.yarnpkg.com/same/-/same-1.0.0.tgz",
+                ),
+                RegistryPackage(
+                    name="bar",
+                    version="1.0.0",
+                    url="https://registry.yarnpkg.com/same/-/same-1.0.0.tgz",
+                ),
+            ],
+            id="registry_packages",
         ),
-        RegistryPackage(
-            name="bar",
-            version="1.0.0",
-            url="https://registry.yarnpkg.com/same/-/same-1.0.0.tgz",
+        pytest.param(
+            [
+                RegistryPackage(
+                    name="foo",
+                    version="1.0.0",
+                    url="https://registry.yarnpkg.com/@colors/colors/-/colors-1.6.0.tgz",
+                ),
+                RegistryPackage(
+                    name="bar",
+                    version="1.0.0",
+                    url="https://registry.yarnpkg.com/@colors/colors/-/colors-1.6.0.tgz",
+                ),
+            ],
+            id="scoped_registry_packages",
         ),
-    ]
-
-    with pytest.raises(PackageManagerError):
-        _verify_no_offline_mirror_collisions(packages)
+        pytest.param(
+            [
+                LinkPackage(name="foo", version="1.0.0", path=RootedPath("/path/to/foo")),
+                FilePackage(name="bar", version="1.0.0", path=RootedPath("/path/to/bar")),
+            ],
+            id="skipped_packages",
+        ),
+    ],
+)
+def test_verify_offline_mirror_collisions_pass(packages: Iterable[YarnClassicPackage]) -> None:
+    _verify_no_offline_mirror_collisions(packages)
 
 
-def test_verify_offline_mirror_collisions_scoped_registry_packages() -> None:
-    packages: Iterable[YarnClassicPackage] = [
-        RegistryPackage(
-            name="foo",
-            version="1.0.0",
-            url="https://registry.yarnpkg.com/@colors/colors/-/colors-1.6.0.tgz",
+@pytest.mark.parametrize(
+    "packages",
+    [
+        pytest.param(
+            [
+                RegistryPackage(
+                    name="foo",
+                    version="1.0.0",
+                    url="https://registry.yarnpkg.com/same/-/same-1.0.0.tgz",
+                ),
+                UrlPackage(
+                    name="bar",
+                    version="1.0.0",
+                    url="https://mirror.example.com/same-1.0.0.tgz",
+                ),
+            ],
+            id="registry_and_url_package",
         ),
-        RegistryPackage(
-            name="bar",
-            version="1.0.0",
-            url="https://registry.yarnpkg.com/@colors/colors/-/colors-1.6.0.tgz",
+        pytest.param(
+            [
+                GitPackage(
+                    name="foo", version="1.0.0", url="https://github.com/user/repo.git#commit-hash"
+                ),
+                GitPackage(
+                    name="bar", version="1.0.0", url="https://github.com/user/repo.git#commit-hash"
+                ),
+            ],
+            id="git_packages",
         ),
-    ]
-
-    with pytest.raises(PackageManagerError):
-        _verify_no_offline_mirror_collisions(packages)
-
-
-def test_verify_offline_mirror_collisions_git_packages() -> None:
-    packages: Iterable[YarnClassicPackage] = [
-        GitPackage(name="foo", version="1.0.0", url="https://github.com/user/repo.git#commit-hash"),
-        GitPackage(name="bar", version="1.0.0", url="https://github.com/user/repo.git#commit-hash"),
-    ]
-
+    ],
+)
+def test_verify_offline_mirror_collisions_fail(packages: Iterable[YarnClassicPackage]) -> None:
     with pytest.raises(PackageManagerError):
         _verify_no_offline_mirror_collisions(packages)