conda / conda-lock

Lightweight lockfile for conda environments
https://conda.github.io/conda-lock/
Other
456 stars 101 forks source link

Support `not`, `and`, and `or` for preprocess selector #595

Closed hoxbro closed 4 months ago

hoxbro commented 5 months ago

Checklist

What is the idea?

Looking at preprocessing-selectors. I was a bit hard to know that not, and, and or was not supported.

Why is this needed?

It will make it easier to use with an environment file that has dependencies that are only available on some platforms.

What should happen?

I took a quick try at implementing the request in conda_lock/src_parser/selectors.py

diff --git a/conda_lock/src_parser/selectors.py b/conda_lock/src_parser/selectors.py
index 657442f..12cee48 100644
--- a/conda_lock/src_parser/selectors.py
+++ b/conda_lock/src_parser/selectors.py
@@ -7,6 +7,13 @@ from typing import Iterator, Optional
 logger = logging.getLogger(__name__)

+def _check_selector(selector: str, cur_platform: set) -> bool:
+    if "not" in selector:
+        return selector.replace("not ", "") not in cur_platform
+    else:
+        return selector in cur_platform
+
+
 def filter_platform_selectors(
     content: str, platform: Optional[str] = None
 ) -> Iterator[str]:
@@ -32,8 +39,21 @@ def filter_platform_selectors(
             continue
         m = sel_pat.match(line)
         if platform and m:
-            cond = m.group(3)
-            if cond in platform_sel[platform]:
+            cond = m.group(3).strip()
+            cond_and = "and" in cond
+            cond_or = "or" in cond
+            if cond_and and cond_or:
+                logger.warning(
+                    f"selector `{cond}` contains both `and` and `or` which is not supported"
+                )
+            cur_platform = platform_sel[platform]
+            if cond_and:
+                check = all(_check_selector(c, cur_platform) for c in cond.split(" and "))
+            elif cond_or:
+                check = any(_check_selector(c, cur_platform) for c in cond.split(" or "))
+            else:
+                check = _check_selector(cond, cur_platform)
+            if check:
                 yield line
             else:
                 logger.warning(
Full file ``` python import logging import re from typing import Iterator, Optional logger = logging.getLogger(__name__) def _check_selector(selector: str, cur_platform: set) -> bool: if "not" in selector: return selector.replace("not ", "") not in cur_platform else: return selector in cur_platform def filter_platform_selectors( content: str, platform: Optional[str] = None ) -> Iterator[str]: """ """ # we support a very limited set of selectors that adhere to platform only # https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#preprocessing-selectors platform_sel = { "linux-64": {"linux64", "unix", "linux"}, "linux-aarch64": {"aarch64", "unix", "linux"}, "linux-ppc64le": {"ppc64le", "unix", "linux"}, # "osx64" is a selector unique to conda-build referring to # platforms on macOS and the Python architecture is x86-64 "osx-64": {"osx64", "osx", "unix"}, "osx-arm64": {"arm64", "osx", "unix"}, "win-64": {"win", "win64"}, } # This code is adapted from conda-build sel_pat = re.compile(r"(.+?)\s*(#.*)\[([^\[\]]+)\](?(2)[^\(\)]*)$") for line in content.splitlines(keepends=False): if line.lstrip().startswith("#"): continue m = sel_pat.match(line) if platform and m: cond = m.group(3).strip() cond_and = "and" in cond cond_or = "or" in cond if cond_and and cond_or: logger.warning( f"selector `{cond}` contains both `and` and `or` which is not supported" ) cur_platform = platform_sel[platform] if cond_and: check = all(_check_selector(c, cur_platform) for c in cond.split(" and ")) elif cond_or: check = any(_check_selector(c, cur_platform) for c in cond.split(" or ")) else: check = _check_selector(cond, cur_platform) if check: yield line else: logger.warning( f"filtered out line `{line}` on platform {platform} due to " f"non-matching selector `{cond}`" ) else: yield line ```
Small examples for testing purpose ``` python for n in filter_platform_selectors("- example # [ linux]", "linux-64"): pass for n in filter_platform_selectors("- example # [not win]", "linux-64"): pass for n in filter_platform_selectors("- example # [not win and not linux]", "linux-64"): pass for n in filter_platform_selectors("- example # [win or linux]", "linux-64"): pass ```

Additional Context

There is mention of this in https://github.com/conda/conda-lock/issues/278 in a comment, but looking at the issue, it seems much broader. If maintainers feel discussion should happen on that issue, please close this issue.

maresb commented 5 months ago

Hey, thanks so much @Hoxbro for taking a stab at this! I appreciate all the time you put into this, but I wouldn't be optimistic about getting something like this merged for two reasons (no fault of yours):

  1. My current priority is to properly fix the underlying categories issue. This is a substantial change currently underway, so at this moment I don't want to consider other stuff like this that will result in merge conflicts.
  2. Preprocessing selectors are on their way out. I'd claim that the prefix.dev team are currently driving the innovation in this space, and they have a more modern format proposal where conditionals are 1st-class citizens rather than hacky comments with a functional purpose.

I hope you don't find this too discouraging. I'm happy to leave this issue open, and we can reevaluate this if things don't go as expected or if we end up deciding against adoption of the prefix.dev convention.

hoxbro commented 4 months ago

Thank you for the quick response.

Will the modern proposal also work with environment.yml files?

You are welcome to close the issue

maresb commented 4 months ago

Thanks for your understanding.

Will the modern proposal also work with environment.yml files?

Good point, I linked to a recipe.yaml instead of an environment.yml. I expect that it should work, but I'm honestly behind on the latest developments in this space. I'd need to read up some more before I could give a definitive answer. If you track it down, then do please let me know! :slightly_smiling_face: