git log in %changelog fixups

Use git-describe to get latest git tag and then using git-log obtain all
commit messages b/w {last_tag}..HEAD and feed the output to %changelog

* fall back to old behaviour when git-log or git-describe fails
* former tests were too complicated - simplify them or merge into
  existing one

Fixes #738
Carries #776

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

packit/upstream.py

@@ -409,8 +409,19 @@ def _get_archive_path_from_output(self, outputs: List[str]) -> Optional[Path]:
                     raise ex
         return None
 
-    def _get_last_tag(self):
-        last_tag = run_command(["git", "describe", "--tags", "--abbrev=0"], output=True).strip()
+    def get_last_tag(self) -> Optional[str]:
+        """ get last git-tag from the repo """
+        try:
+            last_tag = run_command(
+                ["git", "describe", "--tags", "--abbrev=0"],
+                output=True,
+                cwd=self.local_project.working_dir,
+            ).strip()
+        except PackitCommandFailedError as ex:
+            logger.debug(f"{ex!r}")
+            logger.info("Can't describe this repository, are there any git tags?")
+            # no tags in the git repo
+            return None
         return last_tag
 
     def fix_spec(self, archive: str, version: str, commit: str):
@@ -460,9 +471,26 @@ def fix_spec(self, archive: str, version: str, commit: str):
             f"{sanitized_current_branch}{git_desc_suffix}"
         )
 
-        last_tag = self._get_last_tag()
-        cmd = ["git", "log", "--pretty=format:- %s (%an)", f"{last_tag}..HEAD"]
-        msg = run_command(cmd, output=True).strip()
+        last_tag = self.get_last_tag()
+        msg = ""
+        if last_tag:
+            # let's print changes b/w the last tag and now;
+            # ambiguous argument '0.1.0..HEAD': unknown revision or path not in the working tree.
+            # Use '--' to separate paths from revisions, like this
+            cmd = [
+                "git",
+                "log",
+                "--pretty=format:- %s (%an)",
+                f"{last_tag}..HEAD",
+                "--",
+            ]
+            msg = run_command(
+                cmd, output=True, cwd=self.local_project.working_dir
+            ).strip()
+        if not msg:
+            # no describe, no tag - just a boilerplate message w/ commit hash
+            # or, there were no changes b/w HEAD and last_tag, which implies last_tag == HEAD
+            msg = f"- Development snapshot ({commit})"
         logger.debug(f"Setting Release in spec to {release!r}.")
         # instead of changing version, we change Release field
         # upstream projects should take care of versions

tests/integration/test_upstream.py

@@ -221,6 +221,9 @@ def test_create_srpm_git_desc_release(upstream_instance):
     build_srpm(srpm)
     assert re.match(r".+beer-0.1.0-1\.\d+\.\w+\.fc\d{2}.src.rpm$", str(srpm))
 
+    changelog = ups.specfile.spec_content.section("%changelog")
+    assert "- More awesome changes (Packit Test Suite)" in changelog
+
 
 def test_github_app(upstream_instance, tmp_path):
     u, ups = upstream_instance
@@ -246,3 +249,8 @@ def test_github_app(upstream_instance, tmp_path):
         )
         in ups.config.services
     )
+
+
+def test_get_last_tag(upstream_instance):
+    u, ups = upstream_instance
+    assert ups.get_last_tag() == "0.1.0"

tests/unit/test_upstream.py

@@ -1,13 +1,10 @@
 import pytest
-
 from flexmock import flexmock
+
 from packit.actions import ActionName
 from packit.local_project import LocalProject
 from packit.upstream import Upstream
 from tests.spellbook import get_test_config
-from packit.command_handler import LocalCommandHandler
-import packit.upstream as upstream_module
-import datetime
 
 
 @pytest.fixture
@@ -102,38 +99,3 @@ def test_get_commands_for_actions(action_config, result):
         local_project=flexmock(),
     )
     assert ups.get_commands_for_actions(action=ActionName.create_archive) == result
-
-
-def test__get_last_tag(upstream_mock):
-    flexmock(upstream_module)\
-        .should_receive("run_command")\
-        .once()\
-        .with_args(["git", "describe", "--tags", "--abbrev=0"], output=True)\
-        .and_return("1.5")
-    last_tag = upstream_mock._get_last_tag()
-    assert last_tag == "1.5"
-
-
-def test_fix_spec_changelog_message_should_be_actual_commits():
-    flexmock(upstream_module).should_receive("run_command")\
-        .times(2)\
-        .and_return("v1.5.0-14-g241472")\
-        .and_return("actual commit")
-    version = "random-version"
-    commit = "61fad52477f511eab9ba"
-    specfile = flexmock()
-    specfile.should_receive("get_release_number").and_return("1.5.aerd")
-    specfile.should_receive("set_spec_version")
-    upstream = Upstream(
-        package_config=flexmock(
-            actions={ActionName.fix_spec: flexmock()}, synced_files=flexmock()
-        ),
-        config=flexmock(),
-        local_project=flexmock(),
-    )
-    flexmock(upstream)
-    upstream.should_receive("_fix_spec_source").with_args("archive")
-    upstream.should_receive("_fix_spec_prep").with_args(version)
-    upstream.should_receive("_get_last_tag").and_return("1.5")
-    upstream.should_receive("specfile").and_return(specfile)
-    upstream.fix_spec("archive", version, commit)