From bea57d85d909976024d94009da5e100216a41454 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Sun, 2 Jul 2023 00:44:56 +0200 Subject: [PATCH] backend, rpmbuild: move package NEVR parsing to builder Fix #2790 --- backend/copr-backend.spec | 2 - .../copr_backend/background_worker_build.py | 17 ++++-- backend/copr_backend/helpers.py | 58 +------------------ backend/tests/test_background_worker_build.py | 39 ++----------- backend/tests/testlib/__init__.py | 7 ++- .../copr_rpmbuild/automation/srpm_results.py | 3 +- 6 files changed, 22 insertions(+), 104 deletions(-) diff --git a/backend/copr-backend.spec b/backend/copr-backend.spec index c7641fee3..a75ba78f6 100644 --- a/backend/copr-backend.spec +++ b/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 diff --git a/backend/copr_backend/background_worker_build.py b/backend/copr_backend/background_worker_build.py index e9309047a..6c488a165 100644 --- a/backend/copr_backend/background_worker_build.py +++ b/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 diff --git a/backend/copr_backend/helpers.py b/backend/copr_backend/helpers.py index bdccc4c01..f74f2aa4d 100644 --- a/backend/copr_backend/helpers.py +++ b/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" diff --git a/backend/tests/test_background_worker_build.py b/backend/tests/test_background_worker_build.py index 94d39def4..f5524395d 100644 --- a/backend/tests/test_background_worker_build.py +++ b/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 diff --git a/backend/tests/testlib/__init__.py b/backend/tests/testlib/__init__.py index 781a78a8a..037ea854c 100644 --- a/backend/tests/testlib/__init__.py +++ b/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): diff --git a/rpmbuild/copr_rpmbuild/automation/srpm_results.py b/rpmbuild/copr_rpmbuild/automation/srpm_results.py index a95bdfbc9..8d2b353ea 100644 --- a/rpmbuild/copr_rpmbuild/automation/srpm_results.py +++ b/rpmbuild/copr_rpmbuild/automation/srpm_results.py @@ -35,5 +35,6 @@ def get_package_info(self): """ spec_path = locate_spec(self.resultdir) spec = Spec(spec_path) - keys = ["name", "exclusivearch", "excludearch"] + keys = ["name", "epoch", "version", "release", + "exclusivearch", "excludearch"] return {key: getattr(spec, key) for key in keys}