PR#155: feat: function to check whether a specfile uses rpmautospec - fedora-infra/rpmautospec

fedora-infra / rpmautospec

#155 feat: function to check whether a specfile uses rpmautospec

Merged 2 years ago by nphilipp. Opened 2 years ago by scoady.

fedora-infra/ scoady/rpmautospec feature-140 into main

feat: function to check whether a specfile uses rpmautospec

Stephen Coady • 2 years ago

ca34f03

rpmautospec/__init__.py

file modified

+1 -1

		`@@ -1,1 +1,1 @@`
		`- from .misc import AUTORELEASE_MACRO # noqa: F401`
		`+ from .misc import specfile_uses_rpmautospec # noqa: F401`

rpmautospec/misc.py

file modified

+52

		`@@ -16,10 +16,12 @@`
		`# The %autorelease macro including parameters. This is imported into the main package to be used`
		`# from 3rd party code like fedpkg etc.`
		`AUTORELEASE_MACRO = "autorelease(e:s:hp)"`
		`+ AUTORELEASE_SENTINEL = "__AUTORELEASE_SENTINEL__"`

		`release_re = re.compile(r"^(?P<pkgrel>\d+)(?:(?P<middle>.*?)(?:\.(?P<minorbump>\d+))?)?$")`
		`disttag_re = re.compile(r"\.?(?P<distcode>[^\d\.]+)(?P<distver>\d+)")`
		`evr_re = re.compile(r"^(?:(?P<epoch>\d+):)?(?P<version>[^-:]+)(?:-(?P<release>[^-:]+))?$")`
		`+ autochangelog_re = re.compile(r"\s%(?:autochangelog\|\{\??autochangelog\})\s")`

		`rpmvercmp_key = cmp_to_key(`
		`lambda b1, b2: rpm.labelCompare(`
		`@@ -185,3 +187,53 @@`
		`raise`

		`return output`
		`+`
		`+`
		`+ def specfile_uses_rpmautospec(`
		`+ specfile: str, check_autorelease: bool = True, check_autochangelog: bool = True`
		`+ ) -> bool:`
		`+ """Check whether or not an RPM spec file uses rpmautospec features."""`
		`+`
		`+ autorelease = check_autorelease_presence(specfile)`
		`+ autochangelog = check_autochangelog_presence(specfile)`
		`+`
		`+ if check_autorelease and check_autochangelog:`
		`+ return autorelease or autochangelog`
		`+ elif check_autorelease:`
		`+ return autorelease`
		`+ elif check_autochangelog:`
		`+ return autochangelog`
		`+ else:`
		`+ raise ValueError("One of check_autorelease and check_autochangelog must be set")`
		`+`
		`+`
		`+ def check_autorelease_presence(filename: str) -> bool:`
		`+ """`
		`+ Use the rpm package to detect the presence of an`
		`+ autorelease macro and return true if found.`
		`+ """`
		`+ cmd = (`
		`+ "rpm",`
		`+ "--define",`
		`+ "{} {}".format(AUTORELEASE_MACRO, AUTORELEASE_SENTINEL),`
		`+ "-q",`
		`+ "--queryformat",`
		`+ "%{release}\n",`
		`+ "--specfile",`
		`+ filename,`
		`+ )`
		`+ popen = subprocess.Popen(cmd, stdout=subprocess.PIPE)`
		`+ release = popen.communicate()[0].decode(errors="replace").split("\n")[0]`
		`+ return release == AUTORELEASE_SENTINEL`
		`+`
		`+`
		`+ def check_autochangelog_presence(filename: str) -> bool:`
		`+ """`
		`+ Search for the autochangelog macro and return true if found.`
		`+ """`
		`+ with open(filename, "r") as specfile:`
		`+ for _, line in enumerate(iter(specfile), start=1):`
		`+ line = line.rstrip("\n")`
		`+ if autochangelog_re.match(line):`
		`+ return True`
		`+ return False`

tests/rpmautospec/test_misc.py

file modified

