Improve and simplify logic related to labels (#11)

Author: almarklein

_version.py

@@ -1,5 +1,5 @@
 """
-_version.py v1.5
+_version.py v1.6
 
 Simple version string management, using a hard-coded version string
 for simplicity and compatibility, while adding git info at runtime.
@@ -38,51 +38,70 @@
 
 def get_version() -> str:
     """Get the version string."""
-    if repo_dir:
-        return get_extended_version()
-    return __version__
+    try:
+        if repo_dir:
+            release, post, tag, dirty = get_version_info_from_git()
+            result = get_extended_version(release, post, tag, dirty)
 
+            # Warn if release does not match base_version.
+            # Can happen between bumping and tagging. And also when merging a
+            # version bump into a working branch, because we use --first-parent.
+            if release and release != base_version:
+                release2, _post, _tag, _dirty = get_version_info_from_git(
+                    first_parent=False
+                )
+                if release2 != base_version:
+                    warning(
+                        f"{project_name} version from git ({release})"
+                        f" and __version__ ({base_version}) don't match."
+                    )
 
-def get_extended_version() -> str:
-    """Get an extended version string with information from git."""
-    release, post, labels = get_version_info_from_git()
+            return result
+
+    except Exception as err:
+        # Failsafe.
+        warning(f"Error getting refined version: {err}")
 
-    # Start version string (__version__ string is leading)
+    return base_version
+
+
+def get_extended_version(release: str, post: str, tag: str, dirty: str) -> str:
+    """Get an extended version string with information from git."""
+    # Start version string (__version__ string is leading).
     version = base_version
-    tag_prefix = "#"
+    labels = []
 
     if release and release != base_version:
-        # Can happen between bumping and tagging. And also when merging a
-        # version bump into a working branch, because we use --first-parent.
-        release2, _post, _labels = get_version_info_from_git(first_parent=False)
-        if release2 != base_version:
-            warning(
-                f"{project_name} version from git ({release})"
-                f" and __version__ ({base_version}) don't match."
-            )
-        version += "+from_tag_" + release.replace(".", "_")
-        tag_prefix = "."
-
-    # Add git info
-    if post and post != "0":
+        pre_label = "from_tag_" + release.replace(".", "_")
+        labels = [pre_label, f"post{post}", tag, dirty]
+    elif post and post != "0":
         version += f".post{post}"
-        if labels:
-            version += tag_prefix + ".".join(labels)
-    elif labels and labels[-1] == "dirty":
-        version += tag_prefix + ".".join(labels)
-
+        labels = [tag, dirty]
+    elif dirty:
+        labels = [tag, dirty]
+    else:
+        # If not post and not dirty, show 'clean' version without git tag.
+        pass
+
+    # Compose final version (remove empty labels, e.g. when not dirty).
+    # Everything after the '+' is not sortable (does not get in version_info).
+    label_str = ".".join(label for label in labels if label)
+    if label_str:
+        version += "+" + label_str
     return version
 
 
-def get_version_info_from_git(*, first_parent: bool = True) -> str:
+def get_version_info_from_git(
+    *, first_parent: bool = True
+) -> tuple[str, str, str, str]:
     """
-    Get (release, post, labels) from Git.
+    Get (release, post, tag, dirty) from Git.
 
     With `release` the version number from the latest tag, `post` the
-    number of commits since that tag, and `labels` a tuple with the
-    git-hash and optionally a dirty flag.
+    number of commits since that tag, `tag` the git hash, and `dirty` a string
+    that is either empty or says 'dirty'.
     """
-    # Call out to Git
+    # Call out to Git.
     command = ["git", "describe", "--long", "--always", "--tags", "--dirty"]
     if first_parent:
         command.append("--first-parent")
@@ -92,9 +111,9 @@ def get_version_info_from_git(*, first_parent: bool = True) -> str:
         warning(f"Could not get {project_name} version: {e}")
         p = None
 
-    # Parse the result into parts
+    # Parse the result into parts.
     if p is None:
-        parts = (None, None, "unknown")
+        parts = ("", "", "unknown")
     else:
         output = p.stdout.decode(errors="ignore")
         if p.returncode:
@@ -105,40 +124,50 @@ def get_version_info_from_git(*, first_parent: bool = True) -> str:
                 + "\n\nstderr: "
                 + stderr
             )
-            parts = (None, None, "unknown")
+            parts = ("", "", "unknown")
         else:
             parts = output.strip().lstrip("v").split("-")
             if len(parts) <= 2:
                 # No tags (and thus no post). Only git hash and maybe 'dirty'.
-                parts = (None, None, *parts)
+                parts = ("", "", *parts)
 
-    # Return unpacked parts
-    release, post, *labels = parts
-    return release, post, labels
+    # Return unpacked parts.
+    release = parts[0]
+    post = parts[1]
+    tag = parts[2]
+    dirty = "dirty" if len(parts) > 3 else ""
+    return release, post, tag, dirty
 
 
 def version_to_tuple(v: str) -> tuple:
     parts = []
-    for part in v.split("."):
-        p, _, h = part.partition("#")
-        if p:
-            parts.append(p)
-        if h:
-            parts.append("#" + h)
-    return tuple(int(i) if i.isnumeric() else i for i in parts)
+    for part in v.split("+", maxsplit=1)[0].split("."):
+        if not part:
+            pass
+        elif part.startswith("post"):
+            try:
+                parts.extend(["post", int(part[4:])])
+            except ValueError:
+                parts.append(part)
+        else:
+            try:
+                parts.append(int(part))
+            except ValueError:
+                parts.append(part)
+    return tuple(parts)
 
 
 def warning(m: str) -> None:
     logger.warning(m)
 
 
-# Apply the versioning
+# Apply the versioning.
 base_version = __version__
 __version__ = get_version()
 version_info = version_to_tuple(__version__)
 
 
-# The CLI part
+# The CLI part.
 
 CLI_USAGE = """
 _version.py

test_version.py

@@ -7,15 +7,56 @@
 
 import _version
 
+example_git_output = [
+    (("", "", "", ""), "0.0.0"),
+    (("", "", "unknown", ""), "0.0.0"),
+    (("", "", "unknown", "dirty"), "0.0.0+unknown.dirty"),
+    (("0.0.0", "", "abcd", ""), "0.0.0"),
+    (("0.0.0", "", "abcd", "dirty"), "0.0.0+abcd.dirty"),
+    (("0.0.0", "1", "abcd", ""), "0.0.0.post1+abcd"),
+    (("0.0.0", "1", "abcd", "dirty"), "0.0.0.post1+abcd.dirty"),
+]
+
+
+def test_get_extended_version() -> None:
+    """Test get_extended_version function."""
+    for args, ref in example_git_output:
+        v = _version.get_extended_version(*args)
+        assert v == ref, f"{args} -> {v} != {ref}"
+
+
+def test_failsafe() -> None:
+    """Test that in case anything errors, we don't crash."""
+    ori_repo_dir = _version.repo_dir
+    del _version.repo_dir
+
+    try:
+        v = _version.get_version()
+        assert v == "0.0.0"
+
+    finally:
+        _version.repo_dir = ori_repo_dir
+
+
 example_versions = [
     ("1", (1,)),
     ("1.2", (1, 2)),
     ("1.2.3", (1, 2, 3)),
     ("1.2.3", (1, 2, 3)),
-    ("0.29.0.post4#g3175010", (0, 29, 0, "post4", "#g3175010")),
-    ("2.6.0#gcd877db.dirty", (2, 6, 0, "#gcd877db", "dirty")),
-    ("0.15.0.post16#g63b1a427.dirty", (0, 15, 0, "post16", "#g63b1a427", "dirty")),
-    ("1.#foo", (1, "#foo")),
+    ("1.2.3.post9", (1, 2, 3, "post", 9)),
+    ("1.2.3.post10", (1, 2, 3, "post", 10)),
+    ("0.29.0.post4+g3175010", (0, 29, 0, "post", 4)),
+    (
+        "2.6.0+gcd877db.dirty",
+        (
+            2,
+            6,
+            0,
+        ),
+    ),
+    ("0.15.0.post16+g63b1a427.dirty", (0, 15, 0, "post", 16)),
+    ("2.6.1+from_tag_2_6_0.post3.g72c1d22", (2, 6, 1)),
+    ("1.+foo", (1,)),
 ]