Allow passing nested values to env substitution

dolfinus · dolfinus · commit 545fb448cca6 · 2022-01-06T20:38:19.000+03:00
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -47,6 +47,11 @@ Changelog
 
       Add ``tag_formatter`` option
 
+    .. change::
+      :tags: core, feature
+
+      Allow nested default values to be passed to ``env`` substitution
+
     .. change::
       :tags: tests, feature
 
diff --git a/docs/substitutions/env.rst b/docs/substitutions/env.rst
@@ -22,4 +22,5 @@ You can pass 2 positional options to this substitution:
   - no default value meaus that variable will be replaced with literal ``"UNKNOWN"``
   - ``some value`` - just plain text
   - ``{ccount}`` - any other substitution is supported (**without nesting**)
+  - ``{env:MISSINGVAR:{ccount}}`` - nested too
   - ``IGNORE`` - just substitute empty string
diff --git a/setuptools_git_versioning.py b/setuptools_git_versioning.py
@@ -15,12 +15,12 @@
 from setuptools.dist import Distribution
 from six.moves import collections_abc
 
-DEFAULT_TEMPLATE = "{tag}"  # type: str
-DEFAULT_DEV_TEMPLATE = "{tag}.post{ccount}+git.{sha}"  # type: str
-DEFAULT_DIRTY_TEMPLATE = "{tag}.post{ccount}+git.{sha}.dirty"  # type: str
+DEFAULT_TEMPLATE = "{tag}"
+DEFAULT_DEV_TEMPLATE = "{tag}.post{ccount}+git.{sha}"
+DEFAULT_DIRTY_TEMPLATE = "{tag}.post{ccount}+git.{sha}.dirty"
 DEFAULT_STARTING_VERSION = "0.0.1"
