rpkg

Clone

#589 Fix srpm and binary rpm lookup in lint subcommand
Merged 2 years ago by onosek. Opened 2 years ago by oturpe.
oturpe/rpkg use-layout-parameters-in-lint  into  master

file modified
+36 -15 
@@ -2543,21 +2543,43 @@ 
 

          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)):
 

-             log.warning('No srpm found')
 

+         rpm_globs = set()
 

+         srpm_base = "%s-%s-%s.src.rpm" % (self.repo_name, self.ver, self.rel)
 

+ 

+         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:
 

+             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')
 

+ 

+             # 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'))
 

  

-         # Get the possible built arches
 

-         arches = set(self._get_build_arches_from_spec())
 

          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')))
 

+         for rpm_glob in rpm_globs:
 

+             rpms.update(glob.glob(rpm_glob))
 

          if not rpms:
 

              log.warning('No rpm found')
 

+ 

          cmd = ['rpmlint']
 

          default_rpmlintconf = '{0}.rpmlintrc'.format(self.repo_name)
 

          if info:
 
@@ -2569,11 +2591,10 @@ 
 

          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))
 

          cmd.extend(sorted(rpms))
 

-         # Run the command
 

+ 

          self._run_command(cmd, shell=True)
 

  

      def list_side_tags(self, base_tag=None, user=None):

file modified
+72 -1 
@@ -993,10 +993,13 @@ 
 

          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
 

          glob.side_effect = _mock_glob
 

-         cmd._get_build_arches_from_spec = Mock(return_value=['x86_64', 'x86_64'])
 

+         cmd._get_build_arches_from_spec = Mock(
 

+             return_value=['x86_64', 'x86_64'])
 

  

          cmd.lint()
 

  
@@ -1008,6 +1011,74 @@ 
 

                     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],
 

+                 srpm_path: [srpm_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, '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 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"""

file modified
+9 -5 
@@ -218,15 +218,17 @@ 
 

          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 @@ 
 

          :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 @@ 
 

                          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):

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}.

Also, support for linting mockbuild results is added.
As mockbuild is arguably even more important than local build,
cases where both mockbuild and local build results are present,
mockbuild results are selected.

Resolves #586

rebased onto a61ac06cef18d0b5e409c08795f48d2ee0c062f5
2 years ago

rebased onto 5bf8cc2
2 years ago

pretty please pagure-ci rebuild
2 years ago

Thanks for the improvement.

Commit 324b9b0 fixes this pull-request

Pull-Request has been merged by onosek
2 years ago
Metadata
None
No Tags
Changes Summary 3
+36 -15
file changed
pyrpkg/__init__.py
+72 -1
file changed
tests/test_commands.py
+9 -5
file changed
tests/utils.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.