ENH version migrator refactor

conda_forge_tick/migrators/version.py

@@ -1,5 +1,7 @@
 import os
 import typing
+import re
+import io
 import jinja2
 import collections.abc
 import hashlib
@@ -38,12 +40,15 @@
     "checksum",
 ]
 
+# matches valid jinja2 vars
+JINJA2_VAR_RE = re.compile('{{ ((?:[a-zA-Z]|(?:_[a-zA-Z0-9]))[a-zA-Z0-9_]*) }}')
+
 logger = logging.getLogger("conda_forge_tick.migrators.version")
 
 
 def _gen_key_selector(dct: MutableMapping, key: str):
     for k in dct:
-        if key in k:
+        if k == key or (CONDA_SELECTOR in k and k.split(CONDA_SELECTOR)[0] == key):
             yield k
 
 
@@ -95,22 +100,54 @@ def _try_url_and_hash_it(url: str, hash_type: str):
     return None
 
 
+def _render_jinja2(tmpl, context):
+    return jinja2.Template(tmpl, undefined=jinja2.StrictUndefined).render(**context)
+
+
 def _get_new_url_tmpl_and_hash(url_tmpl: str, context: MutableMapping, hash_type: str):
     new_url_tmpl = None
     new_hash = None
 
     try:
-        url = (
-            jinja2
-            .Template(url_tmpl)
-            .render(**context)
-        )
+        url = _render_jinja2(url_tmpl, context)
         new_hash = _try_url_and_hash_it(url, hash_type)
     except jinja2.UndefinedError:
         new_hash = None
 
     if new_hash is None:
