-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Multiple Categories for Sub-Dependencies in Lockfile #390
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
**.DS_Store | ||
*.egg-info | ||
*.eggs | ||
*.pyc | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,17 @@ | |
|
||
from collections import defaultdict | ||
from textwrap import dedent | ||
from typing import Collection, Dict, List, Mapping, Optional, Sequence, Set, Union | ||
from typing import ( | ||
Collection, | ||
DefaultDict, | ||
Dict, | ||
List, | ||
Mapping, | ||
Optional, | ||
Sequence, | ||
Set, | ||
Union, | ||
) | ||
|
||
import yaml | ||
|
||
|
@@ -38,6 +48,23 @@ def _seperator_munge_get( | |
return d[key.replace("_", "-")] | ||
|
||
|
||
def _truncate_main_category( | ||
planned: Mapping[str, Union[List[LockedDependency], LockedDependency]], | ||
) -> None: | ||
""" | ||
Given the package dependencies with their respective categories | ||
for any package that is in the main category, remove all other associated categories | ||
""" | ||
# Packages in the main category are always installed | ||
# so other categories are not necessary | ||
for targets in planned.values(): | ||
if not isinstance(targets, list): | ||
targets = [targets] | ||
for target in targets: | ||
if "main" in target.categories: | ||
target.categories = {"main"} | ||
|
||
|
||
def apply_categories( | ||
requested: Dict[str, Dependency], | ||
planned: Mapping[str, Union[List[LockedDependency], LockedDependency]], | ||
|
@@ -111,27 +138,31 @@ def dep_name(manager: str, dep: str) -> str: | |
|
||
by_category[request.category].append(request.name) | ||
|
||
# now, map each package to its root request preferring the ones earlier in the | ||
# list | ||
# now, map each package to every root request that requires it | ||
categories = [*categories, *(k for k in by_category if k not in categories)] | ||
root_requests = {} | ||
root_requests: DefaultDict[str, List[str]] = defaultdict(list) | ||
for category in categories: | ||
for root in by_category.get(category, []): | ||
for transitive_dep in dependents[root]: | ||
if transitive_dep not in root_requests: | ||
root_requests[transitive_dep] = root | ||
root_requests[transitive_dep].append(root) | ||
# include root requests themselves | ||
for name in requested: | ||
root_requests[name] = name | ||
root_requests[name].append(name) | ||
|
||
for dep, root in root_requests.items(): | ||
source = requested[root] | ||
for dep, roots in root_requests.items(): | ||
# try a conda target first | ||
targets = _seperator_munge_get(planned, dep) | ||
if not isinstance(targets, list): | ||
targets = [targets] | ||
for target in targets: | ||
target.category = source.category | ||
|
||
for root in roots: | ||
source = requested[root] | ||
for target in targets: | ||
target.categories.add(source.category) | ||
|
||
# For any dep that is part of the 'main' category | ||
# we should remove all other categories | ||
_truncate_main_category(planned) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before this PR, if any package has a root / ancestor package that is part of the We want to maintain that same behavior in this PR. Thus, for any package that is in the |
||
|
||
|
||
def parse_conda_lock_file(path: pathlib.Path) -> Lockfile: | ||
|
@@ -163,7 +194,7 @@ def write_conda_lock_file( | |
content.filter_virtual_packages_inplace() | ||
with path.open("w") as f: | ||
if include_help_text: | ||
categories = set(p.category for p in content.package) | ||
categories: Set[str] = set().union(*(p.categories for p in content.package)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Takes the union of all sets of categories of all dependencies to get the final set of categories to store in the lockfile. |
||
|
||
def write_section(text: str) -> None: | ||
lines = dedent(text).split("\n") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
from collections import defaultdict | ||
from typing import ClassVar, Dict, List, Optional | ||
from typing import ClassVar, Dict, List, Optional, Set | ||
|
||
from conda_lock.lockfile.v1.models import ( | ||
BaseLockedDependency, | ||
|
@@ -17,20 +17,25 @@ | |
|
||
|
||
class LockedDependency(BaseLockedDependency): | ||
def to_v1(self) -> LockedDependencyV1: | ||
return LockedDependencyV1( | ||
name=self.name, | ||
version=self.version, | ||
manager=self.manager, | ||
platform=self.platform, | ||
dependencies=self.dependencies, | ||
url=self.url, | ||
hash=self.hash, | ||
category=self.category, | ||
source=self.source, | ||
build=self.build, | ||
optional=self.category != "main", | ||
) | ||
categories: Set[str] = set() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you want There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that in |
||
|
||
def to_v1(self) -> List[LockedDependencyV1]: | ||
return [ | ||
LockedDependencyV1( | ||
name=self.name, | ||
version=self.version, | ||
manager=self.manager, | ||
platform=self.platform, | ||
dependencies=self.dependencies, | ||
url=self.url, | ||
hash=self.hash, | ||
category=category, | ||
source=self.source, | ||
build=self.build, | ||
optional=category != "main", | ||
) | ||
for category in sorted(self.categories) | ||
] | ||
|
||
|
||
class Lockfile(StrictModel): | ||
|
@@ -121,34 +126,45 @@ def _toposort(package: List[LockedDependency]) -> List[LockedDependency]: | |
|
||
def to_v1(self) -> LockfileV1: | ||
return LockfileV1( | ||
package=[p.to_v1() for p in self.package], | ||
package=[out for p in self.package for out in p.to_v1()], | ||
metadata=self.metadata, | ||
) | ||
|
||
|
||
def _locked_dependency_v1_to_v2(dep: LockedDependencyV1) -> LockedDependency: | ||
def _locked_dependency_v1_to_v2(dep: List[LockedDependencyV1]) -> LockedDependency: | ||
"""Convert a LockedDependency from v1 to v2. | ||
|
||
* Remove the optional field (it is always equal to category != "main") | ||
""" | ||
assert len(dep) > 0 | ||
assert all(d.key() == dep[0].key() for d in dep) | ||
assert len(set(d.category for d in dep)) == len(dep) | ||
|
||
return LockedDependency( | ||
name=dep.name, | ||
version=dep.version, | ||
manager=dep.manager, | ||
platform=dep.platform, | ||
dependencies=dep.dependencies, | ||
url=dep.url, | ||
hash=dep.hash, | ||
category=dep.category, | ||
source=dep.source, | ||
build=dep.build, | ||
name=dep[0].name, | ||
version=dep[0].version, | ||
manager=dep[0].manager, | ||
platform=dep[0].platform, | ||
dependencies=dep[0].dependencies, | ||
url=dep[0].url, | ||
hash=dep[0].hash, | ||
categories={d.category for d in dep}, | ||
source=dep[0].source, | ||
build=dep[0].build, | ||
) | ||
|
||
|
||
def lockfile_v1_to_v2(lockfile_v1: LockfileV1) -> Lockfile: | ||
"""Convert a Lockfile from v1 to v2.""" | ||
final_dependencies = defaultdict(list) | ||
for dep in lockfile_v1.package: | ||
final_dependencies[dep.key()].append(dep) | ||
|
||
return Lockfile( | ||
package=[_locked_dependency_v1_to_v2(p) for p in lockfile_v1.package], | ||
package=[ | ||
_locked_dependency_v1_to_v2(v1_pkgs) | ||
for v1_pkgs in final_dependencies.values() | ||
], | ||
metadata=lockfile_v1.metadata, | ||
) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before, we used to map each package (LockedDependency) to 1 root dependency that is its ancestor (Dependency). A root dependency is a dependency specified in a source file. So the root dependency uses the package as a sub-dependency. As the comment says, we would try to use the first root.
Now, we need to store all root dependencies, in case there are multiple that use a package as a sub-dependency. That will allow us to take multiple
category
values, one from each root, and combine to create thecategories
value for a package.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the core change for the second commit.