Move package NEVR parsing to builder

backend/copr-backend.spec

@@ -56,7 +56,6 @@ BuildRequires: python3-retask
 BuildRequires: python3-setproctitle
 BuildRequires: python3-sphinx
 BuildRequires: python3-tabulate
-BuildRequires: python3-specfile
 BuildRequires: modulemd-tools >= 0.6
 BuildRequires: prunerepo >= %prunerepo_version
 BuildRequires: dnf
@@ -95,7 +94,6 @@ Requires:   python3-retask
 Requires:   python3-setproctitle
 Requires:   python3-tabulate
 Requires:   python3-boto3
-Requires:   python3-specfile
 Requires:   redis
 Requires:   rpm-sign
 Requires:   rsync

backend/copr_backend/background_worker_build.py

@@ -24,7 +24,7 @@
     FrontendClientException,
 )
 from copr_backend.helpers import (
-    call_copr_repo, pkg_name_evr, run_cmd, register_build_result, find_spec_file,
+    call_copr_repo, run_cmd, register_build_result, format_evr,
 )
 from copr_backend.job import BuildJob
 from copr_backend.msgbus import MessageSender
@@ -628,15 +628,20 @@ def _do_createrepo(self):
 
     def _get_srpm_build_details(self, job):
         build_details = {'srpm_url': ''}
-        self.log.info("Retrieving srpm URL from %s", job.results_dir)
+        self.log.info("Retrieving SRPM info from %s", job.results_dir)
+
+        results_path = os.path.join(job.results_dir, "results.json")
+        with open(results_path, "r", encoding="utf-8") as fp:
+            results = json.load(fp)
+
+        build_details["pkg_name"] = results["name"]
+        build_details["pkg_version"] = format_evr(
+            results["epoch"], results["version"], results["release"])
+
         pattern = os.path.join(job.results_dir, '*.src.rpm')
         srpm_file = glob.glob(pattern)[0]
         srpm_name = os.path.basename(srpm_file)
         srpm_url = os.path.join(job.results_dir_url, srpm_name)
-        spec_file_path = find_spec_file(job.results_dir, self.log)
-
-        build_details['pkg_name'], build_details['pkg_version'] = \
-                pkg_name_evr(spec_file_path, srpm_file, self.log)
         build_details['srpm_url'] = srpm_url
         self.log.info("SRPM URL: %s", srpm_url)
         return build_details

backend/copr_backend/helpers.py

@@ -27,13 +27,12 @@
 
 import munch
 from munch import Munch
-from specfile import Specfile
 
 from copr_common.redis_helpers import get_redis_connection
 from copr.v3 import Client
 from copr_backend.constants import DEF_BUILD_USER, DEF_BUILD_TIMEOUT, DEF_CONSECUTIVE_FAILURE_THRESHOLD, \
     CONSECUTIVE_FAILURE_REDIS_KEY, default_log_format
-from copr_backend.exceptions import CoprBackendError, CoprBackendSrpmError
+from copr_backend.exceptions import CoprBackendError
 
 from . import constants
 
@@ -633,22 +632,6 @@ def get_chroot_arch(chroot):
     return chroot.rsplit("-", 2)[2]
 
 
-def find_spec_file(folder, logger):
-    """
-    Returns spec file path or None
-    """
-    spec_pattern = os.path.join(folder, "*.spec")
-    result = glob.glob(spec_pattern)
-    if not result:
-        return None
-
-    if len(result) > 1:
-        logger.error("More than one spec file found! Using the first %s from %s",
-                     str(result[0]), str(result))
-
-    return result[0]
-
-
 def format_evr(epoch, version, release):
     """
     Return evr in format (epoch:)version-release
@@ -659,45 +642,6 @@ def format_evr(epoch, version, release):
     return f"{version}-{release}"
 
 
-def pkg_name_evr(spec_file_path, srpm_path, logger):
-    """
-    Queries a SRPM or spec file for its name and evr (epoch:version-release)
-    """
-    try:
-        spec = Specfile(spec_file_path)
-        return spec.expanded_name, format_evr(spec.expanded_epoch,
-                                              spec.expanded_version,
-                                              spec.expanded_release)
-    except Exception:  # pylint: disable=broad-exception-caught
-        # Specfile library raises too many exception to name the in except block
-        logger.exception("Exception appeared during handling spec file: %s", spec_file_path)
-        return pkg_name_evr_from_srpm(srpm_path, logger)
-
-
-
-def pkg_name_evr_from_srpm(srpm_path, logger):
-    """
-    Queries a package for its name and evr (epoch:version-release)
-    """
-    cmd = ['rpm', '-qp', '--nosignature', '--qf',
-           '%{NAME} %{EPOCH} %{VERSION} %{RELEASE}', srpm_path]
-
-    try:
-        result = run_cmd(cmd, logger=logger)
-    except OSError as e:
-        raise CoprBackendSrpmError(str(e)) from e
-
-    if result.returncode != 0:
-        raise CoprBackendSrpmError('Error querying SRPM')
-
-    try:
-        name, epoch, version, release = result.stdout.split(" ")
-    except ValueError as e:
-        raise CoprBackendSrpmError(str(e)) from e
-
-    return name, format_evr(epoch, version, release)
-
-
 def format_filename(name, version, release, epoch, arch, zero_epoch=False):
     if not epoch.isdigit() and zero_epoch:
         epoch = "0"

backend/tests/test_background_worker_build.py

@@ -27,7 +27,6 @@
 from copr_backend.vm_alloc import ResallocHost, RemoteHostAllocationTerminated
 from copr_backend.background_worker_build import COMMANDS, MIN_BUILDER_VERSION
 from copr_backend.sshcmd import SSHConnectionError
-from copr_backend.exceptions import CoprBackendSrpmError
 
 import testlib
 from testlib import assert_logs_exist, assert_logs_dont_exist
@@ -169,6 +168,7 @@ def f_build_srpm(f_build_something):
     to build RPM.
     """
     config = f_build_something
