Better differentiate between commit ID and version for GL canonical name

Signed-off-by: Tobias Wolf <[email protected]>

Author: NotTheEvilOne

src/gardenlinux/features/__main__.py

@@ -21,12 +21,14 @@
 _ARGS_TYPE_ALLOWED = [
     "cname",
     "cname_base",
+    "commit_id",
     "features",
     "platforms",
     "flags",
     "elements",
     "arch",
     "version",
+    "version_and_commit_id",
     "graph",
 ]
 
@@ -41,10 +43,11 @@ def main() -> None:
     parser = argparse.ArgumentParser()
 
     parser.add_argument("--arch", dest="arch")
+    parser.add_argument("--cname", dest="cname")
+    parser.add_argument("--commit", dest="commit")
     parser.add_argument("--feature-dir", default="features")
-    parser.add_argument("--cname")
-    parser.add_argument("--default-arch")
-    parser.add_argument("--default-version")
+    parser.add_argument("--default-arch", dest="default_arch")
+    parser.add_argument("--default-version", dest="default_version")
     parser.add_argument("--version", dest="version")
 
     parser.add_argument(
@@ -68,7 +71,7 @@ def main() -> None:
 
     arch = args.arch
     flavor = None
-    commit_id = None
+    commit_id = args.commit
     gardenlinux_root = path.dirname(args.feature_dir)
     version = args.version
 
@@ -80,8 +83,7 @@ def main() -> None:
 
     if version is None or version == "":
         try:
-            version_data = get_version_and_commit_id_from_files(gardenlinux_root)
-            version = f"{version_data[0]}-{version_data[1]}"
+            version, commit_id = get_version_and_commit_id_from_files(gardenlinux_root)
         except RuntimeError as exc:
             logging.debug(
                 "Failed to parse version information for GL root '{0}': {1}".format(
@@ -92,7 +94,7 @@ def main() -> None:
             version = args.default_version
 
     if args.cname:
-        cname = CName(args.cname, arch=arch, version=version)
+        cname = CName(args.cname, arch=arch, commit_id=commit_id, version=version)
 
         arch = cname.arch
         flavor = cname.flavor
@@ -108,7 +110,11 @@ def main() -> None:
             "Architecture could not be determined and no default architecture set"
         )
 
-    if version is None or version == "" and (args.type in ("cname", "version")):
+    if (
+        version is None
+        or version == ""
+        and (args.type in ("cname", "commit_id", "version", "version_and_commit_id"))
+    ):
         raise RuntimeError("Version not specified and no default version set")
 
     feature_dir_name = path.basename(args.feature_dir)
@@ -160,7 +166,11 @@ def main() -> None:
             print(",".join(features_by_type["element"]))
         elif args.type == "flags":
             print(",".join(features_by_type["flag"]))
+    elif args.type == "commit_id":
+        print(commit_id)
     elif args.type == "version":
+        print(version)
+    elif args.type == "version_and_commit_id":
         print(f"{version}-{commit_id}")
 
 

src/gardenlinux/features/cname.py

@@ -25,13 +25,14 @@ class CName(object):
                  Apache License, Version 2.0
     """
 
-    def __init__(self, cname, arch=None, version=None):
+    def __init__(self, cname, arch=None, commit_id=None, version=None):
         """
         Constructor __init__(CName)
 
-        :param cname:   Canonical name to represent
-        :param arch:    Architecture if not part of cname
-        :param version: Version if not part of cname
+        :param cname:     Canonical name to represent
+        :param arch:      Architecture if not part of cname
+        :param commit_id: Commit ID if not part of cname
+        :param version:   Version if not part of cname
 
         :since: 0.7.0
         """
@@ -64,11 +65,16 @@ def __init__(self, cname, arch=None, version=None):
             self._arch = arch
 
         if self._version is None and version is not None:
-            re_match = re.match("([a-z0-9.]+)(-([a-z0-9]+))?$", version)
-            assert re_match, f"Not a valid version {version}"
+            # Support version values formatted as <version>-<commit_id>
+            if commit_id is None:
+                re_match = re.match("([a-z0-9.]+)(-([a-z0-9]+))?$", version)
+                assert re_match, f"Not a valid version {version}"
 
-            self._commit_id = re_match[3]
-            self._version = re_match[1]
+                self._commit_id = re_match[3]
+                self._version = re_match[1]
+            else:
+                self._commit_id = commit_id
+                self._version = version
 
     @property
     def arch(self) -> Optional[str]:

src/gardenlinux/features/cname_main.py

@@ -32,6 +32,7 @@ def main():
     parser = argparse.ArgumentParser()
 
     parser.add_argument("--arch", dest="arch")
+    parser.add_argument("--commit", dest="commit")
     parser.add_argument("--feature-dir", default="features")
     parser.add_argument("--version", dest="version")
     parser.add_argument("cname")
@@ -46,6 +47,7 @@ def main():
     assert re_match, f"Not a valid GardenLinux canonical name {args.cname}"
 
     arch = args.arch
+    commit_id = args.commit
     gardenlinux_root = dirname(args.feature_dir)
     version = args.version
 
@@ -54,16 +56,15 @@ def main():
 
     if not version:
         try:
-            version_data = get_version_and_commit_id_from_files(gardenlinux_root)
-            version = f"{version_data[0]}-{version_data[1]}"
+            version, commit_id = get_version_and_commit_id_from_files(gardenlinux_root)
         except RuntimeError as exc:
             logging.warning(
                 "Failed to parse version information for GL root '{0}': {1}".format(
                     gardenlinux_root, exc
                 )
             )
 
-    cname = CName(args.cname, arch=arch, version=version)
+    cname = CName(args.cname, arch=arch, commit_id=commit_id, version=version)
 
     assert cname.arch, "Architecture could not be determined"
 

tests/features/test_cname_main.py

@@ -1,9 +1,10 @@
-import sys
-import types
 import logging
 import pytest
+import sys
+import types
 
 import gardenlinux.features.cname_main as cname_main
+from gardenlinux.features import CName
 
 
 def test_main_happy(monkeypatch, capsys):
@@ -14,14 +15,6 @@ def test_main_happy(monkeypatch, capsys):
     argv = ["prog", "--arch", "amd64", "--version", "1.0-abc123", "flav-amd64"]
     monkeypatch.setattr(sys, "argv", argv)
 
-    class FakeCName:
-        def __init__(self, cname, arch=None, version=None):
-            self.arch = arch
-            self.flavor = "flav"
-            self.version_and_commit_id = "1.0-abc123"
-
-    monkeypatch.setattr(cname_main, "CName", FakeCName)
-
     class FakeGraph:
         in_degree = lambda self: [("f1", 0)]
         edges = [("f1", "f2")]
@@ -62,14 +55,6 @@ def test_main_version_from_file(monkeypatch, capsys):
         lambda root: ("2.0", "abcdef12"),
     )
 
-    class FakeCName:
-        def __init__(self, cname, arch=None, version=None):
-            self.arch = arch
-            self.flavor = "flav"
-            self.version_and_commit_id = version
-
-    monkeypatch.setattr(cname_main, "CName", FakeCName)
-
     class FakeParser:
         def __init__(self, *a, **k):
             pass
@@ -110,15 +95,6 @@ def raise_runtime(_):
         cname_main, "get_version_and_commit_id_from_files", raise_runtime
     )
 
-    # Patch CName to control attributes
-    class FakeCName:
-        def __init__(self, cname, arch=None, version=None):
-            self.arch = arch
-            self.flavor = "flav"
-            self.version_and_commit_id = version
-
-    monkeypatch.setattr(cname_main, "CName", FakeCName)
-
     # Patch Parser for minimal valid graph
     class FakeParser:
         def __init__(self, *a, **k):
@@ -165,14 +141,6 @@ def test_cname_main_missing_arch_in_cname_raises(monkeypatch):
     argv = ["prog", "--version", "1.0", "flav"]
     monkeypatch.setattr(sys, "argv", argv)
 
-    class FakeCName:
-        def __init__(self, cname, arch=None, version=None):
-            self.arch = None  # Force missing arch
-            self.flavor = "flav"
-            self.version_and_commit_id = "1.0-abc"
-
-    monkeypatch.setattr(cname_main, "CName", FakeCName)
-
     # Act / Assert
     with pytest.raises(AssertionError):
         cname_main.main()

tests/features/test_main.py

@@ -132,6 +132,27 @@ def test_main_prints_arch(monkeypatch, capsys):
     assert "amd64" in out
 
 
+def test_main_prints_commit_id(monkeypatch, capsys):
+    # Arrange
+    argv = ["prog", "--arch", "amd64", "--features", "f1", "commit_id"]
+    monkeypatch.setattr(sys, "argv", argv)
+    monkeypatch.setattr(
+        fema,
+        "Parser",
+        lambda *a, **kw: types.SimpleNamespace(filter=lambda *a, **k: None),
+    )
+    # Patch get_version_and_commit_id_from_files
+    monkeypatch.setattr(
+        fema, "get_version_and_commit_id_from_files", lambda root: ("1.2.3", "abcdef12")
+    )
+
+    # Act
+    fema.main()
+
+    captured = capsys.readouterr()
+    assert "abcdef12" == captured.out.strip()
+
+
 def test_main_prints_flags_elements_platforms(monkeypatch, capsys):
     # Arrange
     argv = [
@@ -186,7 +207,28 @@ def test_main_prints_version(monkeypatch, capsys):
     fema.main()
 
     captured = capsys.readouterr()
-    assert "1.2.3-abcdef12" in captured.out
+    assert "1.2.3" == captured.out.strip()
+
+
+def test_main_prints_version_and_commit_id(monkeypatch, capsys):
+    # Arrange
+    argv = ["prog", "--arch", "amd64", "--features", "f1", "version_and_commit_id"]
+    monkeypatch.setattr(sys, "argv", argv)
+    monkeypatch.setattr(
+        fema,
+        "Parser",
+        lambda *a, **kw: types.SimpleNamespace(filter=lambda *a, **k: None),
+    )
+    # Patch get_version_and_commit_id_from_files
+    monkeypatch.setattr(
+        fema, "get_version_and_commit_id_from_files", lambda root: ("1.2.3", "abcdef12")
+    )
+
+    # Act
+    fema.main()
+
+    captured = capsys.readouterr()
+    assert "1.2.3-abcdef12" == captured.out.strip()
 
 
 def test_main_arch_raises_missing_verison(monkeypatch, capsys):
@@ -202,15 +244,6 @@ def test_main_arch_raises_missing_verison(monkeypatch, capsys):
 
 def test_main_with_cname_print_cname(monkeypatch, capsys):
     # Arrange
-    class FakeCName:
-        def __init__(self, cname, arch=None, version=None):
-            self.arch = arch
-            self.flavor = "flav"
-            self.commit_id = "abc123"
-            self.version = version
-
-    monkeypatch.setattr(fema, "CName", FakeCName)
-
     class FakeGraph:
         def in_degree(self):
             # Simulate a graph where one feature has no dependencies