From 0c947900936e062914664c0e2a5f356326accb6a Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 1/12] Make argparse setup compatible with Python 3.6 Previously, ArgumentParser.add_subparsers() didn't accept the `required` argument. Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/cli.py b/rpmautospec/cli.py index c322b6e..b49f5e1 100644 --- a/rpmautospec/cli.py +++ b/rpmautospec/cli.py @@ -74,7 +74,9 @@ def get_cli_args(args: typing.List[str]) -> argparse.Namespace: # parsers for sub-commands - subparsers = parser.add_subparsers(dest="subcommand", required=True) + # ArgumentParser.add_subparsers() only accepts the `required` argument from Python 3.7 on. + subparsers = parser.add_subparsers(dest="subcommand") + subparsers.required = True for subcmd_module in (changelog, release, process_distgit): subcmd_name = subcmd_module.register_subcommand(subparsers) From de9a982d0fb83cc43f61d9b8baf426e5bdfbac12 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 2/12] Remove unused code Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/misc.py b/rpmautospec/misc.py index d28f253..7ecfef4 100644 --- a/rpmautospec/misc.py +++ b/rpmautospec/misc.py @@ -3,7 +3,6 @@ import logging import re import subprocess from pathlib import Path -from typing import List from typing import Optional from typing import Tuple from typing import Union @@ -18,8 +17,6 @@ import rpm AUTORELEASE_MACRO = "autorelease(e:s:hp)" AUTORELEASE_SENTINEL = "__AUTORELEASE_SENTINEL__" -release_re = re.compile(r"^(?P\d+)(?:(?P.*?)(?:\.(?P\d+))?)?$") -disttag_re = re.compile(r"\.?(?P[^\d\.]+)(?P\d+)") evr_re = re.compile(r"^(?:(?P\d+):)?(?P[^-:]+)(?:-(?P[^-:]+))?$") autochangelog_re = re.compile(r"\s*%(?:autochangelog|\{\??autochangelog\})\s*") @@ -47,26 +44,6 @@ def parse_evr(evr_str: str) -> Tuple[int, str, Optional[str]]: return epoch, match.group("version"), match.group("release") -def parse_epoch_version(epoch_version_str: str) -> Tuple[int, str]: - e, v, r = parse_evr(epoch_version_str) - if r is not None: - raise ValueError(epoch_version_str) - return e, v - - -def parse_release_tag(tag: str) -> Tuple[Optional[int], Optional[str], Optional[str]]: - pkgrel = middle = minorbump = None - match = release_re.match(tag) - if match: - pkgrel = int(match.group("pkgrel")) - middle = match.group("middle") - try: - minorbump = int(match.group("minorbump")) - except TypeError: - pass - return pkgrel, middle, minorbump - - def get_rpm_current_version(path: str, name: Optional[str] = None, with_epoch: bool = False) -> str: """Retrieve the current version set in the spec file named ``name``.spec at the given path. @@ -117,18 +94,6 @@ def koji_init(koji_url_or_session: Union[str, koji.ClientSession]) -> koji.Clien return _kojiclient -def get_package_builds(pkgname: str) -> List[dict]: - assert _kojiclient - - pkgid = _kojiclient.getPackageID(pkgname) - if not pkgid: - raise ValueError(f"Package {pkgname!r} not found!") - - # Don't add queryOpts={"order": "-nvr"} or similar, this sorts alphanumerically and and this is - # not how EVRs should be sorted. - return _kojiclient.listBuilds(pkgid, type="rpm") - - def query_current_git_commit_hash( path: str, log_options: typing.Optional[typing.List[str]] = None, From 1180df28ce59ceb64b3d5cb6954a849f1ef096d9 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 3/12] Move printing release number into main() Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/release.py b/rpmautospec/release.py index 1633c1b..77aee01 100644 --- a/rpmautospec/release.py +++ b/rpmautospec/release.py @@ -69,10 +69,10 @@ def calculate_release(srcdir: Union[str, Path]) -> int: release = releaseCount - _log.info("calculate_release release: %s", release) return release def main(args): """Main method.""" - calculate_release(srcdir=args.srcdir) + release = calculate_release(srcdir=args.srcdir) + _log.info("calculate_release release: %s", release) From 55b6befd5da20bd79995912582d00850fdca0af6 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 4/12] Add history processor class ... and move release number calculation into it. Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py new file mode 100644 index 0000000..35c1c93 --- /dev/null +++ b/rpmautospec/pkg_history.py @@ -0,0 +1,79 @@ +import logging +import re +import shutil +from pathlib import Path +from subprocess import CalledProcessError +from tempfile import TemporaryDirectory +from typing import Union + +from .misc import checkout_git_commit, get_rpm_current_version, query_current_git_commit_hash + + +log = logging.getLogger(__name__) + + +class PkgHistoryProcessor: + + pathspec_unknown_re = re.compile( + r"error: pathspec '[^']+' did not match any file\(s\) known to git" + ) + + def __init__(self, spec_or_path: Union[str, Path]): + if isinstance(spec_or_path, str): + spec_or_path = Path(spec_or_path) + + spec_or_path = spec_or_path.absolute() + + if not spec_or_path.exists(): + raise RuntimeError(f"Spec file or path '{spec_or_path}' doesn't exist.") + elif spec_or_path.is_dir(): + self.path = spec_or_path + self.name = spec_or_path.name + self.specfile = spec_or_path / f"{self.name}.spec" + elif spec_or_path.is_file(): + if spec_or_path.suffix != ".spec": + raise ValueError( + "File specified as `spec_or_path` must have '.spec' as an extension." + ) + self.path = spec_or_path.parent + self.name = spec_or_path.stem + self.specfile = spec_or_path + else: + raise RuntimeError("File specified as `spec_or_path` is not a regular file.") + + if not self.specfile.exists(): + raise RuntimeError(f"Spec file '{self.specfile}' doesn't exist in '{self.path}'.") + + def calculate_release_number(self) -> int: + # Count the number of commits between version changes to create the release + releaseCount = 0 + + with TemporaryDirectory(prefix="rpmautospec-") as workdir: + repocopy = f"{workdir}/{self.name}" + shutil.copytree(self.path, repocopy) + + # capture the hash of the current commit version + head = query_current_git_commit_hash(repocopy) + log.info("calculate_release head: %s", head) + + latest_version = current_version = get_rpm_current_version(repocopy, with_epoch=True) + + # in loop/recursively: + while latest_version == current_version: + try: + releaseCount += 1 + # while it's the same, go back a commit + commit = checkout_git_commit(repocopy, head + "~" + str(releaseCount)) + log.info("Checking commit %s ...", commit) + current_version = get_rpm_current_version(repocopy, with_epoch=True) + log.info("... -> %s", current_version) + except CalledProcessError as e: + stderr = e.stderr.decode("UTF-8", errors="replace").strip() + match = self.pathspec_unknown_re.match(stderr) + if match: + break + + release = releaseCount + + log.info("calculate_release release: %s", release) + return release diff --git a/rpmautospec/release.py b/rpmautospec/release.py index 77aee01..533ef78 100644 --- a/rpmautospec/release.py +++ b/rpmautospec/release.py @@ -1,25 +1,13 @@ #!/usr/bin/python3 import logging -import re -import shutil -import tempfile from pathlib import Path -from subprocess import CalledProcessError from typing import Union -from .misc import ( - get_rpm_current_version, - query_current_git_commit_hash, - checkout_git_commit, -) +from .pkg_history import PkgHistoryProcessor _log = logging.getLogger(__name__) -pathspec_unknown_re = re.compile( - r"error: pathspec '[^']+' did not match any file\(s\) known to git" -) - def register_subcommand(subparsers): subcmd_name = "calculate-release" @@ -37,39 +25,9 @@ def register_subcommand(subparsers): def calculate_release(srcdir: Union[str, Path]) -> int: - # Count the number of commits between version changes to create the release - releaseCount = 0 - - srcdir = Path(srcdir) - - with tempfile.TemporaryDirectory(prefix="rpmautospec-") as workdir: - repocopy = f"{workdir}/{srcdir.name}" - shutil.copytree(srcdir, repocopy) - - # capture the hash of the current commit version - head = query_current_git_commit_hash(repocopy) - _log.info("calculate_release head: %s", head) - - latest_version = current_version = get_rpm_current_version(repocopy, with_epoch=True) - - # in loop/recursively: - while latest_version == current_version: - try: - releaseCount += 1 - # while it's the same, go back a commit - commit = checkout_git_commit(repocopy, head + "~" + str(releaseCount)) - _log.info("Checking commit %s ...", commit) - current_version = get_rpm_current_version(repocopy, with_epoch=True) - _log.info("... -> %s", current_version) - except CalledProcessError as e: - stderr = e.stderr.decode("UTF-8", errors="replace").strip() - match = pathspec_unknown_re.match(stderr) - if match: - break - - release = releaseCount - - return release + processor = PkgHistoryProcessor(srcdir) + release_number = processor.calculate_release_number() + return release_number def main(args): From bd7416c4f47033ac1a566af11c4c65986fe124cd Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 5/12] Use pygit2 instead of executing git Fixes: #118 Signed-off-by: Nils Philippsen --- diff --git a/README.rst b/README.rst index d986694..5252798 100644 --- a/README.rst +++ b/README.rst @@ -10,6 +10,7 @@ This project hosts the ``rpmautospec`` python package and script, which has thes Dependencies: * python3 +* python3-pygit2 General ------- diff --git a/ci/pytest.yaml b/ci/pytest.yaml index 0f2262e..90d0833 100644 --- a/ci/pytest.yaml +++ b/ci/pytest.yaml @@ -8,6 +8,7 @@ package: name: - python3-koji + - python3-pygit2 - python3-pytest - python3-pytest-cov - python3-rpm diff --git a/python-rpmautospec.spec b/python-rpmautospec.spec index b6ba0fe..6337644 100644 --- a/python-rpmautospec.spec +++ b/python-rpmautospec.spec @@ -31,6 +31,7 @@ BuildRequires: python2-devel %if ! %{with epel_le_7} BuildRequires: koji BuildRequires: python3-koji +BuildRequires: python3-pygit2 BuildRequires: python%{python3_pkgversion}-pytest BuildRequires: python%{python3_pkgversion}-pytest-cov BuildRequires: git @@ -54,6 +55,7 @@ Requires: koji Requires: git-core Requires: python3-rpm Requires: python3-koji +Requires: python3-pygit2 %description -n python3-%{srcname} %_description @@ -148,6 +150,9 @@ install -m 644 rpm/macros.d/macros.rpmautospec %{buildroot}%{rpmmacrodir}/ %endif %changelog +* Wed May 12 2021 Nils Philippsen +- depend on python3-pygit2 + * Thu Apr 22 2021 Nils Philippsen - remove the hub plugin diff --git a/rpmautospec/misc.py b/rpmautospec/misc.py index 7ecfef4..2ec7bad 100644 --- a/rpmautospec/misc.py +++ b/rpmautospec/misc.py @@ -6,7 +6,6 @@ from pathlib import Path from typing import Optional from typing import Tuple from typing import Union -import typing import koji import rpm @@ -94,50 +93,6 @@ def koji_init(koji_url_or_session: Union[str, koji.ClientSession]) -> koji.Clien return _kojiclient -def query_current_git_commit_hash( - path: str, - log_options: typing.Optional[typing.List[str]] = None, -): - """Retrieves the git commit hash in ``path`` . - - This method runs `git log -1 --format="%H"` at ``path`` - - This command returns a commit hash number like the following: - 1e86efac2723289c896165bae2e863cb66466376 - ... - """ - _log.debug("query_current_git_commit_hash(): %s", path) - - cmd = ["git", "log", "-1", "--format=%H"] - if log_options: - cmd.extend(log_options) - - _log.debug("query_current_git_commit_hash(): %s", cmd) - return run_command(cmd, cwd=path).decode("UTF-8").strip() - - -def checkout_git_commit( - path: str, - commit: str, - log_options: typing.Optional[typing.List[str]] = None, -) -> typing.List[str]: - """Checks out the git commit in ``path`` specified in ``commit``. - - This method runs the system's `xxxx` command. - ... - """ - _log.debug("checkout_git_commit(): %s", path) - _log.debug("checkout_git_commit(): %s", commit) - - cmd = ["git", "checkout", commit] - if log_options: - cmd.extend(log_options) - - _log.debug("checkout_git_commit(): %s", cmd) - subprocess.check_output(cmd, cwd=path, stderr=subprocess.PIPE) - return query_current_git_commit_hash(path) - - def run_command(command: list, cwd: Optional[str] = None) -> bytes: """Run the specified command in a specific working directory if one is specified. diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py index 35c1c93..4ca9d66 100644 --- a/rpmautospec/pkg_history.py +++ b/rpmautospec/pkg_history.py @@ -1,23 +1,17 @@ import logging -import re -import shutil from pathlib import Path -from subprocess import CalledProcessError from tempfile import TemporaryDirectory -from typing import Union +from typing import Optional, Union -from .misc import checkout_git_commit, get_rpm_current_version, query_current_git_commit_hash +import pygit2 + +from .misc import get_rpm_current_version log = logging.getLogger(__name__) class PkgHistoryProcessor: - - pathspec_unknown_re = re.compile( - r"error: pathspec '[^']+' did not match any file\(s\) known to git" - ) - def __init__(self, spec_or_path: Union[str, Path]): if isinstance(spec_or_path, str): spec_or_path = Path(spec_or_path) @@ -44,36 +38,57 @@ class PkgHistoryProcessor: if not self.specfile.exists(): raise RuntimeError(f"Spec file '{self.specfile}' doesn't exist in '{self.path}'.") - def calculate_release_number(self) -> int: - # Count the number of commits between version changes to create the release - releaseCount = 0 - + try: + if hasattr(pygit2, "GIT_REPOSITORY_OPEN_NO_SEARCH"): + kwargs = {"flags": pygit2.GIT_REPOSITORY_OPEN_NO_SEARCH} + else: + # pygit2 < 1.4.0 + kwargs = {} + # pygit2 < 1.2.0 can't cope with pathlib.Path objects + self.repo = pygit2.Repository(str(self.path), **kwargs) + except pygit2.GitError: + self.repo = None + + def _get_rpm_version_for_commit(self, commit): with TemporaryDirectory(prefix="rpmautospec-") as workdir: - repocopy = f"{workdir}/{self.name}" - shutil.copytree(self.path, repocopy) - - # capture the hash of the current commit version - head = query_current_git_commit_hash(repocopy) - log.info("calculate_release head: %s", head) - - latest_version = current_version = get_rpm_current_version(repocopy, with_epoch=True) - - # in loop/recursively: - while latest_version == current_version: - try: - releaseCount += 1 - # while it's the same, go back a commit - commit = checkout_git_commit(repocopy, head + "~" + str(releaseCount)) - log.info("Checking commit %s ...", commit) - current_version = get_rpm_current_version(repocopy, with_epoch=True) - log.info("... -> %s", current_version) - except CalledProcessError as e: - stderr = e.stderr.decode("UTF-8", errors="replace").strip() - match = self.pathspec_unknown_re.match(stderr) - if match: - break - - release = releaseCount - - log.info("calculate_release release: %s", release) + try: + specblob = commit.tree[self.specfile.name] + except KeyError: + # no spec file + return None + + specpath = Path(workdir) / self.specfile.name + with specpath.open("wb") as specfile: + specfile.write(specblob.data) + + return get_rpm_current_version(workdir, self.name, with_epoch=True) + + def calculate_release_number(self, commit: Optional[pygit2.Commit] = None) -> Optional[int]: + if not self.repo: + # no git repo -> no history + return 1 + + if not commit: + commit = self.repo[self.repo.head.target] + + version = get_rpm_current_version(str(self.path), with_epoch=True) + + release = 1 + + while True: + log.info(f"checking commit {commit.hex}, version {version} - release {release}") + if not commit.parents: + break + assert len(commit.parents) == 1 + + parent = commit.parents[0] + parent_version = self._get_rpm_version_for_commit(parent) + log.info(f" comparing against parent commit {parent.hex}, version {parent_version}") + + if parent_version != version: + break + + release += 1 + commit = parent + return release diff --git a/tests/rpmautospec/test_pkg_history.py b/tests/rpmautospec/test_pkg_history.py new file mode 100644 index 0000000..2381881 --- /dev/null +++ b/tests/rpmautospec/test_pkg_history.py @@ -0,0 +1,249 @@ +import logging +import os +import re +import stat +from pathlib import Path +from shutil import rmtree +from unittest.mock import patch +from tempfile import TemporaryDirectory + +import pygit2 +import pytest + +from rpmautospec.pkg_history import PkgHistoryProcessor + + +SPEC_FILE_TEXT = """Summary: Boo +Name: boo +Version: 1.0 +Release: %autorel +License: CC0 + +%description +Boo + +%changelog +%autochangelog +""" + + +@pytest.fixture +def specfile(): + with TemporaryDirectory() as tmpdir: + tmpdir = Path(tmpdir) + repodir = tmpdir / "test" + repodir.mkdir() + specfile = repodir / "test.spec" + specfile.write_text(SPEC_FILE_TEXT) + + yield specfile + + +@pytest.fixture +def repo(specfile): + # pygit2 < 1.2.0 can't cope with pathlib.Path objects + repopath = str(specfile.parent) + + pygit2.init_repository(repopath, initial_head="rawhide") + if hasattr(pygit2, "GIT_REPOSITORY_OPEN_NO_SEARCH"): + repo = pygit2.Repository(repopath, pygit2.GIT_REPOSITORY_OPEN_NO_SEARCH) + else: + # pygit2 < 1.4.0 + repo = pygit2.Repository(repopath) + + repo.config["user.name"] = "Jane Doe" + repo.config["user.email"] = "jane.doe@example.com" + + # create root commit in "rawhide" branch + index = repo.index + index.add(specfile.name) + index.write() + + tree = index.write_tree() + + oid = repo.create_commit( + None, repo.default_signature, repo.default_signature, "Initial commit", tree, [] + ) + repo.branches.local.create("rawhide", repo[oid]) + + # add another commit (empty) + parent, ref = repo.resolve_refish(repo.head.name) + repo.create_commit( + ref.name, + repo.default_signature, + repo.default_signature, + "Did nothing!", + tree, + [parent.oid], + ) + + yield repo + + +@pytest.fixture +def processor(repo): + processor = PkgHistoryProcessor(repo.workdir) + return processor + + +class TestPkgHistoryProcessor: + + version_re = re.compile(r"^Version: .*$", flags=re.MULTILINE) + + @pytest.mark.parametrize( + "testcase", + ( + "str, is file", + "str, is dir", + "path, is file", + "path, is file, wrong extension", + "path, is dir", + "doesn't exist", + "spec doesn't exist, is dir", + "no git repo", + "not a regular file", + ), + ) + @patch("rpmautospec.pkg_history.pygit2") + def test___init__(self, pygit2, testcase, specfile): + if "wrong extension" in testcase: + # Path.rename() only returns the new path from Python 3.8 on. + specfile.rename(specfile.with_suffix(".foo")) + specfile = specfile.with_suffix(".foo") + + spec_or_path = specfile + + if "is dir" in testcase: + spec_or_path = spec_or_path.parent + + if "spec doesn't exist" in testcase: + specfile.unlink() + elif "doesn't exist" in testcase: + rmtree(specfile.parent) + + if "str" in testcase: + spec_or_path = str(spec_or_path) + + if "doesn't exist" in testcase: + with pytest.raises(RuntimeError) as excinfo: + PkgHistoryProcessor(spec_or_path) + if "spec doesn't exist" in testcase: + expected_message = f"Spec file '{specfile}' doesn't exist in '{specfile.parent}'." + else: + expected_message = f"Spec file or path '{spec_or_path}' doesn't exist." + assert str(excinfo.value) == expected_message + return + + if "not a regular file" in testcase: + specfile.unlink() + os.mknod(specfile, stat.S_IFIFO | stat.S_IRUSR | stat.S_IWUSR) + with pytest.raises(RuntimeError) as excinfo: + PkgHistoryProcessor(spec_or_path) + assert str(excinfo.value) == "File specified as `spec_or_path` is not a regular file." + return + + if "wrong extension" in testcase: + with pytest.raises(ValueError) as excinfo: + PkgHistoryProcessor(spec_or_path) + assert str(excinfo.value) == ( + "File specified as `spec_or_path` must have '.spec' as an extension." + ) + return + + if "no git repo" in testcase: + + class GitError(Exception): + pass + + pygit2.GitError = GitError + pygit2.Repository.side_effect = GitError + + processor = PkgHistoryProcessor(spec_or_path) + + assert processor.specfile == specfile + assert processor.path == specfile.parent + + pygit2.Repository.assert_called_once() + + if "no git repo" in testcase: + assert processor.repo is None + else: + assert processor.repo + + @pytest.mark.parametrize("testcase", ("normal", "no spec file")) + def test__get_rpm_version_for_commit(self, testcase, specfile, repo, processor): + head_commit = repo[repo.head.target] + + if testcase == "no spec file": + index = repo.index + index.remove(specfile.name) + index.write() + + tree = index.write_tree() + + parent, ref = repo.resolve_refish(repo.head.name) + + head_commit = repo[ + repo.create_commit( + ref.name, + repo.default_signature, + repo.default_signature, + "Be gone, spec file!", + tree, + [parent.oid], + ) + ] + + assert processor._get_rpm_version_for_commit(head_commit) is None + else: + assert processor._get_rpm_version_for_commit(head_commit) == "1.0" + + @pytest.mark.parametrize( + "testcase", + ( + "normal", + "no git repo", + "no commit specified", + "root commit", + "bump version", + ), + ) + def test_calculate_release_number(self, testcase, specfile, repo, processor, caplog): + commit = repo[repo.head.target] + if testcase not in ("no git repo", "root commit", "bump version"): + expected_release_number = 2 + else: + expected_release_number = 1 + + if testcase == "no git repo": + rmtree(specfile.parent / ".git") + processor = PkgHistoryProcessor(specfile) + elif testcase == "no commit specified": + commit = None + elif testcase == "root commit": + commit = commit.parents[0] + elif testcase == "bump version": + with specfile.open("r") as sf: + contents = sf.read() + + with specfile.open("w") as sf: + sf.write(self.version_re.sub("Version: 2.0", contents)) + + index = repo.index + index.add(specfile.name) + index.write() + + tree = index.write_tree() + + parent, ref = repo.resolve_refish(repo.head.name) + repo.create_commit( + ref.name, + repo.default_signature, + repo.default_signature, + "Update to 2.0", + tree, + [parent.oid], + ) + + with caplog.at_level(logging.DEBUG): + assert processor.calculate_release_number(commit) == expected_release_number From a092ca27ead28fd21661d6256cc23e4965a24977 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 6/12] Determine spec file version silently Don't log errors when a spec file can't be parsed, there could be any number of unparseable versions of the spec file in a repo and we don't want to clog up the logs. Just flag it as `None` and let callers work around that this is missing. Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/misc.py b/rpmautospec/misc.py index 2ec7bad..1a32d9c 100644 --- a/rpmautospec/misc.py +++ b/rpmautospec/misc.py @@ -43,9 +43,13 @@ def parse_evr(evr_str: str) -> Tuple[int, str, Optional[str]]: return epoch, match.group("version"), match.group("release") -def get_rpm_current_version(path: str, name: Optional[str] = None, with_epoch: bool = False) -> str: +def get_rpm_current_version( + path: str, name: Optional[str] = None, with_epoch: bool = False +) -> Optional[str]: """Retrieve the current version set in the spec file named ``name``.spec at the given path. + + Returns None if an error is encountered. """ path = Path(path) @@ -76,11 +80,16 @@ def get_rpm_current_version(path: str, name: Optional[str] = None, with_epoch: b f"{name}.spec", ] - output = None try: - output = run_command(rpm_cmd, cwd=path).decode("UTF-8").split("\n")[0].strip() + output = ( + subprocess.check_output(rpm_cmd, cwd=path, stderr=subprocess.PIPE) + .decode("UTF-8") + .split("\n")[0] + .strip() + ) except Exception: - pass + return None + return output From 4b5dfec5514d68727355d56b619adb640b257f5f Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 7/12] Cache determined spec file versions for commits Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py index 4ca9d66..384a744 100644 --- a/rpmautospec/pkg_history.py +++ b/rpmautospec/pkg_history.py @@ -1,4 +1,5 @@ import logging +from functools import lru_cache from pathlib import Path from tempfile import TemporaryDirectory from typing import Optional, Union @@ -49,6 +50,7 @@ class PkgHistoryProcessor: except pygit2.GitError: self.repo = None + @lru_cache(maxsize=None) def _get_rpm_version_for_commit(self, commit): with TemporaryDirectory(prefix="rpmautospec-") as workdir: try: From bc28b9676e9b88835b74bf93215937d290e12758 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 8/12] Implement generic git history processor PkgHistoryProcessor.run() accepts a list of functions generating coroutines which "visit" every commit, i.e. check if they need information from results of earlier commits, and process these and partial results from previously run visitor coroutines. These coroutines are implemented as generators which yield whether or not to continue first, get sent the partial result for the current commit and results for all parents back in, process these and yield the processed current result like this: def commit_visitor(commit: pygit2.Commit, children_must_continue: bool): # Decide if history needs to be explored further, this can depend on # information in this or parent commits, as well as the combined # outcome for child commits. must_continue = ... # This suspends execution, yields the decision to the caller who # sends back (partial) results for this commit. commit_result, parent_results = yield must_continue # Process this commit, also using its (partial) results (e.g. by # previously run visitors) and full results for the parent(s). ... yield commit_result Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py index 384a744..8b7bbb3 100644 --- a/rpmautospec/pkg_history.py +++ b/rpmautospec/pkg_history.py @@ -1,8 +1,9 @@ import logging -from functools import lru_cache +from collections import defaultdict +from functools import lru_cache, reduce from pathlib import Path from tempfile import TemporaryDirectory -from typing import Optional, Union +from typing import Any, Dict, Optional, Sequence, Union import pygit2 @@ -65,6 +66,143 @@ class PkgHistoryProcessor: return get_rpm_current_version(workdir, self.name, with_epoch=True) + def run( + self, + head: Optional[Union[str, pygit2.Commit]] = None, + *, + visitors: Sequence = (), + all_results: bool = False, + ) -> Union[Dict[str, Any], Dict[pygit2.Commit, Dict[str, Any]]]: + if not head: + head = self.repo[self.repo.head.target] + + # maps visited commits to their (in-flight) visitors and if they must + # continue + commit_coroutines = {} + commit_coroutines_must_continue = {} + + # keep track of branches + branch_heads = [head] + branches = [] + + ######################################################################################## + # Unfortunately, pygit2 only tells us what the parents of a commit are, not what other + # commits a commit is parent to (its children). Fortunately, Repository.walk() is quick. + ######################################################################################## + commit_children = defaultdict(list) + for commit in self.repo.walk(head.id): + for parent in commit.parents: + commit_children[parent].append(commit) + + ########################################################################################## + # To process, first walk the tree from the head commit downward, following all branches. + # Check visitors whether they need parent results to do their work, i.e. the history needs + # to be followed further. + ########################################################################################## + + # While new branch heads are encountered... + while branch_heads: + commit = branch_heads.pop(0) + branch = [] + branches.append(branch) + + while True: + if commit in commit_coroutines: + break + + log.debug("%s: %s", commit.short_id, commit.message.split("\n")[0]) + + if commit == head: + children_visitors_must_continue = [True for v in visitors] + else: + this_children = commit_children[commit] + if not all(child in commit_coroutines for child in this_children): + # there's another branch that leads to this parent, put the remainder on the + # stack + branch_heads.append(commit) + if not branch: + # don't keep empty branches on the stack + branches.pop() + break + + # For all visitor coroutines, determine if any of the children must continue. + children_visitors_must_continue = [ + reduce( + lambda must_continue, child: ( + must_continue or commit_coroutines_must_continue[child][vindex] + ), + this_children, + False, + ) + for vindex, v in enumerate(visitors) + ] + + branch.append(commit) + + # Create visitor coroutines for the commit from the functions passed into this + # method. Pass the ordered list of "is there a child whose coroutine of the same + # visitor wants to continue" into it. + commit_coroutines[commit] = coroutines = [ + v(commit, children_visitors_must_continue[vi]) for vi, v in enumerate(visitors) + ] + + # Consult all visitors for the commit on whether we should continue and store the + # results. + commit_coroutines_must_continue[commit] = coroutines_must_continue = [ + next(c) for c in coroutines + ] + + if not any(coroutines_must_continue) or not commit.parents: + break + + if len(commit.parents) > 1: + # merge commit, store new branch head(s) to follow later + branch_parents = commit.parents[1:] + new_branches = [p for p in branch_parents if p not in commit_coroutines] + branch_heads.extend(new_branches) + + # follow (first) parent + commit = commit.parents[0] + + ########################################################################################### + # Now, `branches` contains disjunct lists of commits in new -> old order. Process these in + # reverse, one at a time until encountering a commit where we don't know the results of all + # parents. Then put the remainder back on the stack to be further processed later until we + # run out of branches with commits. + ########################################################################################### + + visited_results = {} + while branches: + branch = branches.pop(0) + while branch: + # Take one commit from the tail end of the branch and process. + commit = branch.pop() + + if not all( + p in visited_results or p not in commit_coroutines for p in commit.parents + ): + # put the unprocessed commit back + branch.append(commit) + # put the unprocessed remainder back + branches.append(branch) + + break + + parent_results = [visited_results.get(p, {}) for p in commit.parents] + + # "Pipe" the (partial) result dictionaries through the second half of all visitors + # for the commit. + visited_results[commit] = reduce( + lambda commit_result, visitor: visitor.send((commit_result, parent_results)), + commit_coroutines[commit], + {"commit-id": commit.id}, + ) + + if all_results: + return visited_results + else: + return visited_results[head] + def calculate_release_number(self, commit: Optional[pygit2.Commit] = None) -> Optional[int]: if not self.repo: # no git repo -> no history diff --git a/tests/rpmautospec/test_pkg_history.py b/tests/rpmautospec/test_pkg_history.py index 2381881..639370a 100644 --- a/tests/rpmautospec/test_pkg_history.py +++ b/tests/rpmautospec/test_pkg_history.py @@ -198,6 +198,33 @@ class TestPkgHistoryProcessor: else: assert processor._get_rpm_version_for_commit(head_commit) == "1.0" + @pytest.mark.parametrize("testcase", ("without commit", "with commit", "all results")) + def test_run(self, testcase, repo, processor): + def noop_visitor(commit, children_must_continue): + current_result, parent_results = yield len(commit.parents) > 0 + yield current_result + + all_results = "all results" in testcase + + head_commit = repo[repo.head.target] + + if testcase == "with commit": + args = [head_commit] + else: + args = [] + + res = processor.run(*args, visitors=[noop_visitor], all_results=all_results) + + assert isinstance(res, dict) + if all_results: + assert all(isinstance(key, pygit2.Commit) for key in res) + # only verify outcome for head commit below + res = res[head_commit] + else: + assert all(isinstance(key, str) for key in res) + + assert res["commit-id"] == head_commit.id + @pytest.mark.parametrize( "testcase", ( From 0902999db4149547945103efb2f6f063dde654e4 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 9/12] Add visitor, use processor for release numbers Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py index 8b7bbb3..967738b 100644 --- a/rpmautospec/pkg_history.py +++ b/rpmautospec/pkg_history.py @@ -66,6 +66,43 @@ class PkgHistoryProcessor: return get_rpm_current_version(workdir, self.name, with_epoch=True) + def release_number_visitor(self, commit: pygit2.Commit, children_must_continue: bool): + """Visit a commit to determine its release number. + + The coroutine returned first determines if the parent chain(s) must be + followed, i.e. if one parent has the same package epoch-version, + suspends execution and yields that to the caller (usually the walk() + method), who later sends the partial results for this commit (to be + modified) and full results of parents back (as dictionaries), resuming + execution to process these and finally yield back the results for this + commit. + """ + epoch_version = self._get_rpm_version_for_commit(commit) + + must_continue = not epoch_version or epoch_version in ( + self._get_rpm_version_for_commit(p) for p in commit.parents + ) + + # Suspend execution, yield whether caller should continue, and get back the (partial) result + # for this commit and parent results as dictionaries on resume. + commit_result, parent_results = yield must_continue + + commit_result["epoch-version"] = epoch_version + + # Find the maximum applicable parent release number and increment by one. + commit_result["release-number"] = ( + max( + ( + res["release-number"] if res and epoch_version == res["epoch-version"] else 0 + for res in parent_results + ), + default=0, + ) + + 1 + ) + + yield commit_result + def run( self, head: Optional[Union[str, pygit2.Commit]] = None, @@ -202,33 +239,3 @@ class PkgHistoryProcessor: return visited_results else: return visited_results[head] - - def calculate_release_number(self, commit: Optional[pygit2.Commit] = None) -> Optional[int]: - if not self.repo: - # no git repo -> no history - return 1 - - if not commit: - commit = self.repo[self.repo.head.target] - - version = get_rpm_current_version(str(self.path), with_epoch=True) - - release = 1 - - while True: - log.info(f"checking commit {commit.hex}, version {version} - release {release}") - if not commit.parents: - break - assert len(commit.parents) == 1 - - parent = commit.parents[0] - parent_version = self._get_rpm_version_for_commit(parent) - log.info(f" comparing against parent commit {parent.hex}, version {parent_version}") - - if parent_version != version: - break - - release += 1 - commit = parent - - return release diff --git a/rpmautospec/release.py b/rpmautospec/release.py index 533ef78..2075477 100644 --- a/rpmautospec/release.py +++ b/rpmautospec/release.py @@ -1,4 +1,3 @@ -#!/usr/bin/python3 import logging from pathlib import Path from typing import Union @@ -26,8 +25,8 @@ def register_subcommand(subparsers): def calculate_release(srcdir: Union[str, Path]) -> int: processor = PkgHistoryProcessor(srcdir) - release_number = processor.calculate_release_number() - return release_number + result = processor.run(visitors=(processor.release_number_visitor,)) + return result["release-number"] def main(args): diff --git a/tests/rpmautospec/test_pkg_history.py b/tests/rpmautospec/test_pkg_history.py index 639370a..0f067dc 100644 --- a/tests/rpmautospec/test_pkg_history.py +++ b/tests/rpmautospec/test_pkg_history.py @@ -1,4 +1,3 @@ -import logging import os import re import stat @@ -200,10 +199,6 @@ class TestPkgHistoryProcessor: @pytest.mark.parametrize("testcase", ("without commit", "with commit", "all results")) def test_run(self, testcase, repo, processor): - def noop_visitor(commit, children_must_continue): - current_result, parent_results = yield len(commit.parents) > 0 - yield current_result - all_results = "all results" in testcase head_commit = repo[repo.head.target] @@ -213,7 +208,11 @@ class TestPkgHistoryProcessor: else: args = [] - res = processor.run(*args, visitors=[noop_visitor], all_results=all_results) + res = processor.run( + *args, + visitors=[processor.release_number_visitor], + all_results=all_results, + ) assert isinstance(res, dict) if all_results: @@ -224,53 +223,4 @@ class TestPkgHistoryProcessor: assert all(isinstance(key, str) for key in res) assert res["commit-id"] == head_commit.id - - @pytest.mark.parametrize( - "testcase", - ( - "normal", - "no git repo", - "no commit specified", - "root commit", - "bump version", - ), - ) - def test_calculate_release_number(self, testcase, specfile, repo, processor, caplog): - commit = repo[repo.head.target] - if testcase not in ("no git repo", "root commit", "bump version"): - expected_release_number = 2 - else: - expected_release_number = 1 - - if testcase == "no git repo": - rmtree(specfile.parent / ".git") - processor = PkgHistoryProcessor(specfile) - elif testcase == "no commit specified": - commit = None - elif testcase == "root commit": - commit = commit.parents[0] - elif testcase == "bump version": - with specfile.open("r") as sf: - contents = sf.read() - - with specfile.open("w") as sf: - sf.write(self.version_re.sub("Version: 2.0", contents)) - - index = repo.index - index.add(specfile.name) - index.write() - - tree = index.write_tree() - - parent, ref = repo.resolve_refish(repo.head.name) - repo.create_commit( - ref.name, - repo.default_signature, - repo.default_signature, - "Update to 2.0", - tree, - [parent.oid], - ) - - with caplog.at_level(logging.DEBUG): - assert processor.calculate_release_number(commit) == expected_release_number + assert res["release-number"] == 2 From 72b034412effdd4c9fabd3c1b0c3170837490237 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:15:45 +0000 Subject: [PATCH 10/12] Add visitor, use processor for changelog Fixes: #116, #117, #142, #150 Signed-off-by: Nils Philippsen --- diff --git a/python-rpmautospec.spec b/python-rpmautospec.spec index 6337644..67ff261 100644 --- a/python-rpmautospec.spec +++ b/python-rpmautospec.spec @@ -52,10 +52,11 @@ Summary: %{summary} %{?python_provide:%python_provide python3-%{srcname}} Requires: koji -Requires: git-core -Requires: python3-rpm Requires: python3-koji Requires: python3-pygit2 +Requires: rpm +# for "rpm --specfile" +Requires: rpm-build >= 4.9 %description -n python3-%{srcname} %_description @@ -71,10 +72,6 @@ Requires: python3-pygit2 %package -n %{srcname} Summary: CLI tool for generating RPM releases and changelogs Requires: python3-%{srcname} = %{version}-%{release} -# We add this require here and not in python3-rpmautospec because we do not want -# it on the builders, the builders plugins will work fine without it but we -# need this in the chroot or when packagers run the CLI on their machines. -Requires: rpm-build >= 4.9 %description -n %{srcname} CLI tool for generating RPM releases and changelogs @@ -150,6 +147,9 @@ install -m 644 rpm/macros.d/macros.rpmautospec %{buildroot}%{rpmmacrodir}/ %endif %changelog +* Wed May 19 2021 Nils Philippsen +- remove git-core, fix RPM related dependencies + * Wed May 12 2021 Nils Philippsen - depend on python3-pygit2 diff --git a/rpmautospec/changelog.py b/rpmautospec/changelog.py index 481ee82..5f4bac3 100644 --- a/rpmautospec/changelog.py +++ b/rpmautospec/changelog.py @@ -1,17 +1,10 @@ -#!/usr/bin/python3 -import datetime import logging -import os -import re -import shutil -import tempfile -import textwrap -import typing +from typing import Any, Dict, Optional, Union -from .misc import get_rpm_current_version, parse_evr, rpmvercmp_key, run_command +from .pkg_history import PkgHistoryProcessor -_log = logging.getLogger(__name__) +log = logging.getLogger(__name__) def register_subcommand(subparsers): @@ -27,229 +20,36 @@ def register_subcommand(subparsers): return subcmd_name -def git_get_log( - path: str, - log_options: typing.Optional[typing.List[str]] = None, - toref: typing.Optional[str] = None, - target: typing.Optional[str] = None, -) -> typing.List[str]: - """Returns the list of the commit logs for the repo in ``path`` . - - This method runs the system's `git log --pretty=oneline --abbrev-commit` - command. - - This command returns git log as follow: - - - - ... - - :kwarg log_options: options to pass to git log - :kwarg toref: a reference/commit to use when generating the log - :kwarg target: the target of the git log command, can be a ref, a - file or nothing - - """ - cmd = ["git", "log", "--pretty=oneline", "--abbrev-commit", "--no-decorate"] - if log_options: - cmd.extend(log_options) - if toref: - cmd.append(f"{toref}..") - if target: - cmd.extend(["--", target]) - - _log.debug("git_get_log(): %s", cmd) - return run_command(cmd, cwd=path).decode("UTF-8").strip().split("\n") - - -def git_get_commit_info(path: str, commithash: str) -> typing.List[str]: - """This function calls `git show --no-patch --format="%P %ct"` on the - specified commit and returns the output from git - """ - cmd = ["git", "show", "--no-patch", "--format=%P|%H|%ct|%aN <%aE>|%s", commithash] - _log.debug("git_get_commit_info: %s", cmd) - return run_command(cmd, cwd=path).decode("UTF-8").strip().split("\n") - - -def git_get_changed_files(path: str, commithash: str) -> typing.List[str]: - """Returns the list of files changed in the specified commit.""" - cmd = ["git", "diff-tree", "--no-commit-id", "--name-only", "-r", commithash] - _log.debug("git_get_changed_files: %s", cmd) - return run_command(cmd, cwd=path).decode("UTF-8").strip().split("\n") - - -def nevrd_to_evr(nevrd: str) -> str: - """Converts a name:epoch-version-release.dist_tag to epoch_version_release - so it can be inserted in the changelog. - - If the nevrd provided does not have at least 2 "-" in it, otherwise - it will be just be cleaned for any potential dist_tag. - """ - if nevrd.count("-") >= 2: - version, release = nevrd.rsplit("-", 2)[1:] - # Append a "-" to the version to make it easier to concatenate later - version += "-" +def _coerce_to_str(str_or_bytes: Union[str, bytes]) -> str: + if isinstance(str_or_bytes, bytes): + str_or_bytes = str_or_bytes.decode("utf-8", errors="replace") + return str_or_bytes + + +def _coerce_to_bytes(str_or_bytes: Union[str, bytes]) -> str: + if isinstance(str_or_bytes, str): + str_or_bytes = str_or_bytes.encode("utf-8") + return str_or_bytes + + +def collate_changelog( + processor_results: Dict[str, Any], result_type: Optional[type] = str +) -> Union[str, bytes]: + changelog = processor_results["changelog"] + if result_type == str: + entry_strings = (_coerce_to_str(entry["data"]) for entry in changelog) else: - version = "" - release = nevrd - release = re.sub(r"\.fc\d+", "", release) - release = re.sub(r"\.el\d+", "", release) - return f"{version}{release}" - - -def produce_changelog(repopath, latest_rel=None): - name = os.path.basename(repopath) - with tempfile.TemporaryDirectory(prefix="rpmautospec-") as workdir: - repocopy = f"{workdir}/{name}" - shutil.copytree(repopath, repocopy) - _log.debug("Working directory: %s", repocopy) - lines = [] - - # FIXME: We don't do tags anymore - tags = [] - - # Get the latest commit in the repo - head = git_get_log(repocopy, log_options=["-1"])[0] - head_hash = head.split(" ", 1)[0] - head_info = git_get_commit_info(repocopy, head_hash)[0] - head_commit_dt = datetime.datetime.utcfromtimestamp(int(head_info.split("|", 3)[2])) - - # Get the current version and build the version-release to be used - # for the latest entry in the changelog, if we can build it - current_evr = None - current_version = get_rpm_current_version(repocopy, name) - if current_version and latest_rel: - latest_rel = nevrd_to_evr(latest_rel) - current_evr = f"{current_version}-{latest_rel}" - - stop_commit_hash = None - changelog = [] - changelog_file = os.path.join(repocopy, "changelog") - if os.path.exists(changelog_file): - stop_commit = git_get_log(repocopy, log_options=["-1"], target="changelog") - if stop_commit: - stop_commit_hash = stop_commit[0].split(" ", 1)[0] - with open(changelog_file) as stream: - changelog = [r.rstrip() for r in stream.readlines()] - - output = [] - entry = [] - evr = current_evr or "LATEST" - last_author = None - toref = None - if stop_commit_hash: - toref = f"{stop_commit_hash}^" - for log_line in git_get_log(repocopy, toref=toref): - if not log_line.strip(): - continue - commit = log_line.split(" ", 1)[0] - - info = git_get_commit_info(repocopy, commit) - if len(info) > 1: - # Ignore merge commits - _log.debug("commit %s is a merge commit, skipping", commit) - continue - - _, commithash, commit_ts, author_info, commit_summary = info[0].split("|", 4) - author_info = author_info.replace("%", "%%") - commit_summary = commit_summary.replace("%", "%%") - - # FIXME: new algo - if commithash in tags: - _log.debug("Tags for the commit: %s: %s", commithash, tags[commithash]) - output.append(entry) - entry = [] - # Use the most recent build for EVR - builds = [] - for b in tags[commithash]: - _epo, _ver, _rel = parse_evr(nevrd_to_evr(b)) - builds.append({"epoch": _epo, "version": _ver, "release": _rel}) - _log.debug("Builds to sort: %s", builds) - if len(builds) > 1: - builds.sort(key=rpmvercmp_key, reverse=True) - - build = builds[0] - if build["epoch"]: - evr = f"{build['epoch']}:{build['version']}-{build['release']}" - else: - evr = f"{build['version']}-{build['release']}" - - commit_dt = datetime.datetime.utcfromtimestamp(int(commit_ts)) - if commit_dt < (head_commit_dt - datetime.timedelta(days=730)): - # Ignore all commits older than 2 years - # if there is a `changelog` file in addition to these commits - # they will be cut down anyway when the RPM gets built, so - # the gap between the commits we are gathering here and the - # ones in the `changelog` file can be ignored. - _log.debug("commit %s is too old, breaking iteration", commit) - break - - files_changed = git_get_changed_files(repocopy, commit) - ignore = True - for filename in files_changed: - if filename.endswith( - ( - ".automount", - ".device", - ".mount", - ".patch", - ".path", - ".pc", - ".preset", - ".scope", - ".service", - ".slice", - ".socket", - ".spec", - ".swap", - ".target", - ".timer", - ) - ): - ignore = False - - if not ignore: - if last_author == author_info and entry: - entry[-1]["commits"].append(commit_summary) - else: - entry.append( - { - "commit": commit, - "commit_ts": commit_ts, - "commit_author": author_info, - "commits": [commit_summary], - "evr": evr, - } - ) - last_author = author_info - else: - _log.debug("commit %s is not changing a file of interest, ignoring", commit) - - # Last entries - output.append(entry) - - wrapper = textwrap.TextWrapper(width=75, subsequent_indent=" ") - for entries in output: - for commit in entries: - commit_dt = datetime.datetime.utcfromtimestamp(int(commit["commit_ts"])) - author_info = commit["commit_author"] - evr = commit["evr"] - lines += [ - f"* {commit_dt.strftime('%a %b %d %Y')} {author_info} - {evr}", - ] - for message in reversed(commit["commits"]): - if message.strip(): - lines += ["- %s" % wrapper.fill(message.strip())] - lines += [""] - - # Add the existing changelog if there is one - lines.extend(changelog) - return lines + entry_strings = (_coerce_to_bytes(entry["data"]) for entry in changelog) + return "\n\n".join(entry_strings) + + +def produce_changelog(spec_or_repo): + processor = PkgHistoryProcessor(spec_or_repo) + result = processor.run(visitors=(processor.release_number_visitor, processor.changelog_visitor)) + return collate_changelog(result) def main(args): """Main method.""" - - repopath = args.worktree_path.rstrip(os.path.sep) - changelog = produce_changelog(repopath) - _log.info("\n".join(changelog)) + changelog = produce_changelog(args.worktree_path) + log.info(changelog) diff --git a/rpmautospec/misc.py b/rpmautospec/misc.py index 1a32d9c..485451a 100644 --- a/rpmautospec/misc.py +++ b/rpmautospec/misc.py @@ -1,14 +1,10 @@ -from functools import cmp_to_key -import logging import re import subprocess from pathlib import Path from typing import Optional -from typing import Tuple from typing import Union import koji -import rpm # The %autorelease macro including parameters. This is imported into the main package to be used @@ -16,32 +12,10 @@ import rpm AUTORELEASE_MACRO = "autorelease(e:s:hp)" AUTORELEASE_SENTINEL = "__AUTORELEASE_SENTINEL__" -evr_re = re.compile(r"^(?:(?P\d+):)?(?P[^-:]+)(?:-(?P[^-:]+))?$") autochangelog_re = re.compile(r"\s*%(?:autochangelog|\{\??autochangelog\})\s*") -rpmvercmp_key = cmp_to_key( - lambda b1, b2: rpm.labelCompare( - (str(b1["epoch"]), b1["version"], b1["release"]), - (str(b2["epoch"]), b2["version"], b2["release"]), - ), -) - _kojiclient = None -_log = logging.getLogger(__name__) - - -def parse_evr(evr_str: str) -> Tuple[int, str, Optional[str]]: - match = evr_re.match(evr_str) - - if not match: - raise ValueError(evr_str) - - epoch = match.group("epoch") or 0 - epoch = int(epoch) - - return epoch, match.group("version"), match.group("release") - def get_rpm_current_version( path: str, name: Optional[str] = None, with_epoch: bool = False @@ -102,22 +76,6 @@ def koji_init(koji_url_or_session: Union[str, koji.ClientSession]) -> koji.Clien return _kojiclient -def run_command(command: list, cwd: Optional[str] = None) -> bytes: - """Run the specified command in a specific working directory if one - is specified. - """ - output = None - try: - output = subprocess.check_output(command, cwd=cwd, stderr=subprocess.PIPE) - except subprocess.CalledProcessError as e: - _log.error("Command `%s` return code: `%s`", " ".join(command), e.returncode) - _log.error("stdout:\n-------\n%s", e.stdout) - _log.error("stderr:\n-------\n%s", e.stderr) - raise - - return output - - def specfile_uses_rpmautospec( specfile: str, check_autorelease: bool = True, check_autochangelog: bool = True ) -> bool: diff --git a/rpmautospec/pkg_history.py b/rpmautospec/pkg_history.py index 967738b..408778d 100644 --- a/rpmautospec/pkg_history.py +++ b/rpmautospec/pkg_history.py @@ -1,8 +1,11 @@ +import datetime as dt import logging from collections import defaultdict +from fnmatch import fnmatchcase from functools import lru_cache, reduce from pathlib import Path from tempfile import TemporaryDirectory +from textwrap import TextWrapper from typing import Any, Dict, Optional, Sequence, Union import pygit2 @@ -14,6 +17,15 @@ log = logging.getLogger(__name__) class PkgHistoryProcessor: + + changelog_ignore_patterns = [ + ".gitignore", + # no need to ignore "changelog" explicitly + "gating.yaml", + "sources", + "tests/*", + ] + def __init__(self, spec_or_path: Union[str, Path]): if isinstance(spec_or_path, str): spec_or_path = Path(spec_or_path) @@ -103,6 +115,138 @@ class PkgHistoryProcessor: yield commit_result + @staticmethod + def _files_changed_in_diff(diff: pygit2.Diff): + files = set() + for delta in diff.deltas: + if delta.old_file: + files.add(delta.old_file.path) + if delta.new_file: + files.add(delta.new_file.path) + return files + + def changelog_visitor(self, commit: pygit2.Commit, children_must_continue: bool): + """Visit a commit to generate changelog entries for it and its parents. + + It first determines if parent chain(s) must be followed, i.e. if the + changelog file was modified in this commit or all its children and yields that to the + caller, who later sends the partial results for this commit (to be + modified) and full results of parents back (as dictionaries), which + get processed and the results for this commit yielded again. + """ + # Check if the spec file exists, if not, there will be no changelog. + specfile_present = f"{self.name}.spec" in commit.tree + + # Find out if the changelog is different from every parent (or present, in the case of the + # root commit). + try: + changelog_blob = commit.tree["changelog"] + except KeyError: + changelog_blob = None + + # With root commits, changelog present means it was changed + changelog_changed = False if commit.parents else bool(changelog_blob) + for parent in commit.parents: + try: + par_changelog_blob = parent.tree["changelog"] + except KeyError: + par_changelog_blob = None + if changelog_blob == par_changelog_blob: + changelog_changed = False + + # Establish which parent to follow (if any, and if we can). + parent_to_follow = None + merge_unresolvable = False + if len(commit.parents) < 2: + if commit.parents: + parent_to_follow = commit.parents[0] + else: + for parent in commit.parents: + if commit.tree == parent.tree: + # Merge done with strategy "ours" or equivalent, i.e. (at least) one parent has + # the same content. Follow this parent + parent_to_follow = parent + break + else: + # Didn't break out of loop => no parent with same tree found. If the changelog + # is different from all parents, it was updated in the merge commit and we don't + # care. If it didn't change, we don't know how to continue and need to flag that. + merge_unresolvable = not changelog_changed + + commit_result, parent_results = yield ( + not (changelog_changed or merge_unresolvable) + and specfile_present + and children_must_continue + ) + + changelog_entry = { + "commit-id": commit.id, + } + + changelog_author = f"{commit.author.name} <{commit.author.email}>" + changelog_date = dt.datetime.utcfromtimestamp(commit.commit_time).strftime("%a %b %d %Y") + changelog_evr = f"{commit_result['epoch-version']}-{commit_result['release-number']}" + changelog_header = f"* {changelog_date} {changelog_author} {changelog_evr}" + + if not specfile_present: + # no spec file => start fresh + commit_result["changelog"] = () + elif merge_unresolvable: + changelog_entry["data"] = f"{changelog_header}\n- RPMAUTOSPEC: unresolvable merge" + changelog_entry["error"] = "unresolvable merge" + previous_changelog = () + commit_result["changelog"] = (changelog_entry,) + elif changelog_changed: + if changelog_blob: + # Don't decode, we'll paste as is. + changelog_entry["data"] = changelog_blob.data + else: + # Changelog removed. Oops. + changelog_entry[ + "data" + ] = f"{changelog_header}\n- RPMAUTOSPEC: changelog file removed" + changelog_entry["error"] = "changelog file removed" + commit_result["changelog"] = (changelog_entry,) + else: + # Pull previous changelog entries from parent result (if any). + if len(commit.parents) == 1: + previous_changelog = parent_results[0].get("changelog", ()) + else: + previous_changelog = () + for candidate in parent_results: + if candidate["commit-id"] == parent_to_follow: + previous_changelog = candidate.get("changelog", ()) + break + + # Check if this commit should be considered for the RPM changelog. + if parent_to_follow: + diff = parent_to_follow.tree.diff_to_tree(commit.tree) + else: + diff = commit.tree.diff_to_tree(swap=True) + changed_files = self._files_changed_in_diff(diff) + # Skip if no files changed (i.e. commit solely for changelog/build) or if any files are + # not to be ignored. + skip_for_changelog = changed_files and all( + any(fnmatchcase(f, pat) for pat in self.changelog_ignore_patterns) + for f in changed_files + ) + + if not skip_for_changelog: + commit_subject = commit.message.split("\n")[0].strip() + if commit_subject.startswith("-"): + commit_subject = commit_subject[1:].lstrip() + if not commit_subject: + commit_subject = "RPMAUTOSPEC: empty commit log subject after stripping" + changelog_entry["error"] = "empty commit log subject" + wrapper = TextWrapper(width=75, subsequent_indent=" ") + wrapped_msg = wrapper.fill(f"- {commit_subject}") + changelog_entry["data"] = f"{changelog_header}\n{wrapped_msg}" + commit_result["changelog"] = (changelog_entry,) + previous_changelog + else: + commit_result["changelog"] = previous_changelog + + yield commit_result + def run( self, head: Optional[Union[str, pygit2.Commit]] = None, diff --git a/tests/rpmautospec/test_misc.py b/tests/rpmautospec/test_misc.py index dc94f99..dccb0a9 100644 --- a/tests/rpmautospec/test_misc.py +++ b/tests/rpmautospec/test_misc.py @@ -1,7 +1,5 @@ import logging import os -import subprocess -from unittest import mock import pytest @@ -13,29 +11,6 @@ __here__ = os.path.dirname(__file__) class TestMisc: """Test the rpmautospec.misc module""" - @mock.patch("rpmautospec.misc.subprocess.check_output") - @pytest.mark.parametrize("raise_exception", (False, True)) - def test_run_command(self, check_output, raise_exception, caplog): - """Test run_command()""" - caplog.set_level(logging.DEBUG) - - if not raise_exception: - check_output.return_value = "Some output" - assert misc.run_command(["command"]) == "Some output" - check_output.assert_called_once_with(["command"], cwd=None, stderr=subprocess.PIPE) - assert not any(rec.levelno >= logging.WARNING for rec in caplog.records) - else: - check_output.side_effect = subprocess.CalledProcessError( - returncode=139, - cmd=["command"], - output="Some command", - stderr="And it failed!", - ) - with pytest.raises(subprocess.CalledProcessError) as excinfo: - misc.run_command(["command"]) - assert str(excinfo.value) == "Command '['command']' returned non-zero exit status 139." - assert any(rec.levelno == logging.ERROR for rec in caplog.records) - def test_specfile_uses_rpmautospec_no_macros(self, caplog): """Test no macros on specfile_uses_rpmautospec()""" caplog.set_level(logging.DEBUG) diff --git a/tests/rpmautospec/test_pkg_history.py b/tests/rpmautospec/test_pkg_history.py index 0f067dc..62b2b27 100644 --- a/tests/rpmautospec/test_pkg_history.py +++ b/tests/rpmautospec/test_pkg_history.py @@ -210,7 +210,7 @@ class TestPkgHistoryProcessor: res = processor.run( *args, - visitors=[processor.release_number_visitor], + visitors=[processor.release_number_visitor, processor.changelog_visitor], all_results=all_results, ) @@ -224,3 +224,15 @@ class TestPkgHistoryProcessor: assert res["commit-id"] == head_commit.id assert res["release-number"] == 2 + + changelog = res["changelog"] + top_entry = changelog[0] + + assert top_entry["commit-id"] == head_commit.id + for snippet in ( + "Jane Doe ", + "- Did nothing!", + ): + assert snippet in top_entry["data"] + + assert all("error" not in entry for entry in changelog) From 0aab00408fd34f4036aacf83c03c2de233ef3f5a Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:17:41 +0000 Subject: [PATCH 11/12] Use 'log' consistently for logging Signed-off-by: Nils Philippsen --- diff --git a/koji_plugins/rpmautospec_builder.py b/koji_plugins/rpmautospec_builder.py index e5f0ed8..4ca3b39 100644 --- a/koji_plugins/rpmautospec_builder.py +++ b/koji_plugins/rpmautospec_builder.py @@ -9,8 +9,8 @@ from rpmautospec import process_distgit CONFIG_FILE = "/etc/kojid/plugins/rpmautospec.conf" CONFIG = None -_log = logging.getLogger(__name__) -_log.setLevel(logging.DEBUG) +log = logging.getLogger(__name__) +log.setLevel(logging.DEBUG) pagure_proxy = None @@ -23,7 +23,7 @@ def process_distgit_cb(cb_type, *, srcdir, taskinfo, **kwargs): return if not process_distgit.needs_processing(srcdir): - _log.info("No %autorelease/%autochangelog found, skipping.") + log.info("No %autorelease/%autochangelog found, skipping.") return global CONFIG, pagure_proxy @@ -33,7 +33,7 @@ def process_distgit_cb(cb_type, *, srcdir, taskinfo, **kwargs): CONFIG = koji.read_config_files([(CONFIG_FILE, True)]) except Exception: message = "While attempting to read config file %s, an exception occurred:" - _log.exception(message, CONFIG_FILE) + log.exception(message, CONFIG_FILE) return process_distgit.process_specfile(srcdir=srcdir) diff --git a/rpmautospec/process_distgit.py b/rpmautospec/process_distgit.py index 1dd953d..3473e2e 100644 --- a/rpmautospec/process_distgit.py +++ b/rpmautospec/process_distgit.py @@ -11,7 +11,7 @@ from .misc import koji_init from .release import calculate_release -_log = logging.getLogger(__name__) +log = logging.getLogger(__name__) __here__ = os.path.dirname(__file__) autorelease_template = """## START: Set by rpmautospec @@ -191,9 +191,9 @@ def process_distgit(srcdir, dist, session, actions=None): features_used.append("%autochangelog") if not features_used: - _log.info("The spec file doesn't use automatic release or changelog.") + log.info("The spec file doesn't use automatic release or changelog.") else: - _log.info("Features used by the spec file: %s", ", ".join(features_used)) + log.info("Features used by the spec file: %s", ", ".join(features_used)) if "process-specfile" in actions and processing_necessary: process_specfile( diff --git a/rpmautospec/release.py b/rpmautospec/release.py index 2075477..5e06230 100644 --- a/rpmautospec/release.py +++ b/rpmautospec/release.py @@ -5,7 +5,7 @@ from typing import Union from .pkg_history import PkgHistoryProcessor -_log = logging.getLogger(__name__) +log = logging.getLogger(__name__) def register_subcommand(subparsers): @@ -32,4 +32,4 @@ def calculate_release(srcdir: Union[str, Path]) -> int: def main(args): """Main method.""" release = calculate_release(srcdir=args.srcdir) - _log.info("calculate_release release: %s", release) + log.info("calculate_release release: %s", release) From b4d17ba51614ae92dbf6862dc512c4de704a112a Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: May 25 2021 15:17:43 +0000 Subject: [PATCH 12/12] Accept shortcuts for current directory In the calculte-release and generate-changelog commands, accept "." or an empty path for the current directory. Fixes: #116 Signed-off-by: Nils Philippsen --- diff --git a/rpmautospec/changelog.py b/rpmautospec/changelog.py index 5f4bac3..45b1f53 100644 --- a/rpmautospec/changelog.py +++ b/rpmautospec/changelog.py @@ -15,7 +15,12 @@ def register_subcommand(subparsers): help="Generate changelog entries from git commit logs", ) - gen_changelog_parser.add_argument("worktree_path", help="Path to the dist-git worktree") + gen_changelog_parser.add_argument( + "spec_or_path", + default=".", + nargs="?", + help="Path to package worktree or the spec file within", + ) return subcmd_name @@ -51,5 +56,5 @@ def produce_changelog(spec_or_repo): def main(args): """Main method.""" - changelog = produce_changelog(args.worktree_path) + changelog = produce_changelog(args.spec_or_path) log.info(changelog) diff --git a/rpmautospec/release.py b/rpmautospec/release.py index 5e06230..371bf78 100644 --- a/rpmautospec/release.py +++ b/rpmautospec/release.py @@ -17,19 +17,22 @@ def register_subcommand(subparsers): ) calc_release_parser.add_argument( - "srcdir", help="Clone of the dist-git repository to use for input" + "spec_or_path", + default=".", + nargs="?", + help="Path to package worktree or the spec file within", ) return subcmd_name -def calculate_release(srcdir: Union[str, Path]) -> int: - processor = PkgHistoryProcessor(srcdir) +def calculate_release(spec_or_path: Union[str, Path]) -> int: + processor = PkgHistoryProcessor(spec_or_path) result = processor.run(visitors=(processor.release_number_visitor,)) return result["release-number"] def main(args): """Main method.""" - release = calculate_release(srcdir=args.srcdir) + release = calculate_release(args.spec_or_path) log.info("calculate_release release: %s", release)