+79

		`@@ -1,4 +1,5 @@`
		`import logging`
		`+ import os`
		`import subprocess`
		`from unittest import mock`

		`@@ -6,6 +7,8 @@`

		`from rpmautospec import misc`

		`+ __here__ = os.path.dirname(__file__)`
		`+`

		`class TestMisc:`
		`"""Test the rpmautospec.misc module"""`
		`@@ -32,3 +35,79 @@`
		`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)`
		`+`
		`+ specfile_path = os.path.join(`
		`+ __here__,`
		`+ os.path.pardir,`
		`+ "test-data",`
		`+ "test-specfiles",`
		`+ "no-macros.spec",`
		`+ )`
		`+`
		`+ result = misc.specfile_uses_rpmautospec(specfile_path)`
		`+`
		`+ assert result is False`
		`+`
		`+ def test_specfile_uses_rpmautospec_autorelease_only(self, caplog):`
		`+ """Test autorelease only on specfile_uses_rpmautospec()"""`
		`+ caplog.set_level(logging.DEBUG)`
		`+`
		`+ specfile_path = os.path.join(`
		`+ __here__,`
		`+ os.path.pardir,`
		`+ "test-data",`
		`+ "test-specfiles",`
		`+ "autorelease-only.spec",`
		`+ )`
		`+`
		`+ result = misc.specfile_uses_rpmautospec(specfile_path)`
		`+ assert result is True`
		`+`
		`+ result_no_autorelease = misc.specfile_uses_rpmautospec(`
		`+ specfile_path, check_autorelease=False`
		`+ )`
		`+ assert result_no_autorelease is False`
		`+`
		`+ def test_specfile_uses_rpmautospec_autochangelog_only(self, caplog):`
		`+ """Test autochangelog only on specfile_uses_rpmautospec()"""`
		`+ caplog.set_level(logging.DEBUG)`
		`+`
		`+ specfile_path = os.path.join(`
		`+ __here__,`
		`+ os.path.pardir,`
		`+ "test-data",`
		`+ "test-specfiles",`
		`+ "autochangelog-only.spec",`
		`+ )`
		`+`
		`+ result = misc.specfile_uses_rpmautospec(specfile_path)`
		`+ assert result is True`
		`+`
		`+ result_no_changelog = misc.specfile_uses_rpmautospec(`
		`+ specfile_path, check_autochangelog=False`
		`+ )`
		`+ assert result_no_changelog is False`
		`+`
		`+ def test_specfile_uses_rpmautospec_throws_error(self, caplog):`
		`+ """Test specfile_uses_rpmautospec() throws an error when both params are false"""`
		`+ caplog.set_level(logging.DEBUG)`
		`+`
		`+ specfile_path = os.path.join(`
		`+ __here__,`
		`+ os.path.pardir,`
		`+ "test-data",`
		`+ "test-specfiles",`
		`+ "autochangelog-only.spec",`
		`+ )`
		`+`
		`+ result = misc.specfile_uses_rpmautospec(specfile_path)`
		`+ assert result is True`
		`+`
		`+ with pytest.raises(ValueError):`
		`+ misc.specfile_uses_rpmautospec(`
		`+ specfile_path, check_autochangelog=False, check_autorelease=False`
		`+ )`

tests/test-data/test-specfiles/autochangelog-only.spec

file added

+34

		`@@ -0,0 +1,34 @@`
		`+ # Our dummy-test-packages are named after canary varieties, meet Gloster, Rubino and Crested`
		`+ # Source: https://www.omlet.co.uk/guide/finches_and_canaries/canary/canary_varieties`
		`+ Name: dummy-test-package-gloster`
		`+`
		`+ Version: 0`
		`+ Release: 7`
		`+ Summary: Dummy Test Package called Gloster`
		`+ License: MIT`
		`+ URL: http://fedoraproject.org/wiki/DummyTestPackages`
		`+`
		`+ # The tarball contains a file with an uuid to test later and a LICENSE`
		`+ Source0: %{name}-%{version}.tar.gz`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ This is a dummy test package for the purposes of testing if the Fedora CI`
		`+ pipeline is working. There is nothing useful here.`
		`+`
		`+ %prep`
		`+ %autosetup`
		`+`
		`+ %build`
		`+ # nothing to do`
		`+`
		`+ %install`
		`+ mkdir -p %{buildroot}%{_datadir}`
		`+ cp -p uuid %{buildroot}%{_datadir}/%{name}`
		`+`
		`+ %files`
		`+ %license LICENSE`
		`+ %{_datadir}/%{name}`
		`+`
		`+ %autochangelog`
		`\ No newline at end of file`

tests/test-data/test-specfiles/autorelease-only.spec

file added