+    config.ssh.resultdir = "srpm-builds/02914697"
     config.fe_client.return_value.get.return_value = _get_srpm_job()
     config.ssh.set_command(
         "copr-rpmbuild --verbose --drop-resultdir --srpm --task-url "
@@ -315,32 +315,15 @@ def test_prev_build_backup(f_build_rpm_case):
 def test_full_srpm_build(_parse_results, f_build_srpm):
     worker = f_build_srpm.bw
     worker.process()
-    assert worker.job.pkg_name == "example"
+    assert worker.job.pkg_name == "hello"
 
     # TODO: fix this is ugly pkg_version testament
     assert worker.job.pkg_version is None
-    assert worker.job.__dict__["pkg_version"] == "1.0.14-1"
+    assert worker.job.__dict__["pkg_version"] == "2.8-1"
 
     assert worker.job.srpm_url == (
         "https://example.com/results/@copr/PROJECT_2/srpm-builds/"
-        "00855954/example-1.0.14-1.fc30.src.rpm")
-
-
-@mock.patch("copr_backend.background_worker_build.find_spec_file")
-@_patch_bwbuild_object("BuildBackgroundWorker._parse_results")
-def test_full_srpm_build_without_specfile(_parse_results, mock_find_spec_file,
-                                          f_build_srpm):
-    mock_find_spec_file.return_value = None
-
-    worker = f_build_srpm.bw
-    worker.process()
-
-    assert worker.job.pkg_name == "example"
-    assert worker.job.pkg_version is None
-    assert worker.job.__dict__["pkg_version"] == "1.0.14-1.fc30"
-    assert worker.job.srpm_url == (
-            "https://example.com/results/@copr/PROJECT_2/srpm-builds/"
-            "00855954/example-1.0.14-1.fc30.src.rpm")
+        "00855954/hello-2.8-1.src.rpm")
 
 
 @mock.patch("copr_backend.sign.SIGN_BINARY", "tests/fake-bin-sign")
@@ -786,20 +769,6 @@ def test_createrepo_failure(mc_call_copr_repo, f_build_rpm_case, caplog):
         "Finished build: id=848963 failed=True ",
     ], caplog)
 
-@_patch_bwbuild_object("pkg_name_evr")
-@_patch_bwbuild_object("BuildBackgroundWorker._parse_results")
-def test_pkg_collect_failure(_parse_results, mc_pkg_evr, f_build_srpm, caplog):
-    mc_pkg_evr.side_effect = CoprBackendSrpmError("srpm error")
-    config = f_build_srpm
-    worker = config.bw
-    worker.process()
-    assert_logs_exist([
-        "Error while collecting built packages",
-        "Worker failed build, took ",
-        "Finished build: id=855954 failed=True ",
-    ], caplog)
-    assert worker.job.status == 0  # fail
-
 @_patch_bwbuild_object("BuildBackgroundWorker._parse_results")
 def test_existing_compressed_file(_parse_results, f_build_rpm_case, caplog):
     config = f_build_rpm_case

backend/tests/testlib/__init__.py

@@ -134,6 +134,7 @@ def __init__(self, user=None, host=None, config_file=None, log=None):
                          0, "", "")
         self.set_command("copr-rpmbuild-log",
                          0, "build log stdout\n", "build log stderr\n")
+        self.resultdir = "fedora-30-x86_64/00848963-example"
 
     def set_command(self, cmd, exit_code, stdout, stderr, action=None,
                     return_action=None):
@@ -177,8 +178,8 @@ def _full_source_path(self, src):
     def rsync_download(self, src, dest, logfile=None, max_retries=0):
         data = os.environ["TEST_DATA_DIRECTORY"]
         trail_slash = src.endswith("/")
