-
Notifications
You must be signed in to change notification settings - Fork 1
[ABI Dependency] Best-Effort support for pip
#123
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
base: main
Are you sure you want to change the base?
Changes from all commits
7f2e494
9a4f968
4e3c685
90caea7
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,7 +1,14 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| import os | ||
| from importlib import metadata | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| from packaging.utils import canonicalize_name | ||
| from packaging.version import Version | ||
|
|
||
| from variantlib.constants import VARIANT_ABI_DEPENDENCY_NAMESPACE | ||
| from variantlib.models.variant import VariantDescription | ||
| from variantlib.models.variant import VariantFeature | ||
| from variantlib.models.variant import VariantProperty | ||
|
|
@@ -20,6 +27,20 @@ | |
| from variantlib.protocols import VariantFeatureValue | ||
| from variantlib.protocols import VariantNamespace | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _normalize_package_name(name: str) -> str: | ||
| # VALIDATION_FEATURE_NAME_REGEX does not accepts "-" | ||
| return canonicalize_name(name).replace("-", "_") | ||
|
|
||
|
|
||
| def _generate_version_matches(version: str) -> Generator[str]: | ||
| vspec = Version(version) | ||
| yield f"{vspec.major}" | ||
| yield f"{vspec.major}.{vspec.minor}" | ||
| yield f"{vspec.major}.{vspec.minor}.{vspec.micro}" | ||
|
|
||
|
|
||
| def filter_variants( | ||
| vdescs: list[VariantDescription], | ||
|
|
@@ -124,8 +145,68 @@ def sort_and_filter_supported_variants( | |
| :param property_priorities: Ordered list of `VariantProperty` objects. | ||
| :return: Sorted and filtered list of `VariantDescription` objects. | ||
| """ | ||
|
|
||
| validate_type(vdescs, list[VariantDescription]) | ||
| validate_type(supported_vprops, list[VariantProperty]) | ||
|
|
||
| if namespace_priorities is None: | ||
| namespace_priorities = [] | ||
|
|
||
| # Avoiding modification in place | ||
| namespace_priorities = namespace_priorities.copy() | ||
| supported_vprops = supported_vprops.copy() | ||
|
|
||
| # ======================================================================= # | ||
| # ABI DEPENDENCY INJECTION # | ||
| # ======================================================================= # | ||
|
|
||
| # 1. Manually fed from environment variable | ||
| # Note: come first for "implicit higher priority" | ||
| # Env Var Format: `VARIANT_ABI_DEPENDENCY=packageA==1.2.3,...,packageZ==7.8.9` | ||
| if variant_abi_deps_env := os.environ.get("VARIANT_ABI_DEPENDENCY"): | ||
| for pkg_spec in variant_abi_deps_env.split(","): | ||
| try: | ||
| pkg_name, pkg_version = pkg_spec.split("==", maxsplit=1) | ||
| except ValueError: | ||
| logger.exception( | ||
| "`VARIANT_ABI_DEPENDENCY` received an invalid value " | ||
| "`%(pkg_spec)s`. It will be ignored.\n" | ||
| "Expected format: `packageA==1.2.3,...,packageZ==7.8.9`.", | ||
| {"pkg_spec": pkg_spec}, | ||
| ) | ||
| continue | ||
|
|
||
| supported_vprops.extend( | ||
DEKHTIARJonathan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| VariantProperty( | ||
| namespace=VARIANT_ABI_DEPENDENCY_NAMESPACE, | ||
| feature=_normalize_package_name(pkg_name), | ||
| value=_ver, | ||
| ) | ||
| for _ver in _generate_version_matches(pkg_version) | ||
| ) | ||
|
|
||
| # 2. Automatically populate from the current python environment | ||
| packages = [ | ||
| (dist.name, dist.version) for dist in metadata.distributions() | ||
| ] | ||
| for pkg_name, pkg_version in sorted(packages): | ||
| supported_vprops.extend( | ||
| VariantProperty( | ||
| namespace=VARIANT_ABI_DEPENDENCY_NAMESPACE, | ||
| feature=_normalize_package_name(pkg_name), | ||
| value=_ver, | ||
| ) | ||
| for _ver in _generate_version_matches(pkg_version) | ||
|
Comment on lines
+194
to
+199
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. This (quite complex) bit is repeated twice — move it to a function? 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'm not sure it's worth it - it's easy to read - moving the whole thing to a function would make it slightly less easy to read. You tell me - honestly I don't mind enough to argue |
||
| ) | ||
|
|
||
| # 3. Adding `VARIANT_ABI_DEPENDENCY_NAMESPACE` at the back of`namespace_priorities` | ||
| namespace_priorities.append(VARIANT_ABI_DEPENDENCY_NAMESPACE) | ||
DEKHTIARJonathan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| # ======================================================================= # | ||
| # NULL VARIANT # | ||
| # ======================================================================= # | ||
|
|
||
| # Adding the `null-variant` to the list - always "compatible" | ||
| if (null_variant := VariantDescription()) not in vdescs: | ||
| """Add a null variant description to the list.""" | ||
| # This is needed to ensure that we always consider the null variant | ||
|
|
@@ -139,7 +220,9 @@ def sort_and_filter_supported_variants( | |
| """No supported properties provided, return no variants.""" | ||
| return [] | ||
|
|
||
| validate_type(supported_vprops, list[VariantProperty]) | ||
| # ======================================================================= # | ||
| # FILTERING # | ||
| # ======================================================================= # | ||
|
|
||
| # Step 1: we remove any duplicate, or unsupported `VariantDescription` on | ||
| # this platform. | ||
|
|
@@ -153,6 +236,10 @@ def sort_and_filter_supported_variants( | |
| ) | ||
| ) | ||
|
|
||
| # ======================================================================= # | ||
| # SORTING # | ||
| # ======================================================================= # | ||
|
|
||
| # Step 2: we sort the supported `VariantProperty`s based on their respective | ||
| # priority. | ||
| sorted_supported_vprops = sort_variant_properties( | ||
|
|
||
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.
Mixed feelings about this. We can't handle versions with more than 3 components here.
My main worry is upstream doing something like succession of
2.8, 2.8.1, 2.8.2...— but I guess we are hacking it into 2.8.0 then, so that would kinda work.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.
If you have a better idea let me know but IMHO it's reasonable and easy to read.