From af5c616bff1dbb00f34c009d12f3398f2205a509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0oltis?= Date: Mon, 20 Jan 2025 13:30:22 +0100 Subject: [PATCH] yarn v1: Drop offline mirror collision exception for registry packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes https://github.com/containerbuildsystem/cachi2/issues/788 Signed-off-by: Michal Šoltis --- .../package_managers/yarn_classic/main.py | 26 +++- .../yarn_classic/test_main.py | 111 ++++++++++++------ 2 files changed, 96 insertions(+), 41 deletions(-) diff --git a/cachi2/core/package_managers/yarn_classic/main.py b/cachi2/core/package_managers/yarn_classic/main.py index 4b6c160f4..d445c01ab 100644 --- 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 index cefdc0799..5b3e7d362 100644 --- 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)