From 2dc54e9703834ffac652580cb387c539e7578e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Kemetm=C3=BCller?= Date: Fri, 10 Nov 2023 10:43:27 +0100 Subject: [PATCH 1/5] Support multi-file version constraints Until https://github.com/conda/conda/pull/11612 is resolved we do a simplified version of combining build strings. This is a change in behavior as previously package version constraings would simply be overwritten instead of combined. --- conda_lock/models/lock_spec.py | 122 +++++++++++++++++++++++++++ conda_lock/src_parser/aggregation.py | 6 +- tests/test_conda_lock.py | 117 +++++++++++++++++++++++-- 3 files changed, 236 insertions(+), 9 deletions(-) diff --git a/conda_lock/models/lock_spec.py b/conda_lock/models/lock_spec.py index 6448800b..02f501f1 100644 --- a/conda_lock/models/lock_spec.py +++ b/conda_lock/models/lock_spec.py @@ -1,8 +1,12 @@ +from __future__ import annotations + +import copy import hashlib import json import pathlib import typing +from fnmatch import fnmatchcase from typing import Dict, List, Optional, Union from pydantic import BaseModel, Field, validator @@ -24,23 +28,141 @@ class _BaseDependency(StrictModel): def sorted_extras(cls, v: List[str]) -> List[str]: return sorted(v) + def _merge_base(self, other: _BaseDependency) -> _BaseDependency: + if other is None: + return self + if ( + self.name != other.name + or self.manager != other.manager + or self.category != other.category + ): + raise ValueError( + "Cannot merge incompatible dependencies: {self} != {other}" + ) + return _BaseDependency( + name=self.name, + manager=self.manager, + category=self.category, + extras=list(set(self.extras + other.extras)), + ) + class VersionedDependency(_BaseDependency): version: str build: Optional[str] = None conda_channel: Optional[str] = None + @staticmethod + def _merge_matchspecs( + matchspec1: Optional[str], + matchspec2: Optional[str], + combine_constraints: bool = True, + ) -> Optional[str]: + if matchspec1 == matchspec2: + return matchspec1 + if matchspec1 is None or matchspec1 == "": + return matchspec2 + if matchspec2 is None or matchspec2 == "": + return matchspec1 + if fnmatchcase(matchspec1, matchspec2): + return matchspec1 + if fnmatchcase(matchspec2, matchspec1): + return matchspec2 + if not combine_constraints: + raise ValueError( + f"Found incompatible constraint {matchspec1}, {matchspec2}" + ) + return f"{matchspec1},{matchspec2}" + + def merge(self, other: Optional[VersionedDependency]) -> VersionedDependency: + if other is None: + return self + + if ( + self.conda_channel is not None + and other.conda_channel is not None + and self.conda_channel != other.conda_channel + ): + raise ValueError( + f"VersionedDependency has two different conda_channels:\n{self}\n{other}" + ) + merged_base = self._merge_base(other) + try: + build = self._merge_matchspecs( + self.build, other.build, combine_constraints=False + ) + except ValueError as exc: + raise ValueError( + f"Unsupported usage of two incompatible builds for same dependency {self}, {other}" + ) from exc + + return VersionedDependency( + name=merged_base.name, + manager=merged_base.manager, + category=merged_base.category, + extras=merged_base.extras, + version=self._merge_matchspecs(self.version, other.version), # type: ignore + build=build, + conda_channel=self.conda_channel or other.conda_channel, + ) + class URLDependency(_BaseDependency): url: str hashes: List[str] + def merge(self, other: Optional[URLDependency]) -> URLDependency: + if other is None: + return self + if self.url != other.url: + raise ValueError(f"URLDependency has two different urls:\n{self}\n{other}") + + if self.hashes != other.hashes: + raise ValueError( + f"URLDependency has two different hashess:\n{self}\n{other}" + ) + merged_base = self._merge_base(other) + + return URLDependency( + name=merged_base.name, + manager=merged_base.manager, + category=merged_base.category, + extras=merged_base.extras, + url=self.url, + hashes=self.hashes, + ) + class VCSDependency(_BaseDependency): source: str vcs: str rev: Optional[str] = None + def merge(self, other: Optional[VCSDependency]) -> VCSDependency: + if other is None: + return self + if self.source != other.source: + raise ValueError( + f"VCSDependency has two different sources:\n{self}\n{other}" + ) + + if self.vcs != other.vcs: + raise ValueError(f"VCSDependency has two different vcss:\n{self}\n{other}") + + if self.rev is not None and other.rev is not None and self.rev != other.rev: + raise ValueError(f"VCSDependency has two different revs:\n{self}\n{other}") + merged_base = self._merge_base(other) + + return VCSDependency( + name=merged_base.name, + manager=merged_base.manager, + category=merged_base.category, + extras=merged_base.extras, + source=self.source, + vcs=self.vcs, + rev=self.rev or other.rev, + ) + Dependency = Union[VersionedDependency, URLDependency, VCSDependency] diff --git a/conda_lock/src_parser/aggregation.py b/conda_lock/src_parser/aggregation.py index d2b5349b..51a033dd 100644 --- a/conda_lock/src_parser/aggregation.py +++ b/conda_lock/src_parser/aggregation.py @@ -34,7 +34,11 @@ def aggregate_lock_specs( lock_spec.dependencies.get(platform, []) for lock_spec in lock_specs ): key = (dep.manager, dep.name) - unique_deps[key] = dep + if unique_deps.get(key) is not None and type(unique_deps[key]) != type(dep): + raise ValueError( + f"Unsupported use of different dependency types for same package:\n{dep}\n{unique_deps[key]}" + ) + unique_deps[key] = dep.merge(unique_deps.get(key)) # type: ignore dependencies[platform] = list(unique_deps.values()) diff --git a/tests/test_conda_lock.py b/tests/test_conda_lock.py index 3381dbc6..976dcc6c 100644 --- a/tests/test_conda_lock.py +++ b/tests/test_conda_lock.py @@ -1625,22 +1625,123 @@ def test_aggregate_lock_specs(): assert actual.content_hash() == expected.content_hash() -def test_aggregate_lock_specs_override_version(): - base_spec = LockSpecification( - dependencies={"linux-64": [_make_spec("package", "=1.0")]}, +def test_aggregate_lock_specs_combine_version(): + first_spec = LockSpecification( + dependencies={"linux-64": [_make_spec("package", ">1.0")]}, + channels=[Channel.from_string("conda-forge")], + sources=[Path("base.yml")], + ) + + second_spec = LockSpecification( + dependencies={"linux-64": [_make_spec("package", "<2.0")]}, + channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], + sources=[Path("additional.yml")], + ) + + result_spec = LockSpecification( + dependencies={"linux-64": [_make_spec("package", "<2.0,>1.0")]}, + channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], + sources=[Path("result.yml")], + ) + + agg_spec = aggregate_lock_specs([first_spec, second_spec], platforms=["linux-64"]) + + assert agg_spec.dependencies == result_spec.dependencies + + +def test_aggregate_lock_specs_combine_build(): + first_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency(name="openblas", version="*", build="openmp*"), + VersionedDependency( + name="_openmp_mutex", version="4.5", build="*_llvm" + ), + ] + }, channels=[Channel.from_string("conda-forge")], sources=[Path("base.yml")], ) - override_spec = LockSpecification( - dependencies={"linux-64": [_make_spec("package", "=2.0")]}, + second_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency( + name="openblas", version="0.3.20", build="openmp_h53a8fd6_1" + ), + VersionedDependency( + name="_openmp_mutex", version="4.5", build="2_kmp_llvm" + ), + ] + }, + channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], + sources=[Path("second.yml")], + ) + + third_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency( + name="openblas", version="*", build="openmp_h53a8fd6_1" + ), + VersionedDependency( + name="_openmp_mutex", version="4.5", build="*_kmp_llvm" + ), + ] + }, + channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], + sources=[Path("third.yml")], + ) + + result_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency( + name="openblas", version="0.3.20", build="openmp_h53a8fd6_1" + ), + VersionedDependency( + name="_openmp_mutex", version="4.5", build="2_kmp_llvm" + ), + ] + }, channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], - sources=[Path("override.yml")], + sources=[Path("result.yml")], + ) + + agg_spec = aggregate_lock_specs( + [first_spec, second_spec, third_spec], platforms=["linux-64"] + ) + + assert agg_spec.dependencies == result_spec.dependencies + + +def test_aggregate_lock_specs_combine_build_incompatible(): + first_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency( + name="openblas", version="0.3.20", build="openmp_h53a8fd6_2" + ), + ] + }, + channels=[Channel.from_string("conda-forge")], + sources=[Path("base.yml")], ) - agg_spec = aggregate_lock_specs([base_spec, override_spec], platforms=["linux-64"]) + second_spec = LockSpecification( + dependencies={ + "linux-64": [ + VersionedDependency( + name="openblas", version="0.3.20", build="openmp_h53a8fd6_1" + ), + ] + }, + channels=[Channel.from_string("internal"), Channel.from_string("conda-forge")], + sources=[Path("second.yml")], + ) - assert agg_spec.dependencies == override_spec.dependencies + with pytest.raises(ValueError): + aggregate_lock_specs([first_spec, second_spec], platforms=["linux-64"]) def test_aggregate_lock_specs_invalid_channels(): From d6c487364b11b2632bdfd2303015415d9f339914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Kemetm=C3=BCller?= Date: Sat, 25 Nov 2023 14:36:44 +0000 Subject: [PATCH 2/5] Split up _merge_matchspecs --- conda_lock/models/lock_spec.py | 52 +++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/conda_lock/models/lock_spec.py b/conda_lock/models/lock_spec.py index 02f501f1..5406df5e 100644 --- a/conda_lock/models/lock_spec.py +++ b/conda_lock/models/lock_spec.py @@ -53,26 +53,34 @@ class VersionedDependency(_BaseDependency): conda_channel: Optional[str] = None @staticmethod - def _merge_matchspecs( - matchspec1: Optional[str], - matchspec2: Optional[str], - combine_constraints: bool = True, + def _merge_versions( + version1: str, + version2: str, + ) -> str: + if version1 == version2: + return version1 + if version1 == "" or version1 == "*": + return version2 + if version2 == "" or version2 == "*": + return version1 + return f"{version1},{version2}" + + @staticmethod + def _merge_builds( + build1: Optional[str], + build2: Optional[str], ) -> Optional[str]: - if matchspec1 == matchspec2: - return matchspec1 - if matchspec1 is None or matchspec1 == "": - return matchspec2 - if matchspec2 is None or matchspec2 == "": - return matchspec1 - if fnmatchcase(matchspec1, matchspec2): - return matchspec1 - if fnmatchcase(matchspec2, matchspec1): - return matchspec2 - if not combine_constraints: - raise ValueError( - f"Found incompatible constraint {matchspec1}, {matchspec2}" - ) - return f"{matchspec1},{matchspec2}" + if build1 == build2: + return build1 + if build1 is None or build1 == "": + return build2 + if build2 is None or build2 == "": + return build1 + if fnmatchcase(build1, build2): + return build1 + if fnmatchcase(build2, build1): + return build2 + raise ValueError(f"Found incompatible constraint {build1}, {build2}") def merge(self, other: Optional[VersionedDependency]) -> VersionedDependency: if other is None: @@ -88,9 +96,7 @@ def merge(self, other: Optional[VersionedDependency]) -> VersionedDependency: ) merged_base = self._merge_base(other) try: - build = self._merge_matchspecs( - self.build, other.build, combine_constraints=False - ) + build = self._merge_builds(self.build, other.build) except ValueError as exc: raise ValueError( f"Unsupported usage of two incompatible builds for same dependency {self}, {other}" @@ -101,7 +107,7 @@ def merge(self, other: Optional[VersionedDependency]) -> VersionedDependency: manager=merged_base.manager, category=merged_base.category, extras=merged_base.extras, - version=self._merge_matchspecs(self.version, other.version), # type: ignore + version=self._merge_versions(self.version, other.version), build=build, conda_channel=self.conda_channel or other.conda_channel, ) From 19436cb7310134cd46c473011aacb0a3ee4259b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Kemetm=C3=BCller?= Date: Sat, 25 Nov 2023 14:51:52 +0000 Subject: [PATCH 3/5] Add tests for _merge_versions/_merge_builds --- tests/test_conda_lock.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_conda_lock.py b/tests/test_conda_lock.py index 976dcc6c..bd6ba69a 100644 --- a/tests/test_conda_lock.py +++ b/tests/test_conda_lock.py @@ -1715,6 +1715,33 @@ def test_aggregate_lock_specs_combine_build(): assert agg_spec.dependencies == result_spec.dependencies +def test_merging_matchspec_parts(): + assert VersionedDependency._merge_versions("4.5", "*") == "4.5" + assert VersionedDependency._merge_versions("*", "4.5") == "4.5" + assert VersionedDependency._merge_versions("", "4.5") == "4.5" + assert VersionedDependency._merge_versions("4.5", "") == "4.5" + assert VersionedDependency._merge_versions("", "") == "" + assert VersionedDependency._merge_versions("*", "*") == "*" + assert VersionedDependency._merge_versions("*", "") == "" + assert VersionedDependency._merge_versions("", "*") == "*" + + assert VersionedDependency._merge_builds("", "") == "" + assert VersionedDependency._merge_builds("*", "*") == "*" + assert VersionedDependency._merge_builds(None, None) is None + + assert VersionedDependency._merge_builds("", "2_kmp_llvm") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("*", "2_kmp_llvm") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("*_llvm", "2_kmp_llvm") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("*_llvm", "*_kmp_llvm") == "*_kmp_llvm" + assert VersionedDependency._merge_builds(None, "2_kmp_llvm") == "2_kmp_llvm" + + assert VersionedDependency._merge_builds("2_kmp_llvm", "") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("2_kmp_llvm", "*") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("2_kmp_llvm", "*_llvm") == "2_kmp_llvm" + assert VersionedDependency._merge_builds("*_kmp_llvm", "*_llvm") == "*_kmp_llvm" + assert VersionedDependency._merge_builds("2_kmp_llvm", None) == "2_kmp_llvm" + + def test_aggregate_lock_specs_combine_build_incompatible(): first_spec = LockSpecification( dependencies={ From 1eafa23a9d1e4683a57a6b6c2f5241f7898fc5d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Kemetm=C3=BCller?= Date: Sat, 25 Nov 2023 14:57:35 +0000 Subject: [PATCH 4/5] Simplify aggregation We need to keep the "# type: ignore" because otherwise we get the following error in mypy: - conda_lock/src_parser/aggregation.py:42: error: Argument 1 to "merge" of "VersionedDependency" has incompatible type "VersionedDependency | URLDependency | VCSDependency | None"; expected "VersionedDependency | None" [arg-type] - conda_lock/src_parser/aggregation.py:42: error: Argument 1 to "merge" of "URLDependency" has incompatible type "VersionedDependency | URLDependency | VCSDependency | None"; expected "URLDependency | None" [arg-type] - conda_lock/src_parser/aggregation.py:42: error: Argument 1 to "merge" of "VCSDependency" has incompatible type "VersionedDependency | URLDependency | VCSDependency | None"; expected "VCSDependency | None" [arg-type] --- conda_lock/src_parser/aggregation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conda_lock/src_parser/aggregation.py b/conda_lock/src_parser/aggregation.py index 51a033dd..0bb5b11c 100644 --- a/conda_lock/src_parser/aggregation.py +++ b/conda_lock/src_parser/aggregation.py @@ -34,11 +34,12 @@ def aggregate_lock_specs( lock_spec.dependencies.get(platform, []) for lock_spec in lock_specs ): key = (dep.manager, dep.name) - if unique_deps.get(key) is not None and type(unique_deps[key]) != type(dep): + existing_dep = unique_deps.get(key) + if existing_dep is not None and type(unique_deps[key]) != type(dep): raise ValueError( f"Unsupported use of different dependency types for same package:\n{dep}\n{unique_deps[key]}" ) - unique_deps[key] = dep.merge(unique_deps.get(key)) # type: ignore + unique_deps[key] = dep.merge(existing_dep) # type: ignore dependencies[platform] = list(unique_deps.values()) From 83792713bcd09e91a09811d2e05e0f6f60ef3e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Kemetm=C3=BCller?= Date: Sat, 25 Nov 2023 15:22:27 +0000 Subject: [PATCH 5/5] Don't disallow merging different categories --- conda_lock/models/lock_spec.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/conda_lock/models/lock_spec.py b/conda_lock/models/lock_spec.py index 5406df5e..3dbd7bdd 100644 --- a/conda_lock/models/lock_spec.py +++ b/conda_lock/models/lock_spec.py @@ -31,18 +31,14 @@ def sorted_extras(cls, v: List[str]) -> List[str]: def _merge_base(self, other: _BaseDependency) -> _BaseDependency: if other is None: return self - if ( - self.name != other.name - or self.manager != other.manager - or self.category != other.category - ): + if self.name != other.name or self.manager != other.manager: raise ValueError( "Cannot merge incompatible dependencies: {self} != {other}" ) return _BaseDependency( name=self.name, manager=self.manager, - category=self.category, + category=self.category, # TODO: Merge categories extras=list(set(self.extras + other.extras)), )