+58

		`@@ -0,0 +1,58 @@`
		`+ # Our dummy-test-packages are named after canary varieties, meet Gloster, Rubino and Crested`
		`+ # Source: https://www.omlet.co.uk/guide/finches_and_canaries/canary/canary_varieties`
		`+ Name: dummy-test-package-gloster`
		`+`
		`+ Version: 0`
		`+ Release: %{autorelease}`
		`+ Summary: Dummy Test Package called Gloster`
		`+ License: MIT`
		`+ URL: http://fedoraproject.org/wiki/DummyTestPackages`
		`+`
		`+ # The tarball contains a file with an uuid to test later and a LICENSE`
		`+ Source0: %{name}-%{version}.tar.gz`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ This is a dummy test package for the purposes of testing if the Fedora CI`
		`+ pipeline is working. There is nothing useful here.`
		`+`
		`+ %prep`
		`+ %autosetup`
		`+`
		`+ %build`
		`+ # nothing to do`
		`+`
		`+ %install`
		`+ mkdir -p %{buildroot}%{_datadir}`
		`+ cp -p uuid %{buildroot}%{_datadir}/%{name}`
		`+`
		`+ %files`
		`+ %license LICENSE`
		`+ %{_datadir}/%{name}`
		`+`
		`+ %changelog`
		`+ * Fri Mar 27 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-7`
		`+ - Undo vandalism`
		`+ - Change license to MIT`
		`+`
		`+ * Fri Mar 27 2020 King ISØ-8859 <kingiso8859@example.com> - 0-7`
		`+ - Honour the tradition of antiquated encodings!`
		`+`
		`+ * Fri Mar 27 2020 Nils Philippsen <nils@redhat.com> - 0-6`
		`+ - Convert to automatic release and changelog`
		`+`
		`+ * Tue Jan 21 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-5`
		`+ - rebuilt`
		`+`
		`+ * Thu Jan 16 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-4`
		`+ - rebuilt`
		`+`
		`+ * Fri Jan 10 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-3`
		`+ - rebuilt`
		`+`
		`+ * Fri Jan 10 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-2`
		`+ - rebuilt`
		`+`
		`+ * Thu Dec 19 2019 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-1`
		`+ - Initial packaging work`

tests/test-data/test-specfiles/both-macros.spec

file added

+34

		`@@ -0,0 +1,34 @@`
		`+ # Our dummy-test-packages are named after canary varieties, meet Gloster, Rubino and Crested`
		`+ # Source: https://www.omlet.co.uk/guide/finches_and_canaries/canary/canary_varieties`
		`+ Name: dummy-test-package-gloster`
		`+`
		`+ Version: 0`
		`+ Release: %{autorelease}`
		`+ Summary: Dummy Test Package called Gloster`
		`+ License: MIT`
		`+ URL: http://fedoraproject.org/wiki/DummyTestPackages`
		`+`
		`+ # The tarball contains a file with an uuid to test later and a LICENSE`
		`+ Source0: %{name}-%{version}.tar.gz`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ This is a dummy test package for the purposes of testing if the Fedora CI`
		`+ pipeline is working. There is nothing useful here.`
		`+`
		`+ %prep`
		`+ %autosetup`
		`+`
		`+ %build`
		`+ # nothing to do`
		`+`
		`+ %install`
		`+ mkdir -p %{buildroot}%{_datadir}`
		`+ cp -p uuid %{buildroot}%{_datadir}/%{name}`
		`+`
		`+ %files`
		`+ %license LICENSE`
		`+ %{_datadir}/%{name}`
		`+`
		`+ %autochangelog`

tests/test-data/test-specfiles/no-macros.spec

file added

