From 5bf8cc24dea5d5fb1ba5c66dfd7bb58d42c3cc01 Mon Sep 17 00:00:00 2001 From: Otto Urpelainen Date: Nov 28 2021 10:08:38 +0000 Subject: [PATCH 1/2] Fix srpm and binary rpm lookup in lint subcommand Subcommand lint was looking for srpm only in repository root, and for rpms only with globs of form %{ARCH}/*.rpm. This works in many cases, but not when the config option 'results_dir' is set to 'subdir' or when (if ever) SRPMLayout is used. Fixed by using relevant layout parameters when looking for files to lint. It is difficult to account for layout parameter 'rpmfilename' properly. In addition to a basename, that parameter can contain a path relative to 'rpmdir' and may contain rpm macros. Since macro evaluation is not possible in the 'rpkg lint' context, a simple heuristic is used instead, covering the known cases: 1. only basename 2. relative path '%{ARCH}'. Signed-off-by: Otto Urpelainen --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index db64224..ce62a2a 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -2543,21 +2543,29 @@ class Commands(object): specified by the command line argument. """ - # Check for srpm - srpm = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel) - if not os.path.exists(os.path.join(self.path, srpm)): + # Find srpm + srpm_base = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel) + srpm = os.path.join(self.layout.srcrpmdir, srpm_base) + if not os.path.exists(srpm): log.warning('No srpm found') - # Get the possible built arches - arches = set(self._get_build_arches_from_spec()) + # Find binary rpms rpms = set() - for arch in arches: - if os.path.exists(os.path.join(self.path, arch)): - # For each available arch folder, lists file and keep - # those ending with .rpm - rpms.update(glob.glob(os.path.join(self.path, arch, '*.rpm'))) + if self.layout.rpmfilename.find('/') == -1: + # No directories created under rpmdir + rpms.update(glob.glob(os.path.join(self.layout.rpmdir, '*.rpm'))) + else: + # Assuming rpmfilename starts with %{ARCH}/, the rpm default + arches = set(self._get_build_arches_from_spec()) + + for arch in arches: + archdir = os.path.join(self.layout.rpmdir, arch) + if os.path.exists(archdir): + rpms.update(glob.glob(os.path.join(archdir, '*.rpm'))) + if not rpms: log.warning('No rpm found') + cmd = ['rpmlint'] default_rpmlintconf = '{0}.rpmlintrc'.format(self.repo_name) if info: @@ -2569,11 +2577,12 @@ class Commands(object): elif os.path.isfile(os.path.join(self.path, ".rpmlint")): self.log.warning('.rpmlint file usage as default rpmlint configuration is deprecated ' 'Use {0} instead'.format(default_rpmlintconf)) - cmd.append(os.path.join(self.path, self.spec)) - if os.path.exists(os.path.join(self.path, srpm)): - cmd.append(os.path.join(self.path, srpm)) + + cmd.append(os.path.join(self.layout.specdir, self.spec)) + if os.path.exists(srpm): + cmd.append(srpm) cmd.extend(sorted(rpms)) - # Run the command + self._run_command(cmd, shell=True) def list_side_tags(self, base_tag=None, user=None): diff --git a/tests/test_commands.py b/tests/test_commands.py index 0dfc072..9de014f 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -994,9 +994,44 @@ class TestLint(CommandTestCase): return { os.path.join(cmd.path, 'x86_64', '*.rpm'): [bin_path], }[g] + + exists.side_effect = _mock_exists + glob.side_effect = _mock_glob + cmd._get_build_arches_from_spec = Mock( + return_value=['x86_64', 'x86_64']) + + cmd.lint() + + self.assertEqual( + run.call_args_list, + [call(['rpmlint', + os.path.join(cmd.path, 'docpkg.spec'), + srpm_path, + bin_path, + ], shell=True)]) + + @patch('glob.glob') + @patch('os.path.exists') + @patch('pyrpkg.Commands._run_command') + @patch('pyrpkg.Commands.load_rpmdefines', new=mock_load_rpmdefines) + @patch('pyrpkg.Commands.rel', new_callable=PropertyMock) + def test_lint_dist_git_results_layout(self, rel, run, exists, glob): + rel.return_value = '2.fc26' + + cmd = self.make_commands(results_dir='subdir') + srpm_path = os.path.join(cmd.path, 'results', + 'docpkg-1.2-2.fc26.src.rpm') + bin_path = os.path.join(cmd.path, 'results', + 'docpkg-1.2-2.fc26.x86_64.rpm') + + def _mock_exists(path): + return path in [srpm_path, os.path.join(cmd.path, 'results')] + + def _mock_glob(g): + return {os.path.join(cmd.path, 'results', '*.rpm'): [bin_path]}[g] + exists.side_effect = _mock_exists glob.side_effect = _mock_glob - cmd._get_build_arches_from_spec = Mock(return_value=['x86_64', 'x86_64']) cmd.lint() diff --git a/tests/utils.py b/tests/utils.py index c36a655..fa51625 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -218,15 +218,17 @@ class CommandTestCase(RepoCreationMixin, Assertions, Utils, unittest.TestCase): if self.require_test_repos and self.create_repo_per_test: self.destroy_fake_repos() - def make_commands(self, path=None, user=None, dist=None, target=None, quiet=None): + def make_commands(self, path=None, user=None, dist=None, target=None, + quiet=None, results_dir=None): """Helper method for creating Commands object for test cases This is where you should extend to add more features to support additional requirements from other Commands specific test cases. - Some tests need customize one of user, dist, target, and quiet options - when creating an instance of Commands. Keyword arguments user, dist, - target, and quiet here is for this purpose. + Some tests need customize one of user, dist, target, quiet and + result_dir options when creating an instance of Commands. Keyword + arguments user, dist, target, quiet and results_dir are here is for + this purpose. :param str path: path to repository where this Commands will work on top of @@ -234,6 +236,7 @@ class CommandTestCase(RepoCreationMixin, Assertions, Utils, unittest.TestCase): :param str dist: dist passed to --dist option :param str target: target passed to --target option :param str quiet: quiet passed to --quiet option + :param str results_dir: results_dir passed to results_dir config option """ _repo_path = path if path else self.cloned_repo_path return Commands(_repo_path, @@ -241,7 +244,8 @@ class CommandTestCase(RepoCreationMixin, Assertions, Utils, unittest.TestCase): gitbaseurl, anongiturl, branchre, kojiconfig, build_client, - user=user, dist=dist, target=target, quiet=quiet) + user=user, dist=dist, target=target, quiet=quiet, + results_dir=results_dir) @staticmethod def checkout_branch(repo, branch_name): From 38b6ae23b1c22cd3ea297c2c7ebf43c1b05298d4 Mon Sep 17 00:00:00 2001 From: Otto Urpelainen Date: Nov 28 2021 10:08:38 +0000 Subject: [PATCH 2/2] Add support for mockbuild in lint command Previously, the lint command assumed that linted srpm and binary rpms were in locations specified by the repository layout. However, the mockbuild command uses its own directory structure. Support for mockbuild results linting is added here. As mockbuild is arguably even more important than local build, cases where both mockbuild and local build results are present, mockbuild results are selected. --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index ce62a2a..3ac1a63 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -2543,26 +2543,40 @@ class Commands(object): specified by the command line argument. """ - # Find srpm + rpm_globs = set() srpm_base = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel) - srpm = os.path.join(self.layout.srcrpmdir, srpm_base) - if not os.path.exists(srpm): - log.warning('No srpm found') - # Find binary rpms - rpms = set() - if self.layout.rpmfilename.find('/') == -1: - # No directories created under rpmdir - rpms.update(glob.glob(os.path.join(self.layout.rpmdir, '*.rpm'))) + mockdir = os.path.join( + "results_%s" % self.repo_name, self.ver, self.rel) + if (os.path.exists(mockdir)): + log.info("Mockbuild results directory found. " + "Linting mockbuild results.") + rpm_globs.add(os.path.join(mockdir, '*.rpm')) else: - # Assuming rpmfilename starts with %{ARCH}/, the rpm default - arches = set(self._get_build_arches_from_spec()) + log.info("Mockbuild results directory not found. " + "Linting local build results.") + srpm = os.path.join(self.layout.srcrpmdir, srpm_base) + if os.path.exists(srpm): + rpm_globs.add(srpm) + else: + log.warning('No srpm found') - for arch in arches: - archdir = os.path.join(self.layout.rpmdir, arch) - if os.path.exists(archdir): - rpms.update(glob.glob(os.path.join(archdir, '*.rpm'))) + # Binary rpms + if self.layout.rpmfilename.find('/') == -1: + # No directories created under rpmdir + rpm_globs.add(os.path.join(self.layout.rpmdir, '*.rpm')) + else: + # Assuming rpmfilename starts with %{ARCH}/, the rpm default + arches = set(self._get_build_arches_from_spec()) + + for arch in arches: + archdir = os.path.join(self.layout.rpmdir, arch) + if os.path.exists(archdir): + rpm_globs.add(os.path.join(archdir, '*.rpm')) + rpms = set() + for rpm_glob in rpm_globs: + rpms.update(glob.glob(rpm_glob)) if not rpms: log.warning('No rpm found') @@ -2579,8 +2593,6 @@ class Commands(object): 'Use {0} instead'.format(default_rpmlintconf)) cmd.append(os.path.join(self.layout.specdir, self.spec)) - if os.path.exists(srpm): - cmd.append(srpm) cmd.extend(sorted(rpms)) self._run_command(cmd, shell=True) diff --git a/tests/test_commands.py b/tests/test_commands.py index 9de014f..079f53a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -993,6 +993,7 @@ class TestLint(CommandTestCase): def _mock_glob(g): return { os.path.join(cmd.path, 'x86_64', '*.rpm'): [bin_path], + srpm_path: [srpm_path], }[g] exists.side_effect = _mock_exists @@ -1028,7 +1029,10 @@ class TestLint(CommandTestCase): return path in [srpm_path, os.path.join(cmd.path, 'results')] def _mock_glob(g): - return {os.path.join(cmd.path, 'results', '*.rpm'): [bin_path]}[g] + return { + os.path.join(cmd.path, 'results', '*.rpm'): [bin_path], + srpm_path: [srpm_path], + }[g] exists.side_effect = _mock_exists glob.side_effect = _mock_glob @@ -1043,6 +1047,38 @@ class TestLint(CommandTestCase): bin_path, ], shell=True)]) + @patch('glob.glob') + @patch('os.path.exists') + @patch('pyrpkg.Commands._run_command') + @patch('pyrpkg.Commands.load_rpmdefines', new=mock_load_rpmdefines) + @patch('pyrpkg.Commands.rel', new_callable=PropertyMock) + def lint_mockbuild(self, rel, run, exists, glob): + rel.return_value = '2.fc26' + + cmd = self.make_commands() + mockdir = os.path.join(cmd.path, 'results_docpkg/1.2.2/2.fc26') + srpm_path = os.path.join(mockdir, 'docpkg-1.2-2.fc26.src.rpm') + bin_path = os.path.join(mockdir, 'docpkg-1.2-2.fc26.x86_64.rpm') + + def _mock_exists(path): + return path in [mockdir] + + def _mock_glob(g): + return {mockdir: [srpm_path, bin_path]}[g] + + exists.side_effect = _mock_exists + glob.side_effect = _mock_glob + + cmd.lint() + + self.assertEqual( + run.call_args_list, + [call(['rpmlint', + os.path.join(cmd.path, 'dockpkg.spec'), + srpm_path, + bin_path, + ], shell=True)]) + class TestRunCommand(CommandTestCase): """Test _run_command"""