-        # try some stuff
+        # replace ext only
+        for exthave, extrep in permutations(EXTS, 2):
+            try:
+                new_url_tmpl = (
+                    url_tmpl
+                    .replace(exthave, extrep)
+                )
+                url = _render_jinja2(new_url_tmpl, context)
+                new_hash = _try_url_and_hash_it(url, hash_type)
+            except jinja2.UndefinedError:
+                new_hash = None
+
+            if new_hash is not None:
+                break
+
+    if new_hash is None:
+        # replace v's only
+        for vhave, vrep in permutations(["v{{ v", "{{ v"]):
+            try:
+                new_url_tmpl = (
+                    url_tmpl
+                    .replace(vhave, vrep)
+                )
+                url = _render_jinja2(new_url_tmpl, context)
+                new_hash = _try_url_and_hash_it(url, hash_type)
+            except jinja2.UndefinedError:
+                new_hash = None
+
+            if new_hash is not None:
+                break
+
+    if new_hash is None:
+        # try both
         for (vhave, vrep), (exthave, extrep) in product(
             permutations(["v{{ v", "{{ v"]),
             permutations(EXTS, 2),
@@ -121,11 +158,7 @@ def _get_new_url_tmpl_and_hash(url_tmpl: str, context: MutableMapping, hash_type
                     .replace(vhave, vrep)
                     .replace(exthave, extrep)
                 )
-                url = (
-                    jinja2
-                    .Template(new_url_tmpl)
-                    .render(**context)
-                )
+                url = _render_jinja2(new_url_tmpl, context)
                 new_hash = _try_url_and_hash_it(url, hash_type)
             except jinja2.UndefinedError:
                 new_hash = None
@@ -178,7 +211,7 @@ def _try_to_update_version(cmeta: Any, src: str, hash_type: str):
     if ha is None:
         return False
 
-    updated_version = False
+    updated_version = True
 
     # first we compile all selectors
     possible_selectors = _compile_all_selectors(cmeta, src)
@@ -190,7 +223,7 @@ def _try_to_update_version(cmeta: Any, src: str, hash_type: str):
     # these are then updated
 
     for selector in possible_selectors:
-        logger.debug("selector: %s", selector)
+        logger.info("selector: %s", selector)
         url_key = 'url'
         if selector is not None:
             for key in _gen_key_selector(src, 'url'):
@@ -217,12 +250,36 @@ def _try_to_update_version(cmeta: Any, src: str, hash_type: str):
             else:
                 context[key] = val
 
-        logger.debug('url key: %s', url_key)
-        logger.debug('hash key: %s', hash_key)
-        logger.debug(
-            "    jinja2 context:\n        %s",
-            "\n        ".join(pprint.pformat(context).split("\n")),
-        )
+        # get all of the possible variables in the url
+        # if we do not have them or any selector versions, then
+        # we are not updating something so fail
+        jinja2_var_set = set()
+        if isinstance(src[url_key], collections.abc.MutableSequence):
+            for url_tmpl in src[url_key]:
+                jinja2_var_set |= set(JINJA2_VAR_RE.findall(url_tmpl))
+        else:
+            jinja2_var_set |= set(JINJA2_VAR_RE.findall(src[url_key]))
+
+        jinja2_var_set |= set(JINJA2_VAR_RE.findall(src[hash_key]))
+
+        skip_this_selector = False
+        for var in jinja2_var_set:
+            if len(list(_gen_key_selector(cmeta.jinja2_vars, var))) == 0:
+                updated_version = False
+                break
+
+            # we have a variable, but maybe not this selector?
+            # that's ok
+            if var not in context:
+                skip_this_selector = True
+
+        if skip_this_selector:
+            continue
+
+        logger.info('url key: %s', url_key)
+        logger.info('hash key: %s', hash_key)
+        logger.info(
+            "jinja2 context: %s", pprint.pformat(context))
 
         # now try variations of the url to get the hash
         if isinstance(src[url_key], collections.abc.MutableSequence):
@@ -259,7 +316,11 @@ def _try_to_update_version(cmeta: Any, src: str, hash_type: str):
             else:
                 new_hash = None
 
-        updated_version |= (new_hash is not None)
+        logger.info('new hash: %s', new_hash)
+        if new_hash is not None:
+            logger.info('new url tmpl: %s', new_url_tmpl)
+
+        updated_version &= (new_hash is not None)
 
     return updated_version
 
@@ -325,6 +386,12 @@ def migrate(
         with open(os.path.join(recipe_dir, "meta.yaml"), 'r') as fp:
             cmeta = CondaMetaYAML(fp.read())
 
+        # cache round-tripped yaml for testing later
+        s = io.StringIO()
+        cmeta.dump(s)
+        s.seek(0)
+        old_meta_yaml = s.read()
+
         version = attrs["new_version"]
         assert isinstance(version, str)
 
@@ -340,33 +407,55 @@ def migrate(
 
         # replace the version
         if 'version' in cmeta.jinja2_vars:
+            # cache old version for testing later
+            old_version = cmeta.jinja2_vars['version']
             cmeta.jinja2_vars['version'] = version
         else:
-            logger.warning(
+            logger.ciritical(
                 "Migrations do not work on versions "
-                "outside of jinja2 w/ selectors"
+                "not specified with jinja2!"
             )
-            return super().migrate(recipe_dir, attrs)
+            return {}
+
+        if len(list(_gen_key_selector(cmeta.meta, 'source'))) > 0:
+            did_update = True
+            for src_key in _gen_key_selector(cmeta.meta, 'source'):
+                if isinstance(cmeta.meta[src_key], collections.abc.MutableSequence):
+                    for src in cmeta.meta[src_key]:
+                        did_update &= _try_to_update_version(cmeta, src, hash_type)
+                else:
+                    did_update &= _try_to_update_version(
+                        cmeta,
+                        cmeta.meta[src_key],
+                        hash_type,
+                    )
+        else:
+            did_update = False
 
-        did_update = False
-        for src_key in _gen_key_selector(cmeta.meta, 'source'):
-            if isinstance(cmeta.meta[src_key], collections.abc.MutableSequence):
-                for src in cmeta.meta[src_key]:
-                    did_update |= _try_to_update_version(cmeta, src, hash_type)
-            else:
-                did_update |= _try_to_update_version(
-                    cmeta,
-                    cmeta.meta[src_key],
-                    hash_type,
-                )
+        if did_update:
+            # if the yaml did not change, then we did not migrate actually
+            cmeta.jinja2_vars['version'] = old_version
+            s = io.StringIO()
+            cmeta.dump(s)
+            s.seek(0)
+            if s.read() == old_meta_yaml:
+                did_update = False
+
+            # put back version
+            cmeta.jinja2_vars['version'] = version
 
         if did_update:
             with indir(recipe_dir):
                 with open('meta.yaml', 'w') as fp:
                     cmeta.dump(fp)
                 self.set_build_number("meta.yaml")
 
-        return super().migrate(recipe_dir, attrs)
+            return super().migrate(recipe_dir, attrs)
+        else:
+            logger.critical(
+                "Recipe did not change in version migration!"
+            )
+            return {}
 
     def pr_body(self, feedstock_ctx: FeedstockContext) -> str:
         pred = [

conda_forge_tick/recipe_parser/_parser.py

@@ -197,8 +197,16 @@ def _replace_jinja2_vars(lines: List[str], jinja2_vars: dict) -> List[str]:
     pairs in `jinja2_vars` will be added as new statements at the top.
     """
     # these regex find jinja2 set statements without and with selectors
-    jinja2_re = re.compile(r'^(\s*){%\s*set\s*(.*)=\s*(.*)%}(.*)')
-    jinja2_re_selector = re.compile(r'^(\s*){%\s*set\s*(.*)=\s*(.*)%}\s*#\s*\[(.*)\]')
+    jinja2_re = re.compile(
+        r'^(\s*){%\s*set\s*([a-zA-Z0-9_]+)\s*=\s*(['
+        + "'"
+        + r'"a-zA-Z0-9_.\-:\\\/]+)\s*%}(.*)'
+    )
+    jinja2_re_selector = re.compile(
+        r'^(\s*){%\s*set\s*([a-zA-Z0-9_]+)\s*=\s*(['
+        + "'"
+        + r'"a-zA-Z0-9_.\-:\\\/]+)\s*%}\s*#\s*\[(.*)\]'
+    )
 
     all_jinja2_keys = set(list(jinja2_vars.keys()))
     used_jinja2_keys = set()

tests/test_migrators.py

@@ -1734,7 +1734,8 @@ def run_test_migration(
     m_ctx = MigratorContext(mm_ctx, m)
     m.bind_to_ctx(m_ctx)
 
-    mr_out.update(bot_rerun=False)
+    if mr_out:
+        mr_out.update(bot_rerun=False)
     with open(os.path.join(tmpdir, "meta.yaml"), "w") as f:
         f.write(inp)
 
@@ -1762,7 +1763,9 @@ def run_test_migration(
     pmy["version"] = pmy['meta_yaml']["package"]["version"]
     pmy["req"] = set()
     for k in ["build", "host", "run"]:
-        pmy["req"] |= set(pmy['meta_yaml'].get("requirements", {}).get(k, set()))
+        req = pmy['meta_yaml'].get("requirements", {}) or {}
+        _set = req.get(k) or set()
+        pmy["req"] |= set(_set)
     pmy["raw_meta_yaml"] = inp
     pmy.update(kwargs)
 
@@ -1772,6 +1775,8 @@ def run_test_migration(
 
     mr = m.migrate(tmpdir, pmy)
     assert mr_out == mr
+    if not mr:
+        return
 
     pmy.update(PRed=[frozen_to_json_friendly(mr)])
     with open(os.path.join(tmpdir, "meta.yaml"), "r") as f:

tests/test_recipe_parser.py

@@ -19,6 +19,11 @@ def test_parsing():
 {% set name = 'val2' %}#[py3k and win]
 {% set version = '4.5.6' %}
 
+{% set build = 0 %}
+{% if False %}
+{% set build = build + 100 %}
+{% endif %}
+
 package:
   name: {{ name|lower }}
 
@@ -36,6 +41,11 @@ def test_parsing():
 {% set name = "val2" %}  # [py3k and win]
 {% set version = "4.5.6" %}
 
+{% set build = 0 %}
+{% if False %}
+{% set build = build + 100 %}
+{% endif %}
+
 package:
   name: {{ name|lower }}
 
@@ -72,6 +82,7 @@ def test_parsing():
     # now add stuff and test outputs
     cm.jinja2_vars['foo'] = 'bar'
     cm.jinja2_vars['xfoo__###conda-selector###__win or osx'] = 10
+    cm.jinja2_vars['build'] = 100
     cm.meta['about'] = 10
     cm.meta['requirements__###conda-selector###__win'] = 'blah'
     cm.meta['requirements__###conda-selector###__not win'] = 'not_win_blah'
@@ -88,6 +99,11 @@ def test_parsing():
 {% set name = "val2" %}  # [py3k and win]
 {% set version = "4.5.6" %}
 
+{% set build = 100 %}
+{% if False %}
+{% set build = build + 100 %}
+{% endif %}
+
 package:
   name: {{ name|lower }}