Improve template url generations (#3108)

1. Check that a git repository is defined and get the basic facts
2. Try to get a fully-qualified reference from branch (rev-parse
   --symbolic-full-name), tag (describe --tags), or the first
   entry of for-each-ref
3. Set fmf_id.ref to a short name if possible. Depends on if we
   are a branch or tag.
4. Get all remaining facts from the fully-qualified reference and
   for-each-ref

Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Co-authored-by: Petr Šplíchal <[email protected]>

docs/releases.rst

@@ -20,6 +20,11 @@ new ``--flavor`` argument. Using this argument the user can choose between a
 ``custom`` flavor where user must provide a custom template using a
 ``--template-path`` argument.
 
+The ``fmf-id.ref`` will now try to report the most human-readable committish
+reference, either branch, tag, git-describe, or if all fails the commit hash.
+You may encounter this in the verbose log of ``tmt tests show`` or plan/test
+imports.
+
 
 tmt-1.36.1
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

docs/templates/story.rst.j2

@@ -54,7 +54,7 @@
     :param tmt.base.Link link: link to render.
 #}
 {% macro emit_tmt_repo_link(link) %}
-{{ _emit_remote_link(link, link.target, "https://github.com/teemtee/tmt/tree/" + (STORY.fmf_id.ref or 'main') + "/" + link.target.lstrip('/')) }}
+{{ _emit_remote_link(link, link.target, link.target | web_git_url(STORY.fmf_id.url, STORY.fmf_id.ref)) }}
 {% endmacro %}
 
 {#
@@ -78,7 +78,7 @@
 {% elif plugin_name == "provision/testcloud" %}
     {% set plugin_name = "provision/virtual" %}
 {% endif %}
-{{ _emit_remote_link(link, plugin_name, "https://github.com/teemtee/tmt/tree/" + (STORY.fmf_id.ref or 'main') + "/" + link.target) }}
+{{ _emit_remote_link(link, plugin_name, link.target | web_git_url(STORY.fmf_id.url, STORY.fmf_id.ref)) }}
 {% endmacro %}
 
 {#

tests/discover/filtering.sh

@@ -86,7 +86,10 @@ rlJournalStart
         rlAssertEquals "Check that number of fmf-ids equals to tests number" \
                        "$ids_amount" "$tests_amount"
         rlAssertEquals "Check url" "$url_discover" "$url_fmf_id"
-        rlAssertEquals "Check ref" "HEAD" "$ref_fmf_id"
+        # Expected ref comes from running git describe on tmt repo:
+        #  git describe --tags eae4d527
+        expected_ref="0.10-8-geae4d527"
+        rlAssertEquals "Check ref" "$expected_ref" "$ref_fmf_id"
     rlPhaseEnd
 
     rlPhaseStartTest 'fmf-id (w/o url): Show fmf ids for discovered tests'

tests/unit/test_utils.py

@@ -54,7 +54,7 @@ def local_git_repo(tmppath: Path) -> Path:
     origin = tmppath / 'origin'
     origin.mkdir()
 
-    run(Command('git', 'init'), cwd=origin)
+    run(Command('git', 'init', '-b', 'main'), cwd=origin)
     run(
         Command('git', 'config', '--local', 'user.email', '[email protected]'),
         cwd=origin)
@@ -920,6 +920,45 @@ def test_not_pushed(cls, origin_and_local_git_repo: tuple[Path, Path], root_logg
             False, 'Not pushed changes in .fmf/version main.fmf')
 
 
+@pytest.mark.parametrize("git_ref",
+                         ["tag", "branch", "merge", "commit"])
+def test_fmf_id(local_git_repo, root_logger, git_ref):
+    run(Command('git', 'checkout', '-b', 'other_branch'), cwd=local_git_repo)
+    # Initialize tmt tree with a test
+    tmt.Tree.init(logger=root_logger, path=local_git_repo, template="empty", force=False)
+    with (local_git_repo / "test.fmf").open("w") as f:
+        f.write('test: echo')
+    run(Command('git', 'add', '-A'), cwd=local_git_repo)
+    run(Command('git', 'commit', '-m', 'Initialized tmt tree'), cwd=local_git_repo)
+    commit_hash = run(Command('git', 'rev-parse', 'HEAD'), cwd=local_git_repo).stdout.strip()
+
+    if git_ref == "tag":
+        run(Command('git', 'tag', 'some_tag'), cwd=local_git_repo)
+        run(Command('git', 'checkout', 'some_tag'), cwd=local_git_repo)
+    if git_ref == "commit":
+        # Create an empty commit and checkout the previous commit
+        run(Command('git', 'commit', '--allow-empty', '-m',
+            'Random other commit'), cwd=local_git_repo)
+        run(Command('git', 'checkout', 'HEAD^'), cwd=local_git_repo)
+    if git_ref == "merge":
+        run(Command('git', 'checkout', '--detach', 'main'), cwd=local_git_repo)
+        run(Command('git', 'merge', 'other_branch'), cwd=local_git_repo)
+        commit_hash = run(Command('git', 'rev-parse', 'HEAD'),
+                          cwd=local_git_repo).stdout.strip()
+
+    fmf_id = tmt.utils.fmf_id(name="/test", fmf_root=local_git_repo, logger=root_logger)
+    assert fmf_id.git_root == local_git_repo
+    assert fmf_id.ref is not None
+    if git_ref == "tag":
+        assert fmf_id.ref == "some_tag"
+    if git_ref == "branch":
+        assert fmf_id.ref == "other_branch"
+    if git_ref == "merge":
+        assert fmf_id.ref == commit_hash
+    if git_ref == "commit":
+        assert fmf_id.ref == commit_hash
+
+
 class TestGitAdd:
     @classmethod
     def test_not_in_repository(

tmt/base.py

@@ -795,7 +795,6 @@ def fmf_id(self) -> FmfId:
         return tmt.utils.fmf_id(
             name=self.name,
             fmf_root=self.fmf_root,
-            always_get_ref=True,
             logger=self._logger)
 
     @functools.cached_property

tmt/utils/__init__.py

@@ -4086,56 +4086,28 @@ def fmf_id(
         *,
         name: str,
         fmf_root: Path,
-        always_get_ref: bool = False,
         logger: tmt.log.Logger) -> 'tmt.base.FmfId':
     """ Return full fmf identifier of the node """
 
-    def run(command: Command) -> str:
-        """ Run command, return output """
-        try:
-            result = command.run(cwd=fmf_root, logger=logger)
-            if result.stdout is None:
-                return ""
-            return result.stdout.strip()
-        except RunError:
-            # Always return an empty string in case 'git' command is run in a non-git repo
-            return ""
-
     from tmt.base import FmfId
+    from tmt.utils.git import GitInfo
 
     fmf_id = FmfId(fmf_root=fmf_root, name=name)
+    git_info = GitInfo.from_fmf_root(fmf_root=fmf_root, logger=logger)
 
-    # Prepare url (for now handle just the most common schemas)
-    branch = run(Command("git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"))
-    try:
-        remote_name = branch[:branch.index('/')]
-    except ValueError:
-        remote_name = 'origin'
-    remote = run(Command("git", "config", "--get", f"remote.{remote_name}.url"))
-
-    from tmt.utils.git import default_branch, git_root, public_git_url
-    fmf_id.url = public_git_url(remote) if remote else None
+    # If we couldn't resolve the git metadata, keep the git metadata empty
+    if not git_info:
+        return fmf_id
 
+    # Populate the git metadata from GitInfo
+    # TODO: Save GitInfo inside FmfId as-is
+    fmf_id.git_root = git_info.git_root
     # Construct path (if different from git root)
-    fmf_id.git_root = git_root(fmf_root=fmf_root, logger=logger)
-
-    if fmf_id.git_root:
-        if fmf_id.git_root.resolve() != fmf_root.resolve():
-            fmf_id.path = Path('/') / fmf_root.relative_to(fmf_id.git_root)
-
-        # Get the ref (skip for the default)
-        fmf_id.default_branch = default_branch(repository=fmf_id.git_root, logger=logger)
-        if fmf_id.default_branch is None:
-            fmf_id.ref = None
-        else:
-            ref = run(Command("git", "rev-parse", "--abbrev-ref", "HEAD"))
-            if ref != fmf_id.default_branch or always_get_ref:
-                fmf_id.ref = ref
-            else:
-                # Note that it is a valid configuration without having a default
-                # branch here. Consumers of returned fmf_id object should check
-                # the fmf_id contains everything they need.
-                fmf_id.ref = None
+    if fmf_id.git_root.resolve() != fmf_root.resolve():
+        fmf_id.path = Path('/') / fmf_root.relative_to(fmf_id.git_root)
+    fmf_id.ref = git_info.ref
+    fmf_id.url = git_info.url
+    fmf_id.default_branch = git_info.default_branch
 
     return fmf_id
 

tmt/utils/git.py

@@ -1,6 +1,7 @@
 """ Test Metadata Utilities """
 
 
+import dataclasses
 import functools
 import os
 import re
@@ -26,6 +27,118 @@
     import tmt.base
 
 
+@dataclasses.dataclass
+class GitInfo:
+    """ Data container for commonly queried git data. """
+
+    #: Path to the git root.
+    git_root: Path
+
+    #: Most human-readable git ref.
+    ref: str
+
+    #: Git remote linked to the current git ref.
+    remote: str
+
+    #: Default branch of the remote.
+    default_branch: Optional[str]
+
+    #: Public url of the remote.
+    url: Optional[str]
+
+    @classmethod
+    @functools.cache
+    def from_fmf_root(cls, *, fmf_root: Path, logger: tmt.log.Logger) -> Optional["GitInfo"]:
+        """
+        Get the current git info of an fmf tree.
+
+        :param fmf_root: Root path of the fmf tree
+        :param logger: Current tmt logger
+        :return: Git info container or ``None`` if git metadata could not be resolved
+        """
+
+        def run(command: Command) -> str:
+            """
+            Run command, return output.
+            We don't need the stderr here, but we need exit status.
+            """
+            result = command.run(cwd=fmf_root, logger=logger)
+            if result.stdout is None:
+                return ""
+            return result.stdout.strip()
+
+        # Prepare url (for now handle just the most common schemas)
+        try:
+            # Check if we are a git repo
+            run(Command("git", "rev-parse", "--is-inside-work-tree"))
+
+            # Initialize common git facts
+            # Get some basic git references for HEAD
+            all_refs = run(Command("git", "for-each-ref", "--format=%(refname)", "--points-at=@"))
+            logger.debug("git all_refs", all_refs, level=3)
+            # curr_ref is either HEAD or fully-qualified (branch) reference
+            curr_ref = run(Command("git", "rev-parse", "--symbolic-full-name", "@"))
+            logger.debug("git initial curr_ref", curr_ref, level=3)
+            # Get the top-level git_root
+            _git_root = git_root(fmf_root=fmf_root, logger=logger)
+            assert _git_root is not None  # narrow type
+        except RunError:
+            # Not a git repo, everything should be pointing to None at this point
+            return None
+
+        if curr_ref != "HEAD":
+            # The reference is fully qualified -> we are on a branch
+            # Get the short name
+            branch = run(Command("git", "for-each-ref", "--format=%(refname:short)", curr_ref))
+            ref = branch
+        else:
+            # Not on a branch, check if we are on a tag or just a refs
+            try:
+                tags = run(Command("git", "describe", "--tags"))
+                logger.debug("git tags", tags, level=3)
+                # Is it possible to find which tag was used to checkout?
+                # Now we just assume the first tag is the one we want
+                tag_used = tags.splitlines()[0]
+                logger.debug("Using tag", tag_used, level=3)
+                # Point curr_ref to the fully-qualified ref
+                curr_ref = f"refs/tags/{tag_used}"
+                ref = tag_used
+            except RunError:
+                # We are not on a tag, just use the first available reference
+                curr_ref = all_refs.splitlines()[0] if all_refs else curr_ref
+                # Point the ref to the commit
+                commit = run(Command("git", "rev-parse", curr_ref))
+                logger.debug("Using commit", commit, level=3)
+                ref = commit
+
+        logger.debug("curr_ref used", curr_ref, level=3)
+        remote_name = run(
+            Command(
+                "git",
+                "for-each-ref",
+                "--format=%(upstream:remotename)",
+                curr_ref))
+        if not remote_name:
+            # If no specific upstream is defined, default to `origin`
+            remote_name = "origin"
+        try:
+            remote = run(Command("git", "config", "--get", f"remote.{remote_name}.url"))
+            url = public_git_url(remote)
+            _default_branch = default_branch(
+                repository=_git_root, remote=remote, logger=logger)
+        except RunError:
+            url = None
+            _default_branch = None
+
+        return GitInfo(
+            git_root=_git_root,
+            ref=ref,
+            remote=remote_name,
+            url=url,
+            default_branch=_default_branch
+            )
+
+
 # Avoid multiple subprocess calls for the same url
 @functools.cache
 def check_git_url(url: str, logger: tmt.log.Logger) -> str:

tmt/utils/templates.py

@@ -21,6 +21,7 @@
 import jinja2.exceptions
 
 from tmt.utils import GeneralError, Path
+from tmt.utils.git import web_git_url
 
 
 def _template_filter_basename(  # type: ignore[reportUnusedFunction,unused-ignore]
@@ -283,6 +284,25 @@ def _template_filter_listed(  # type: ignore[reportUnusedFunction,unused-ignore]
         join=join))
 
 
+def _template_filter_web_git_url(  # type: ignore[reportUnusedFunction,unused-ignore]
+        path_str: str,
+        url: str,
+        ref: str) -> str:
+    """
+    Sanitize git url using :py:meth:`tmt.utils.web_git_url`
+
+    .. code-block:: jinja
+
+        {{ "/path/to/the/code.py" | web_git_url(STORY.fmf_id.url, STORY.fmf_id.ref) }}
+
+        {{ "/tmt/base.py" | web_git_url("https://github.com/teemtee/tmt.git", "main") }}
+        -> https://github.com/teemtee/tmt/tree/main/tmt/base.py
+
+    """
+    path = Path(path_str) if path_str else None
+    return web_git_url(url, ref, path)
+
+
 TEMPLATE_FILTERS: dict[str, Callable[..., Any]] = {
     _name.replace('_template_filter_', ''): _obj
     for _name, _obj in locals().items() if callable(_obj) and _name.startswith('_template_filter_')