+58

		`@@ -0,0 +1,58 @@`
		`+ # Our dummy-test-packages are named after canary varieties, meet Gloster, Rubino and Crested`
		`+ # Source: https://www.omlet.co.uk/guide/finches_and_canaries/canary/canary_varieties`
		`+ Name: dummy-test-package-gloster`
		`+`
		`+ Version: 0`
		`+ Release: 7`
		`+ Summary: Dummy Test Package called Gloster`
		`+ License: MIT`
		`+ URL: http://fedoraproject.org/wiki/DummyTestPackages`
		`+`
		`+ # The tarball contains a file with an uuid to test later and a LICENSE`
		`+ Source0: %{name}-%{version}.tar.gz`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ This is a dummy test package for the purposes of testing if the Fedora CI`
		`+ pipeline is working. There is nothing useful here.`
		`+`
		`+ %prep`
		`+ %autosetup`
		`+`
		`+ %build`
		`+ # nothing to do`
		`+`
		`+ %install`
		`+ mkdir -p %{buildroot}%{_datadir}`
		`+ cp -p uuid %{buildroot}%{_datadir}/%{name}`
		`+`
		`+ %files`
		`+ %license LICENSE`
		`+ %{_datadir}/%{name}`
		`+`
		`+ %changelog`
		`+ * Fri Mar 27 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-7`
		`+ - Undo vandalism`
		`+ - Change license to MIT`
		`+`
		`+ * Fri Mar 27 2020 King ISØ-8859 <kingiso8859@example.com> - 0-7`
		`+ - Honour the tradition of antiquated encodings!`
		`+`
		`+ * Fri Mar 27 2020 Nils Philippsen <nils@redhat.com> - 0-6`
		`+ - Convert to automatic release and changelog`
		`+`
		`+ * Tue Jan 21 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-5`
		`+ - rebuilt`
		`+`
		`+ * Thu Jan 16 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-4`
		`+ - rebuilt`
		`+`
		`+ * Fri Jan 10 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-3`
		`+ - rebuilt`
		`+`
		`+ * Fri Jan 10 2020 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-2`
		`+ - rebuilt`
		`+`
		`+ * Thu Dec 19 2019 Pierre-Yves Chibon <pingou@pingoured.fr> - 0-1`
		`+ - Initial packaging work`

scoady commented 2 years ago

Signed-off-by: Stephen Coady scoady@redhat.com

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

pre-commit : FAILURE in 1m 41s
pytest : SUCCESS in 1m 16s
rpm-mock-build : SUCCESS in 4m 29s

rebased onto 37416b80f8cc64b1fa9d9e3e7b41d9b1836a62f4

2 years ago

zuul commented 2 years ago

Build succeeded.

pre-commit : SUCCESS in 1m 46s
pytest : SUCCESS in 1m 15s
rpm-mock-build : SUCCESS in 4m 34s

nphilipp commented on line 3 of rpmautospec/__init__.py 2 years ago

We should probably drop AUTORELEASE_MACRO here, 3rd parties should only use the function. What do you think?

Metadata Update from @nphilipp:
- Request assigned

2 years ago

nphilipp commented on line 14 of rpmautospec/misc.py 2 years ago

Is this something that black changed? AIUI, PEP8 doesn't require 2 blank lines here, but it's not prohibited either.

scoady commented 2 years ago

Is this something that black changed? AIUI, PEP8 doesn't require 2 blank lines here, but it's not prohibited either.

I thought black did it but I just checked and you're right it doesn't have a problem. Maybe I ran flake8 as well and that complained.

rebased onto 5e22ac02d0fbf1c73c73621f687f9865eca75bb9

2 years ago

zuul commented 2 years ago

Build succeeded.

pre-commit : SUCCESS in 1m 34s
pytest : SUCCESS in 1m 16s
rpm-mock-build : SUCCESS in 4m 14s

nphilipp commented on line 35 of rpmautospec/misc.py 2 years ago

Not the least because this is external API, this should get a docstring and defined the return type, e.g.:

def specfile_uses_rpmautospec(
    specfile: str, check_autorelease: bool = True, check_autochangelog: bool = True
) -> bool:
    """Check whether or not an RPM spec file uses rpmautospec features."""
    autorelease = ...

nphilipp commented on line 46 of rpmautospec/misc.py 2 years ago

Both check_autorelease and check_autochangelog set to False is likely an error, let's raise an exception here, e.g.:

    else:
        raise ValueError("One of check_autorelease and check_autochangelog must be set.")

nphilipp commented on line 22 of tests/rpmautospec/test_misc.py 2 years ago

Test methods are named after the methods they test :wink: with optional suffixes, here I'd use something like:

    def test_specfile_uses_rpmautospec_no_macros(self, caplog):
        """Test specfile_uses_rpmautospec() on a spec file without macros."""

rebased onto 49ef50e35e6abc421df0c48c851a239dbb0797e2

2 years ago

zuul commented 2 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

rebased onto 144d8f20f8a73fcde5a56343bc8b43ef1f088d46

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci