From ab452219407f289b256c795aeccb1672e529cc3c Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Mon, 14 Aug 2023 21:16:49 +0200 Subject: [PATCH] backend: don't run external command to collect built packages See #2850 --- backend/copr-backend.spec | 2 +- .../copr_backend/background_worker_build.py | 26 +++++++++--------- backend/tests/test_background_worker_build.py | 27 +++++++------------ 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/backend/copr-backend.spec b/backend/copr-backend.spec index 0379ea647..1c1cd5b07 100644 --- a/backend/copr-backend.spec +++ b/backend/copr-backend.spec @@ -3,7 +3,7 @@ %endif %global prunerepo_version 1.20 -%global tests_version 3 +%global tests_version 4 %global tests_tar test-data-copr-backend %global copr_common_version 0.16.4.dev diff --git a/backend/copr_backend/background_worker_build.py b/backend/copr_backend/background_worker_build.py index 8be2de7a6..d0344a1f2 100644 --- a/backend/copr_backend/background_worker_build.py +++ b/backend/copr_backend/background_worker_build.py @@ -646,20 +646,20 @@ def _get_srpm_build_details(self, job): return build_details def _collect_built_packages(self, job): - self.log.info("Listing built binary packages in %s", - job.results_dir) - - cmd = ( - "builtin cd {0} && " - "for f in `ls *.rpm | grep -v \"src.rpm$\"`; do" - " rpm --nosignature -qp --qf \"%{{NAME}} %{{VERSION}}\n\" $f; " - "done".format(shlex.quote(job.results_dir)) - ) + """ + Return all built RPM packages as one string, e.g. + 'copr-builder 0.68\ncopr-distgit-client 0.68\ncopr-rpmbuild 0.68' + """ + self.log.info("Listing built binary packages in %s", job.results_dir) + + packages = [] + for pkg in self.job.results["packages"]: + if pkg["arch"] == "src": + continue + packages.append("{0} {1}".format(pkg["name"], pkg["version"])) - result = run_cmd(cmd, shell=True, logger=self.log) - built_packages = result.stdout.strip() - self.log.info("Built packages:\n%s", built_packages) - return built_packages + self.log.info("Built packages:\n%s", packages) + return "\n".join(packages) def _get_build_details(self, job): diff --git a/backend/tests/test_background_worker_build.py b/backend/tests/test_background_worker_build.py index f5524395d..1dfe19563 100644 --- a/backend/tests/test_background_worker_build.py +++ b/backend/tests/test_background_worker_build.py @@ -271,8 +271,7 @@ def raise_exc(): assert (logging.INFO, MESSAGES["repo_waiting"]) \ in [(r[1], r[2]) for r in caplog.record_tuples] -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_full_rpm_build_no_sign(_parse_results, f_build_rpm_case, caplog): +def test_full_rpm_build_no_sign(f_build_rpm_case, caplog): """ Go through the whole (successful) build of a binary RPM """ @@ -311,8 +310,7 @@ def test_prev_build_backup(f_build_rpm_case): assert len(glob.glob(os.path.join(prev_results, rsync_patt))) == 1 -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_full_srpm_build(_parse_results, f_build_srpm): +def test_full_srpm_build(f_build_srpm): worker = f_build_srpm.bw worker.process() assert worker.job.pkg_name == "hello" @@ -328,8 +326,7 @@ def test_full_srpm_build(_parse_results, f_build_srpm): @mock.patch("copr_backend.sign.SIGN_BINARY", "tests/fake-bin-sign") @mock.patch("copr_backend.sign._sign_one") -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_build_and_sign(_parse_results, mc_sign_one, f_build_rpm_sign_on, caplog): +def test_build_and_sign(mc_sign_one, f_build_rpm_sign_on, caplog): config = f_build_rpm_sign_on worker = config.bw worker.process() @@ -581,9 +578,8 @@ def test_cancel_before_start(f_build_rpm_sign_on, caplog): ], caplog) @_patch_bwbuild_object("CANCEL_CHECK_PERIOD", 0.5) -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") @mock.patch("copr_backend.sign.SIGN_BINARY", "tests/fake-bin-sign") -def test_build_retry(_parse_results, f_build_rpm_sign_on): +def test_build_retry(f_build_rpm_sign_on): config = f_build_rpm_sign_on worker = config.bw class _SideEffect(): @@ -693,8 +689,7 @@ def test_cancel_build_during_log_download(f_build_rpm_sign_on, caplog): COMMON_MSGS["not finished"], ], caplog) -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_ssh_connection_error(_parse_results, f_build_rpm_case, caplog): +def test_ssh_connection_error(f_build_rpm_case, caplog): class _SideEffect: counter = 0 def __call__(self): @@ -721,8 +716,7 @@ def test_average_step(): @_patch_bwbuild_object("time.sleep", mock.MagicMock()) @_patch_bwbuild_object("time.time") -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_retry_for_ssh_tail_failure(_parse_results, mc_time, f_build_rpm_case, +def test_retry_for_ssh_tail_failure(mc_time, f_build_rpm_case, caplog): mc_time.side_effect = list(range(500)) class _SideEffect: @@ -769,8 +763,7 @@ def test_createrepo_failure(mc_call_copr_repo, f_build_rpm_case, caplog): "Finished build: id=848963 failed=True ", ], caplog) -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_existing_compressed_file(_parse_results, f_build_rpm_case, caplog): +def test_existing_compressed_file(f_build_rpm_case, caplog): config = f_build_rpm_case config.ssh.precreate_compressed_log_file = True worker = config.bw @@ -781,8 +774,7 @@ def test_existing_compressed_file(_parse_results, f_build_rpm_case, caplog): "Finished build: id=848963 failed=False ", # still success! ], caplog) -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_tail_f_nonzero_exit(_parse_results, f_build_rpm_case, caplog): +def test_tail_f_nonzero_exit(f_build_rpm_case, caplog): config = f_build_rpm_case worker = config.bw class _SideEffect: @@ -841,8 +833,7 @@ def test_unable_to_start_builder(f_build_srpm, caplog): assert_logs_dont_exist(["Retry"], caplog) @_patch_bwbuild_object("time.sleep", mock.MagicMock()) -@_patch_bwbuild_object("BuildBackgroundWorker._parse_results") -def test_retry_vm_factory_take(_parse_results, f_build_srpm, caplog): +def test_retry_vm_factory_take(f_build_srpm, caplog): config = f_build_srpm rhf = config.resalloc_host_factory host = config.host