-ENV_VARS_REGEXP = re.compile(r"\{env:([^:}]+):?([^}]+}?)?\}", re.IGNORECASE | re.UNICODE)  # type: re.Pattern
-TIMESTAMP_REGEXP = re.compile(r"\{timestamp:?([^:}]+)?\}", re.IGNORECASE | re.UNICODE)  # type: re.Pattern
+ENV_VARS_REGEXP = re.compile(r"\{env:(?P<name>[^:}]+):?(?P<default>[^}]+\}*)?\}", re.IGNORECASE | re.UNICODE)
+TIMESTAMP_REGEXP = re.compile(r"\{timestamp:?(?P<fmt>[^:}]+)?\}", re.IGNORECASE | re.UNICODE)
 
 DEFAULT_CONFIG = {
     "template": DEFAULT_TEMPLATE,
@@ -195,30 +195,50 @@ def read_version_from_file(path):  # type: (Union[str, os.PathLike]) -> str
         return file.read().strip()
 
 
-def subst_env_variables(template):  # type: (str) -> str
-    if "{env" in template:
-        for var, default in ENV_VARS_REGEXP.findall(template):
-            if default.upper() == "IGNORE":
-                default = ""
-            elif not default:
-                default = "UNKNOWN"
+def substitute_env_variables(template):  # type: (str) -> str
+    for var, default in ENV_VARS_REGEXP.findall(template):
+        if default.upper() == "IGNORE":
+            default = ""
+        elif not default:
+            default = "UNKNOWN"
 
-            value = os.environ.get(var, default)
-            template, _ = ENV_VARS_REGEXP.subn(value, template, count=1)
+        log.warning(var)
+        log.warning(default)
+        value = os.environ.get(var, default)
+        log.warning(os.environ)
+        log.warning(value)
+        template, _ = ENV_VARS_REGEXP.subn(value, template, count=1)
+        log.warning(template)
 
     return template
 
 
-def subst_timestamp(template):  # type: (str) -> str
-    if "{timestamp" in template:
-        now = datetime.now()
-        for fmt in TIMESTAMP_REGEXP.findall(template):
-            result = now.strftime(fmt or "%s")
-            template, _ = TIMESTAMP_REGEXP.subn(result, template, count=1)
+def substitute_timestamp(template):  # type: (str) -> str
+    now = datetime.now()
+    for fmt in TIMESTAMP_REGEXP.findall(template):
+        result = now.strftime(fmt or "%s")
+        template, _ = TIMESTAMP_REGEXP.subn(result, template, count=1)
 
     return template
 
 
+def resolve_substitutions(template, *args, **kwargs):  # type: (str, *Any, **Any) -> str
+    while True:
+        if "{env" in template:
+            new_template = substitute_env_variables(template)
+            if new_template == template:
+                break
+            else:
+                template = new_template
+        else:
+            break
+
+    if "{timestamp" in template:
+        template = substitute_timestamp(template)
+
+    return template.format(*args, **kwargs)
+
+
 def import_reference(
     ref,  # type: str
     package_name=None,  # Optional[str]
@@ -433,10 +453,7 @@ def version_from_git(
     else:
         t = template
 
-    t = subst_env_variables(t)
-    t = subst_timestamp(t)
-
-    version = t.format(sha=full_sha[:8], tag=tag, ccount=ccount, branch=branch, full_sha=full_sha)
+    version = resolve_substitutions(t, sha=full_sha[:8], tag=tag, ccount=ccount, branch=branch, full_sha=full_sha)
 
     # Ensure local version label only contains permitted characters
     public, sep, local = version.partition("+")
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -9,7 +9,7 @@
 import toml
 import uuid
 
-from typing import Any, Dict, Optional
+from typing import Any, Callable, Dict, Optional
 
 log = logging.getLogger(__name__)
 root = os.path.dirname(os.path.dirname(__file__))
@@ -163,16 +163,24 @@ def create_config(request):
     return request.param
 
 
-def typed_config(repo, config_creator, config_type, template=None, config=None):
+def typed_config(
+    repo,  # type: str
+    config_creator,  # type: Callable
+    config_type,  # type: str
+    template=None,  # type: Optional[str]
+    template_name=None,  # type: Optional[str]
+    config=None,  # type: Optional[dict]
+):
     if config_type == "tag":
         cfg = {}
     else:
         cfg = {"version_file": "VERSION.txt", "count_commits_from_version_file": True}
 
-    if config_type == "tag":
-        template_name = "template"
-    else:
-        template_name = "dev_template"
+    if template_name is None:
+        if config_type == "tag":
+            template_name = "template"
+        else:
+            template_name = "dev_template"
 
     if template:
         cfg[template_name] = template
diff --git a/tests/test_substitution.py b/tests/test_substitution.py
@@ -1,12 +1,42 @@
 from datetime import datetime
 import pytest
 import re
+import subprocess
 
-from tests.conftest import execute, get_version
+from tests.conftest import execute, get_version_setup_py, create_file, create_setup_py
 
 pytestmark = pytest.mark.all
 
 
+@pytest.mark.parametrize(
+    "template",
+    [
+        "{tag}",
+        "{tag}+a{tag}",
+    ],
+)
+def test_substitution_tag(repo, template):
+    create_setup_py(repo, {"template": template})
+    execute(repo, "git tag 1.2.3")
+
+    assert get_version_setup_py(repo) == template.format(tag="1.2.3")
+
+
+@pytest.mark.parametrize(
+    "dev_template",
+    [
+        "{tag}.{ccount}",
+        "{tag}.{ccount}+a{ccount}",
+    ],
+)
+def test_substitution_ccount(repo, dev_template):
+    create_setup_py(repo, {"dev_template": dev_template})
+    execute(repo, "git tag 1.2.3")
+    create_file(repo)
+
+    assert get_version_setup_py(repo) == dev_template.format(tag="1.2.3", ccount=1)
+
+
 @pytest.mark.parametrize(
     "branch, suffix",
     [
@@ -18,45 +48,103 @@
         ("post", ".post"),
     ],
 )
-def test_substitution_branch(repo, template_config, create_config, branch, suffix):
+@pytest.mark.parametrize(
+    "template, real_template",
+    [
+        ("{tag}{branch}", "{tag}{suffix}0"),
+        ("{tag}{branch}+a{branch}", "{tag}{suffix}0+a{branch}"),
+    ],
+)
+def test_substitution_branch(repo, template, real_template, branch, suffix):
     execute(repo, "git checkout -b {branch}".format(branch=branch))
-    template_config(repo, create_config, template="{tag}{branch}0")
+    create_setup_py(repo, {"template": template})
+    execute(repo, "git tag 1.2.3")
 
-    assert get_version(repo) == "1.2.3{suffix}0".format(suffix=suffix)
+    assert get_version_setup_py(repo) == real_template.format(tag="1.2.3", branch=branch, suffix=suffix)
 
 
 @pytest.mark.parametrize(
-    "template, pipeline_id, suffix",
+    "dev_template, pipeline_id, suffix",
     [
         # leading zeros are removed by setuptools
-        ("{tag}.post{env:PIPELINE_ID:123}", "0234", "234"),
+        ("{tag}.post{env:PIPELINE_ID}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID}", "0234", "234"),
+        ("{tag}.post{env:PIPELINE_ID}", None, "UNKNOWN"),
+        ("{tag}.post{env:PIPELINE_ID}+abc{env:ANOTHER_ENV}", "234", "234+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID}+abc{env:ANOTHER_ENV}", None, "UNKNOWN+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID}+abc{env:MISSING_ENV}", "234", "234+abcunknown"),
+        ("{tag}.post{env:PIPELINE_ID}+abc{env:MISSING_ENV}", None, "UNKNOWN+abcUNKNOWN"),
         ("{tag}.post{env:PIPELINE_ID:123}", "234", "234"),
         ("{tag}.post{env:PIPELINE_ID:123}", None, "123"),
-        ("{tag}.post{env:PIPELINE_ID:IGNORE}", "0234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:ANOTHER_ENV}", "234", "234+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:ANOTHER_ENV}", None, "123+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV}", "234", "234+abcunknown"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV}", None, "123+abcunknown"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV:5.6.7}", "234", "234+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV:5.6.7}", None, "123+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV:B.C:D}", "234", "234+abcb.c.d"),  # allowed
+        ("{tag}.post{env:PIPELINE_ID:123}+abc{env:MISSING_ENV:B-C%D}", None, "123+abcb.c.d"),
         ("{tag}.post{env:PIPELINE_ID:IGNORE}", "234", "234"),
         ("{tag}.post{env:PIPELINE_ID:IGNORE}", None, "0"),
-        ("{tag}.post{env:PIPELINE_ID:{ccount}}", "0234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:ANOTHER_ENV}", "234", "234+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:ANOTHER_ENV}", None, "0+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:MISSING_ENV:5.6.7}", "234", "234+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:MISSING_ENV:5.6.7}", None, "0+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:MISSING_ENV:IGNORE}", "234", "234+abc"),
+        ("{tag}.post{env:PIPELINE_ID:IGNORE}+abc{env:MISSING_ENV:IGNORE}", None, "0+abc"),
         ("{tag}.post{env:PIPELINE_ID:{ccount}}", "234", "234"),
-        ("{tag}.post{env:PIPELINE_ID:{ccount}}", None, "0"),
-        ("{tag}.post{env:PIPELINE_ID}", "0234", "234"),
-        ("{tag}.post{env:PIPELINE_ID}", "234", "234"),
-        ("{tag}.post{env:PIPELINE_ID}", None, "UNKNOWN"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}", None, "1"),
+        ("{tag}.post{env:PIPELINE_ID:{timestamp:%Y}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{timestamp:%Y}}", None, datetime.now().year),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:ANOTHER_ENV}", "234", "234+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:ANOTHER_ENV}", None, "1+abc3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:5.6.7}", "234", "234+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:5.6.7}", None, "1+abc5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:IGNORE}", "234", "234+abc"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:IGNORE}", None, "1+abc"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:{ccount}}", "234", "234+abc1"),
+        ("{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:{ccount}}", None, "1+abc1"),
+        (
+            "{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:{timestamp:%Y}}",
+            "234",
+            "234+abc" + str(datetime.now().year),
+        ),
+        (
+            "{tag}.post{env:PIPELINE_ID:{ccount}}+abc{env:MISSING_ENV:{timestamp:%Y}}",
+            None,
+            "1+abc" + str(datetime.now().year),
+        ),
+        ("{tag}.post{env:PIPELINE_ID:{env:ANOTHER_ENV}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{env:ANOTHER_ENV}}", None, "3.4.5"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV}}", None, "UNKNOWN"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:5.6.7}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:5.6.7}}", None, "5.6.7"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:{ccount}}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:{ccount}}}", None, "1"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:{timestamp:%Y}}}", "234", "234"),
+        ("{tag}.post{env:PIPELINE_ID:{env:MISSING_ENV:{timestamp:%Y}}}", None, datetime.now().year),
+        # empty env variable name
+        ("{tag}.post{env: }", "234", "UNKNOWN"),
     ],
 )