-        src = os.path.join(data, "build_results", "fedora-30-x86_64",
-                           "00848963-example")
+        src = os.path.join(data, "build_results", self.resultdir)
+
         if trail_slash:
             src = src + "/"
 
@@ -193,7 +194,7 @@ def rsync_download(self, src, dest, logfile=None, max_retries=0):
         if self.unlink_success:
             os.unlink(os.path.join(dest, "success"))
 
-        if "PROJECT_2" in dest:
+        if "PROJECT_2" in dest and "fedora-30-x86_64" in dest:
             os.unlink(os.path.join(dest, "example-1.0.14-1.fc30.x86_64.rpm"))
 
 def assert_logs_exist(messages, caplog):

beaker-tests/Sanity/copr-cli-basic-operations/runtest-build-results-json.sh

@@ -22,6 +22,16 @@ rlJournalStart
         rlRun "parse_build_id"
         rlRun "copr watch-build $BUILD_ID"
 
+        # Check the SRPM results.json file that is stored on backend
+        URL_PATH="results/${NAME_PREFIX}TestResultsJson/srpm-builds/$(build_id_with_leading_zeroes)/results.json"
+        path="$RESULTDIR/results.json"
+        rlRun "curl $BACKEND_URL/$URL_PATH > $path"
+        rlRun "cat $path |jq -e '.name == \"hello\"'"
+        rlRun "cat $path |jq -e '.epoch == null'"
+        rlRun "cat $path |jq -e '.version == \"2.8\"'"
+        rlRun "cat $path |jq -e '.release == \"1\"'"
+
+        # Check results.json for the final chroots
         checkResults()
         {
             orig_file=$1

rpmbuild/copr-rpmbuild.spec

@@ -40,7 +40,7 @@ BuildRequires: %{python_pfx}-munch
 BuildRequires: %{python}-requests
 BuildRequires: %{python_pfx}-jinja2
 BuildRequires: %{python_pfx}-simplejson
-BuildRequires: %{python_pfx}-specfile
+BuildRequires: %{python_pfx}-specfile >= 0.19.0
 BuildRequires: python3-backoff >= 1.9.0
 
 BuildRequires: /usr/bin/argparse-manpage
@@ -58,7 +58,7 @@ Requires: %{python_pfx}-jinja2
 Requires: %{python_pfx}-munch
 Requires: %{python}-requests
 Requires: %{python_pfx}-simplejson
-Requires: %{python_pfx}-specfile
+Requires: %{python_pfx}-specfile >= 0.19.0
 Requires: python3-backoff >= 1.9.0
 
 Requires: mock >= 2.0

rpmbuild/copr_rpmbuild/automation/srpm_results.py

@@ -5,7 +5,12 @@
 import os
 import simplejson
 from copr_rpmbuild.automation.base import AutomationTool
-from copr_rpmbuild.helpers import locate_spec, Spec
+from copr_rpmbuild.helpers import (
+    get_rpm_header,
+    locate_srpm,
+    locate_spec,
+    Spec,
+)
 
 
 class SRPMResults(AutomationTool):
@@ -33,7 +38,20 @@ def get_package_info(self):
         """
         Return ``dict`` with interesting package metadata
         """
-        spec_path = locate_spec(self.resultdir)
-        spec = Spec(spec_path)
-        keys = ["name", "exclusivearch", "excludearch"]
-        return {key: getattr(spec, key) for key in keys}
+        keys = ["name", "epoch", "version", "release",
+                "exclusivearch", "excludearch"]
+        try:
+            path = locate_spec(self.resultdir)
+            spec = Spec(path)
+            return {key: getattr(spec, key) for key in keys}
+
+        except Exception:  # pylint: disable=broad-exception-caught
+            # Specfile library raises too many exception to name the
+            # in except block
+            msg = "Exception appeared during handling spec file: {0}".format(path)
+            self.log.exception(msg)
+
+            # Fallback to querying NEVRA from the SRPM package header
+            path = locate_srpm(self.resultdir)
+            hdr = get_rpm_header(path)
+            return {key: hdr[key] for key in keys}

rpmbuild/copr_rpmbuild/helpers.py

@@ -108,6 +108,11 @@ def get_rpm_header(path):
     See docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch16s04.html
     """
     ts = rpm.TransactionSet()
+    # I don't want to copy-paste the value of the protected variable.
+    # IMHO there is only a low chance it will get removed and even if it gets,
+    # we have tests to catch it anyway
+    # pylint: disable=protected-access
+    ts.setVSFlags(rpm._RPMVSF_NOSIGNATURES)
     with open(path, "rb") as f:
         hdr = ts.hdrFromFdno(f.fileno())
         return hdr