-def test_substitution_env(repo, template_config, create_config, template, pipeline_id, suffix):
-    template_config(repo, create_config, template=template)
+def test_substitution_env(repo, dev_template, pipeline_id, suffix):
+    create_setup_py(repo, {"dev_template": dev_template})
+    execute(repo, "git tag 1.2.3")
+    create_file(repo)
 
-    env = {}
+    env = {"ANOTHER_ENV": "3.4.5"}
     if pipeline_id is not None:
-        env = {"PIPELINE_ID": pipeline_id}
+        env["PIPELINE_ID"] = pipeline_id
 
-    assert get_version(repo, env=env) == "1.2.3.post{suffix}".format(suffix=suffix)
+    assert get_version_setup_py(repo, env=env) == "1.2.3.post{suffix}".format(suffix=suffix)
 
 
 @pytest.mark.parametrize(
     "template, fmt, callback",
     [
         ("{tag}.post{timestamp}", "{tag}.post{}", lambda dt: (int(dt.strftime("%s")) // 100,)),
+        ("{tag}.post{timestamp:}", "{tag}.post{}", lambda dt: (int(dt.strftime("%s")) // 100,)),
         ("{tag}.post{timestamp:%s}", "{tag}.post{}", lambda dt: (int(dt.strftime("%s")) // 100,)),
         (
             "{timestamp:%Y}.{timestamp:%m}.{timestamp:%d}+{timestamp:%H%M%S}",
@@ -68,10 +156,13 @@ def test_substitution_env(repo, template_config, create_config, template, pipeli
             "{tag}.post{ccount}+{}",
             lambda dt: (dt.strftime("%Y.%m.%dt%H.%M"),),
         ),
+        # unknown format
+        ("{tag}+git{timestamp:%i}", "{tag}+git.i", lambda x: []),
     ],
 )
-def test_substitution_timestamp(repo, template_config, create_config, template, fmt, callback):
-    template_config(repo, create_config, template=template)
+def test_substitution_timestamp(repo, template, fmt, callback):
+    create_setup_py(repo, {"template": template})
+    execute(repo, "git tag 1.2.3")
 
     value = fmt.format(tag="1.2.3", ccount=0, *callback(datetime.now()))
     pattern = re.compile(r"([^\d\w])0+(\d+[^\d\w]|\d+$)")
@@ -81,4 +172,25 @@ def test_substitution_timestamp(repo, template_config, create_config, template,
         if new_value == value:
             break
         value = new_value
-    assert new_value in get_version(repo)
+    assert new_value in get_version_setup_py(repo)
+
+
+@pytest.mark.parametrize(
+    "template",
+    [
+        "{tag}+a{env}",
+        "{tag}+a{env:}",
+        "{tag}+a{env:MISSING_ENV:{}",
+        "{tag}+a{env:MISSING_ENV:{{}}",
+        "{tag}+a{env:MISSING_ENV:}}",
+        "{tag}+a{env:MISSING_ENV:{}}}",
+        "{tag}+a{timestamp:A:B}",
+        "{tag}+a{timestamp:{%Y}",
+    ],
+)
+def test_substitution_wrong_format(repo, template):
+    create_setup_py(repo, {"template": template})
+    execute(repo, "git tag 1.2.3")
+
+    with pytest.raises(subprocess.CalledProcessError):
+        get_version_setup_py(repo)
