From 4ce957808a96ef172c83b0b5eee8e5b6fc8dcad4 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:33 +0000
Subject: [PATCH 1/31] Use the print function


---

diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 74a9591..e4c1085 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -30,6 +30,7 @@ The idea is:
     - warn user
 """
 
+from __future__ import print_function
 import argparse
 import ConfigParser
 import getpass
@@ -109,7 +110,7 @@ def add_comment_build(output_build, bug):
     koji command and add a comment with this link on the review
     request.
     """
-    print 'Adding comment about the koji build'
+    print('Adding comment about the koji build')
     url = None
     for line in output_build.split('\n'):
         if 'Task info' in line:
@@ -198,7 +199,7 @@ class ReviewRequest(object):
 
     def create_review_request(self, rename_request):
         """ Create the review request on the bugzilla. """
-        print 'Creating review'
+        print('Creating review')
         review_type = 'Review Request'
         if rename_request:
             review_type = 'Rename Request'
@@ -226,21 +227,21 @@ class ReviewRequest(object):
             bug = self.bzclient.createbug(**data)
             bug.refresh()
         except Fault, ex:
-            print ex
+            print(ex)
             self.login_bz()
             return self.create_review_request(rename_request)
         return bug
 
     def do_scratch_build(self, target='rawhide'):
         """ Starts a scratch build on koji. """
-        print 'Starting scratch build'
+        print('Starting scratch build')
         cmd = ['koji', 'build', '--scratch', target, self.srpmfile]
         self.log.debug(cmd)
         try:
             proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             output = proc.communicate()[0]
         except OSError, err:
-            print "OSError : %s" % str(err)
+            print("OSError : %s" % str(err))
         return (output, proc.returncode)
 
     def fill_urls(self):
@@ -272,10 +273,10 @@ class ReviewRequest(object):
              'component': 'Package Review'})
 
         if bugbz:
-            print 'Reviews for a package of the same name have been found:'
+            print('Reviews for a package of the same name have been found:')
         for bug in bugbz:
-            print ' ', bug, '-', bug.resolution
-            print "\t", bug.url
+            print(' ', bug, '-', bug.resolution)
+            print("\t", bug.url)
 
         if bugbz:
             usr_inp = raw_input('Do you want to proceed anyway? [Y/N]')
@@ -296,7 +297,7 @@ class ReviewRequest(object):
             bzurl = 'https://bugzilla.redhat.com'
         else:
             bzurl = 'https://partner-bugzilla.redhat.com'
-            print "Using test bugzilla at: " + bzurl
+            print("Using test bugzilla at: " + bzurl)
 
         self.username = args.username
         if not self.username:
@@ -329,9 +330,9 @@ class ReviewRequest(object):
         bug = self.create_review_request(args.rename_request)
         if not args.no_build:
             add_comment_build(output_build, bug)
-        print 'Review created at: %s/show_bug.cgi?id=%s' % (bzurl,
-                                                            bug.id)
-        print bug
+        print('Review created at: %s/show_bug.cgi?id=%s' % (bzurl,
+                                                            bug.id))
+        print(bug)
 
     def retrieve_description(self):
         """ Retrieve the description tag from a spec file. """
@@ -354,7 +355,7 @@ class ReviewRequest(object):
     def upload_files(self):
         """ Upload the spec file and the src.rpm files into
         fedorapeople.org, ensuring readable mode."""
-        print 'Uploading files into fedorapeople'
+        print('Uploading files into fedorapeople')
         self.log.debug('Target: %s', self.settings.upload_target)
         for path in [self.specfile, self.srpmfile]:
             mode = os.stat(path)[0]
@@ -367,7 +368,7 @@ class ReviewRequest(object):
             proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             output = proc.communicate()[0]
         except OSError, err:
-            print "OSError : %s" % str(err)
+            print("OSError : %s" % str(err))
         return (output, proc.returncode)
 
 
@@ -403,7 +404,7 @@ if __name__ == '__main__':
     try:
         ReviewRequest().main()
     except Exception as err:                 # pylint: disable=broad-except
-        print err
+        print(err)
 
 
 # vim: set expandtab ts=4 sw=4:
diff --git a/src/FedoraReview/download_scratch.py b/src/FedoraReview/download_scratch.py
index 79cd86a..3dcb6dc 100644
--- a/src/FedoraReview/download_scratch.py
+++ b/src/FedoraReview/download_scratch.py
@@ -3,6 +3,7 @@
 
 ''' Download a scrath build from koji. '''
 
+from __future__ import print_function
 import optparse
 import os
 import sys
@@ -18,16 +19,16 @@ def _do_download(task, session, opts):
     ''' Perform download of a single task. '''
 
     if opts.arches:
-        print 'Downloading %s rpms from task %i: %s' \
-            % (', '.join(opts.arches), task['id'], koji.taskLabel(task))
+        print('Downloading %s rpms from task %i: %s'
+              % (', '.join(opts.arches), task['id'], koji.taskLabel(task)))
     else:
-        print 'Downloading rpms from task %i: %s' \
-            % (task['id'], koji.taskLabel(task))
+        print('Downloading rpms from task %i: %s'
+              % (task['id'], koji.taskLabel(task)))
     base_path = koji.pathinfo.taskrelpath(task['id'])
     output = session.listTaskOutput(task['id'])
     prog_meter = urlgrabber.progress.TextMeter()
     if output == []:
-        print "This build is empty, no files to download"
+        print("This build is empty, no files to download")
         sys.exit(1)
     for filename in output:
         if opts.nologs and filename.endswith('log'):
@@ -62,8 +63,8 @@ def _download_scratch_rpms(parser, task_ids, opts):
             parser.error('Task %i did not complete successfully' % task['id'])
 
         if task['method'] == 'build':
-            print 'Getting rpms from children of task %i: %s' \
-                % (task['id'], koji.taskLabel(task))
+            print('Getting rpms from children of task %i: %s'
+                  % (task['id'], koji.taskLabel(task)))
             task_opts = {'parent': task_id,
                          'method': 'buildArch',
                          'state': [koji.TASK_STATES['CLOSED']],
diff --git a/src/FedoraReview/review_helper.py b/src/FedoraReview/review_helper.py
index e5edb69..07d6f20 100644
--- a/src/FedoraReview/review_helper.py
+++ b/src/FedoraReview/review_helper.py
@@ -22,6 +22,7 @@
 Tools for helping Fedora package reviewers
 '''
 
+from __future__ import print_function
 import os.path
 import sys
 import time
@@ -116,23 +117,23 @@ class ReviewHelper(object):
             self.checks.run_checks(output=output,
                                    writedown=not Settings.no_report)
         if not Settings.no_report:
-            print apply_color("Review template in: " + self.outfile,
-                              ansi.green)
-            print apply_color(_EXIT_MESSAGE, ansi.red)
+            print(apply_color("Review template in: " + self.outfile,
+                              ansi.green))
+            print(apply_color(_EXIT_MESSAGE, ansi.red))
 
     @staticmethod
     def _list_flags():
         ''' List all flags in simple, user-friendly format. '''
         checks_lister = ChecksLister()
         for flag in checks_lister.flags.itervalues():
-            print flag.name + ': ' + flag.doc
+            print(flag.name + ': ' + flag.doc)
 
     @staticmethod
     def _list_plugins():
         ''' --display-plugins implementation. '''
         checks_lister = ChecksLister()
         plugins = checks_lister.get_plugins()
-        print ', '.join(plugins)
+        print(', '.join(plugins))
 
     @staticmethod
     def _list_checks():
@@ -141,11 +142,11 @@ class ReviewHelper(object):
         def list_data_by_file(files, checks_list):
             ''' print filename + flags and checks defined in it. '''
             for f in sorted(files):
-                print 'File:  ' + f
+                print('File:  ' + f)
                 flags_by_src = filter(lambda c: c.defined_in == f,
                                       checks_lister.flags.itervalues())
                 for flag in flags_by_src:
-                    print 'Flag: ' + flag.name
+                    print('Flag: ' + flag.name)
                 files_per_src = filter(lambda c: c.defined_in == f,
                                        checks_list)
                 groups = list(set([c.group for c in files_per_src]))
@@ -158,10 +159,10 @@ class ReviewHelper(object):
                     checks = filter(check_match, checks_list)
                     if checks == []:
                         continue
-                    print 'Group: ' + group
+                    print('Group: ' + group)
                     for c in sorted(checks):
-                        print '    %s: %s' % (c.name, c.text)
-                print
+                        print('    %s: %s' % (c.name, c.text))
+                print()
 
         checks_lister = ChecksLister()
         checks_list = list(checks_lister.get_checks().itervalues())
@@ -171,28 +172,28 @@ class ReviewHelper(object):
                            c.needs != ['CheckBuildCompleted'],
                            checks_list)
         for dep in deps_list:
-            print'Dependencies: ' + dep.name + ': ' + \
-                os.path.basename(dep.defined_in)
+            print('Dependencies: ' + dep.name + ': ' +
+                  os.path.basename(dep.defined_in))
             for needed in dep.needs:
-                print '     ' + needed
+                print('     ' + needed)
         deprecators = filter(lambda c: c.deprecates != [], checks_list)
         for dep in deprecators:
-            print 'Deprecations: ' + dep.name + ': ' + \
-                os.path.basename(dep.defined_in)
+            print('Deprecations: ' + dep.name + ': ' +
+                  os.path.basename(dep.defined_in))
             for victim in dep.deprecates:
-                print '    ' + victim
+                print('    ' + victim)
 
     @staticmethod
     def _print_version():
         ''' Handle --version option. '''
         # pylint: disable=superfluous-parens
-        print('fedora-review version ' + __version__ + ' ' + BUILD_FULL)
+        print(('fedora-review version ' + __version__ + ' ' + BUILD_FULL))
         print('external plugins:')
         checks_lister = ChecksLister()
         for registry in checks_lister.groups.itervalues():
             if registry.external_plugin:
-                print "{r.group} version {r.version} {r.build_id}".format(
-                    r=registry)
+                print("{r.group} version {r.version} {r.build_id}".format(
+                    r=registry))
 
     def _do_run(self, outfile=None):
         ''' Initiate, download url:s, run checks a write report. '''
diff --git a/src/FedoraReview/settings.py b/src/FedoraReview/settings.py
index a6e969f..911e4de 100644
--- a/src/FedoraReview/settings.py
+++ b/src/FedoraReview/settings.py
@@ -19,6 +19,7 @@
 ''' Tools for helping Fedora package reviewers '''
 
 
+from __future__ import print_function
 import argparse
 import grp
 import logging
@@ -305,7 +306,7 @@ class _Settings(object):                         # pylint: disable=R0902
         self.do_logger_setup()
         for opt in ['--assign', '--login', '--user', '-a', '-i', '-l']:
             if opt in sys.argv:
-                print self.BZ_OPTS_MESSAGE
+                print(self.BZ_OPTS_MESSAGE)
                 self.init_done = True
                 raise self.SettingsError()
         parser = argparse.ArgumentParser(
diff --git a/test/fr_testcase.py b/test/fr_testcase.py
index 2338c1f..7ebd775 100644
--- a/test/fr_testcase.py
+++ b/test/fr_testcase.py
@@ -20,6 +20,7 @@
 Base class for FedoraReview tests
 '''
 
+from __future__ import print_function
 import os
 import os.path
 import shutil
@@ -68,7 +69,7 @@ class FR_TestCase(unittest.TestCase):
 
     def tearDown(self):
         if 'REVIEW_TEST_GIT_STATUS' in os.environ:
-            print
+            print()
             subprocess.call('git status -uno | grep  "modified:"',
                             shell=True)
         os.chdir(self.startdir)
diff --git a/try-fedora-review b/try-fedora-review
index eb810cc..3376671 100755
--- a/try-fedora-review
+++ b/try-fedora-review
@@ -12,6 +12,7 @@
 #
 # (C) 2011 - Alec Leamas
 
+from __future__ import print_function
 import os
 import os.path
 import sys
@@ -32,10 +33,10 @@ from FedoraReview.review_helper import ReviewHelper
 cmd = ["git", "--git-dir", os.path.join(main_dir, '.git'), "rev-parse", "--abbrev-ref", "HEAD"]
 cur_branch = check_output(cmd).strip()
 if cur_branch == "master":
-    print "\033[91mYou are running fedora-review from 'master' branch. This " \
-          "is probably not what you want to do. Master branch is equal to " \
-          "latest stable release. For development/bugreporting use " \
-          "'devel' branch. See CONTRIBUTE file for more details\033[0m"
+    print("\033[91mYou are running fedora-review from 'master' branch. This "
+          "is probably not what you want to do. Master branch is equal to "
+          "latest stable release. For development/bugreporting use "
+          "'devel' branch. See CONTRIBUTE file for more details\033[0m")
 
 # let's setup a proper git commit hooks so people don't commit ugly code
 # accidentally

From ced190d57327f57445d6ce8812af656239777e94 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:33 +0000
Subject: [PATCH 2/31] Six moves


---

diff --git a/plugins/generic.py b/plugins/generic.py
index 8bfac8e..f9c6654 100644
--- a/plugins/generic.py
+++ b/plugins/generic.py
@@ -23,7 +23,7 @@ import os
 import os.path
 import re
 from glob import glob
-from io import BytesIO
+from six import BytesIO
 from subprocess import Popen, PIPE, CalledProcessError
 
 import rpm
diff --git a/src/FedoraReview/abstract_bug.py b/src/FedoraReview/abstract_bug.py
index 9621060..01bb603 100644
--- a/src/FedoraReview/abstract_bug.py
+++ b/src/FedoraReview/abstract_bug.py
@@ -24,7 +24,7 @@ import tempfile
 
 from abc import ABCMeta, abstractmethod
 from glob import glob
-from urlparse import urlparse
+from six.moves.urllib.parse import urlparse
 
 from helpers_mixin import HelpersMixin
 from review_error import ReviewError
diff --git a/src/FedoraReview/check_base.py b/src/FedoraReview/check_base.py
index 132d87e..d918e41 100644
--- a/src/FedoraReview/check_base.py
+++ b/src/FedoraReview/check_base.py
@@ -21,12 +21,12 @@
 ''' Basic definitions: AbstractCheck + descendants, TestResult.  '''
 
 import re
-import StringIO
+from six.moves import StringIO
 
 from abc import ABCMeta, abstractmethod
 from textwrap import TextWrapper
 
-from helpers_mixin import HelpersMixin
+from .helpers_mixin import HelpersMixin
 
 
 class _Attachment(object):
@@ -288,7 +288,7 @@ class TestResult(object):
 
     def get_text(self):
         ''' Return printable representation of test. '''
-        strbuf = StringIO.StringIO()
+        strbuf = StringIO()
         main_lines = self.wrapper.wrap(self._leader + self.text)
         strbuf.write('\n'.join(main_lines))
         if self.output_extra and self.output_extra != "":
diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index e4c1085..8538181 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -32,16 +32,18 @@ The idea is:
 
 from __future__ import print_function
 import argparse
-import ConfigParser
 import getpass
 import logging
 import os
+import rpm
 import stat
 import subprocess
 from subprocess import Popen, check_output
 import sys
-from xmlrpclib import Fault
-import rpm
+
+import six.moves.configparser
+from six.moves.xmlrpc_client import Fault
+
 from bugzilla.rhbugzilla import RHBugzilla
 
 SETTINGS_FILE = os.path.join(os.environ['HOME'],
@@ -144,7 +146,7 @@ class Settings(object):
         :arg configfile, name of the configuration file loaded.
         :arg sec, section of the configuration retrieved.
         """
-        parser = ConfigParser.ConfigParser()
+        parser = six.moves.configparser.ConfigParser()
         configfile = os.path.join(os.environ['HOME'], configfile)
         is_new = create_conf(configfile)
         parser.read(configfile)
diff --git a/src/FedoraReview/name_bug.py b/src/FedoraReview/name_bug.py
index 8b7f90b..e470b96 100644
--- a/src/FedoraReview/name_bug.py
+++ b/src/FedoraReview/name_bug.py
@@ -21,9 +21,11 @@ import os
 import os.path
 
 from glob import glob
-from urlparse import urlparse
 from review_dirs import ReviewDirs
 from settings import Settings
+
+from six.moves.urllib.parse import urlparse
+
 from abstract_bug import AbstractBug
 
 
diff --git a/src/FedoraReview/source.py b/src/FedoraReview/source.py
index be99503..9c61ee3 100644
--- a/src/FedoraReview/source.py
+++ b/src/FedoraReview/source.py
@@ -23,7 +23,7 @@ Tools for helping Fedora package reviewers
 import os.path
 import shutil
 
-from urlparse import urlparse
+from six.moves.urllib.parse import urlparse
 
 from helpers_mixin import HelpersMixin, DownloadError
 from review_dirs import ReviewDirs

From d48c443433f0ad694b955388bc6013a0470e3a1a Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 3/31] New except syntax


---

diff --git a/plugins/R.py b/plugins/R.py
index 4e21ed4..c12da03 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -59,7 +59,7 @@ class RCheckBase(CheckBase):
                 stream = urllib.urlopen(url)
                 content = stream.read()
                 stream.close()
-            except IOError, err:
+            except IOError as err:
                 self.log.warning('Could not retrieve info from %s', url)
                 self.log.debug('Error: %s', err, exc_info=True)
                 continue
diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 8538181..45aa310 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -228,7 +228,7 @@ class ReviewRequest(object):
         try:
             bug = self.bzclient.createbug(**data)
             bug.refresh()
-        except Fault, ex:
+        except Fault as ex:
             print(ex)
             self.login_bz()
             return self.create_review_request(rename_request)
@@ -242,7 +242,7 @@ class ReviewRequest(object):
         try:
             proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             output = proc.communicate()[0]
-        except OSError, err:
+        except OSError as err:
             print("OSError : %s" % str(err))
         return (output, proc.returncode)
 
@@ -369,7 +369,7 @@ class ReviewRequest(object):
         try:
             proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             output = proc.communicate()[0]
-        except OSError, err:
+        except OSError as err:
             print("OSError : %s" % str(err))
         return (output, proc.returncode)
 
diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index 747511e..1f112a7 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -55,7 +55,7 @@ class HelpersMixin(object):
         output, error = '', 'undefined'
         try:
             output, error = proc.communicate()
-        except OSError, e:
+        except OSError as e:
             self.log.debug("OS error, stderr: %s", error, exc_info=True)
             self.log.error("OS error running %s %s", ' '.join(cmd), str(e))
         return output

From af9c23fe1c906cb0e0030e0ca77e5ad9ed7e2044 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 4/31] Replace iter(keys|values|items) with the non-iter ones


This will work on both Pythons, being a bit less effective on 2.

---

diff --git a/plugins/generic.py b/plugins/generic.py
index f9c6654..8535876 100644
--- a/plugins/generic.py
+++ b/plugins/generic.py
@@ -597,7 +597,7 @@ class CheckLicenseField(GenericCheckBase):
     def _write_license(files_by_license, filename):
         ''' Dump files_by_license to filename. '''
         with open(filename, 'w') as f:
-            for license_ in sorted(files_by_license.iterkeys()):
+            for license_ in sorted(files_by_license.keys()):
                 f.write('\n' + license_ + '\n')
                 f.write('-' * len(license_) + '\n')
                 for path in sorted(files_by_license[license_]):
@@ -642,7 +642,7 @@ class CheckLicenseField(GenericCheckBase):
             license_ = license_.strip()
             if not license_is_valid(license_):
                 license_ = self.unknown_license
-            if license_ not in files_by_license.iterkeys():
+            if license_ not in files_by_license.keys():
                 files_by_license[license_] = []
             files_by_license[license_].append(file_)
         return files_by_license
@@ -672,7 +672,7 @@ class CheckLicenseField(GenericCheckBase):
                 msg += ' Please check the source files for licenses manually.'
             else:
                 msg += ' Licenses found: "' \
-                       + '", "'.join(files_by_license.iterkeys()) + '".'
+                       + '", "'.join(files_by_license.keys()) + '".'
                 if self.unknown_license in files_by_license:
                     msg += ' %d files have unknown license.' % \
                                 len(files_by_license[self.unknown_license])
diff --git a/plugins/generic_build.py b/plugins/generic_build.py
index 36fe968..f3c0433 100644
--- a/plugins/generic_build.py
+++ b/plugins/generic_build.py
@@ -390,7 +390,7 @@ class CheckBuildCompleted(BuildCheckBase):
         text += 'Active plugins: ' + ', '.join(plugins) + '\n'
         plugins = self.checks.get_plugins(False)
         text += 'Disabled plugins: ' + ', '.join(plugins) + '\n'
-        flags = [f for f in self.checks.flags.iterkeys()
+        flags = [f for f in self.checks.flags.keys()
                  if not self.checks.flags[f]]
         flags = ', '.join(flags) if flags else 'None'
         text += 'Disabled flags: ' + flags + '\n'
diff --git a/plugins/generic_should.py b/plugins/generic_should.py
index 6a3ef36..2c10185 100644
--- a/plugins/generic_should.py
+++ b/plugins/generic_should.py
@@ -415,7 +415,7 @@ class CheckPkgConfigFiles(GenericShouldCheckBase):
             return
         passed = 'pass'
         extra = ''
-        for pkg in files_by_pkg.iterkeys():
+        for pkg in files_by_pkg.keys():
             for fn in files_by_pkg[pkg]:
                 if '-devel' not in pkg:
                     passed = 'pending'
diff --git a/plugins/ruby.py b/plugins/ruby.py
index 59996b5..7c061fb 100644
--- a/plugins/ruby.py
+++ b/plugins/ruby.py
@@ -453,7 +453,7 @@ class GemCheckUsesMacros(GemCheckBase):
 
         err_message = []
         # construct the error message for all non-present macros
-        for key, value in re_dict.iteritems():
+        for key, value in re_dict.items():
             if value is False:
                 err_message.append(key.pattern.replace('\\s+', ' '))
 
diff --git a/plugins/shell_api.py b/plugins/shell_api.py
index 8d1e6c1..4686b85 100644
--- a/plugins/shell_api.py
+++ b/plugins/shell_api.py
@@ -174,7 +174,7 @@ def _quote(s):
 def _settings_generator():
     ''' Bash code defining FR_SETTINGS, reflecting Settings. '''
     body = 'declare -A FR_SETTINGS \n'
-    for key in Settings.__dict__.iterkeys():
+    for key in Settings.__dict__.keys():
         if key.startswith('_'):
             continue
         value = Settings.__dict__[key]
@@ -189,7 +189,7 @@ def _settings_generator():
 def _source_generator(spec):
     ''' Bash code defining the %sourceX items. '''
     body = ''
-    for tag, path in spec.sources_by_tag.iteritems():
+    for tag, path in spec.sources_by_tag.items():
         body += 'export ' + tag + '="' + path + '"\n'
     return body
 
@@ -197,7 +197,7 @@ def _source_generator(spec):
 def _patch_generator(spec):
     ''' Bash code defining the %patchX items. '''
     body = ''
-    for tag, path in spec.patches_by_tag.iteritems():
+    for tag, path in spec.patches_by_tag.items():
         body += 'export ' + tag + '="' + path + '"\n'
     return body
 
@@ -227,7 +227,7 @@ def _description_generator(spec):
 def _flags_generator(flags):
     ''' Bash code defining FR_FLAGS,reflecting checks.flags. '''
     body = 'declare -A FR_FLAGS\n'
-    for flag in flags.itervalues():
+    for flag in flags.values():
         body += """FR_FLAGS[%s]='%s'\n""" % (flag.name, str(flag))
     return body
 
diff --git a/src/FedoraReview/checks.py b/src/FedoraReview/checks.py
index 174bf5f..a104624 100644
--- a/src/FedoraReview/checks.py
+++ b/src/FedoraReview/checks.py
@@ -66,7 +66,7 @@ class _CheckDict(dict):
                              first.name, first.defined_in, second.name,
                              second.defined_in)
 
-        if key in self.iterkeys():
+        if key in self.keys():
             log_duplicate(value, self[key])
         dict.__setitem__(self, key, value)
         value.checkdict = self
@@ -76,7 +76,7 @@ class _CheckDict(dict):
             raise TypeError("update: at most 1 arguments, got %d" %
                             len(args))
         other = dict(*args, **kwargs)
-        for key in other.iterkeys():
+        for key in other.keys():
             self[key] = other[key]
 
     def add(self, check):
@@ -115,10 +115,10 @@ class _CheckDict(dict):
 
         value = self[key]
         for victim in value.deprecates:
-            if victim in self.iterkeys():
+            if victim in self.keys():
                 log_kill(self[victim], value)
                 del self[victim]
-        for killer in self.itervalues():
+        for killer in self.values():
             if key in killer.deprecates:
                 log_kill(value, killer)
                 return
@@ -235,7 +235,7 @@ class _ChecksLoader(object):
     def get_plugins(self, state='true_or_false'):
         ''' Return list of groups (i. e., plugins) being active/passive. '''
         plugins = []
-        for p in self.groups.iterkeys():
+        for p in self.groups.keys():
             if state != 'true_or_false':
                 r = self.groups[p]
                 if r.is_user_enabled():
@@ -317,7 +317,7 @@ class Checks(_ChecksLoader):
 
     def deprecate(self):
         ''' Mark all deprecated tests as run. '''
-        allkeys = list(self.checkdict.iterkeys())
+        allkeys = list(self.checkdict.keys())
         for c in allkeys:
             if c not in self.checkdict:
                 continue
@@ -326,7 +326,7 @@ class Checks(_ChecksLoader):
 
     def _get_ready_to_run(self):
         ''' Return checks ready to run, deprecating checks first. '''
-        names = list(self.checkdict.iterkeys())
+        names = list(self.checkdict.keys())
         tests_to_run = filter(self._ready_to_run, names)
         return sorted(tests_to_run,
                       key=lambda t: len(self.checkdict[t].deprecates),
@@ -334,7 +334,7 @@ class Checks(_ChecksLoader):
 
     def is_external_plugin_installed(self, group_name):
         ''' Return True if external plugin install for given group. '''
-        for reg_group, registry in self.groups.iteritems():
+        for reg_group, registry in self.groups.items():
             basename = reg_group.split('.')[0]
             if basename == group_name and registry.external_plugin:
                 return True
diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 45aa310..5f53fc1 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -173,7 +173,7 @@ class Settings(object):
         else:
             opts = set()
 
-        for name in self._dict.iterkeys():
+        for name in self._dict.keys():
             value = None
             if name in opts:
                 value = parser.get(section, name)
diff --git a/src/FedoraReview/datasrc.py b/src/FedoraReview/datasrc.py
index b5da0b5..99ac27a 100644
--- a/src/FedoraReview/datasrc.py
+++ b/src/FedoraReview/datasrc.py
@@ -222,20 +222,20 @@ class RpmDataSource(AbstractDataSource):
         if container:
             return self.rpms_by_pkg[container].filelist
         all_ = []
-        for pkg in self.rpms_by_pkg.iterkeys():
+        for pkg in self.rpms_by_pkg.keys():
             all_.extend(self.rpms_by_pkg[pkg].filelist)
         return all_
 
     def get(self, key=None):
         ''' Return RpmFile object for a package name key. '''
         self.init()
-        if key and key in self.rpms_by_pkg.iterkeys():
+        if key and key in self.rpms_by_pkg.keys():
             return self.rpms_by_pkg[key]
         return None
 
     def get_keys(self):
         self.init()
-        return self.rpms_by_pkg.iterkeys()
+        return self.rpms_by_pkg.keys()
 
 
 class SourcesDataSource(AbstractDataSource):
@@ -244,9 +244,9 @@ class SourcesDataSource(AbstractDataSource):
     def __init__(self, spec):
         AbstractDataSource.__init__(self)
         self.sources_by_tag = {}
-        for tag, url in spec.sources_by_tag.iteritems():
+        for tag, url in spec.sources_by_tag.items():
             self.sources_by_tag[tag] = Source(tag, url)
-        self.containers = [s.tag for s in self.sources_by_tag.itervalues()]
+        self.containers = [s.tag for s in self.sources_by_tag.values()]
         self.files_by_tag = {}
 
     def init(self):
@@ -255,7 +255,7 @@ class SourcesDataSource(AbstractDataSource):
     def _load_files(self, tag):
         """ Ensure that file list for tag is in files_by_tag. """
 
-        if tag in self.files_by_tag.iterkeys():
+        if tag in self.files_by_tag.keys():
             return
         source = self.sources_by_tag[tag]
         if not source.extract_dir:
@@ -277,7 +277,7 @@ class SourcesDataSource(AbstractDataSource):
             self._load_files(container)
             return self.files_by_tag[container]
         all_ = []
-        for key in self.files_by_tag.iterkeys():
+        for key in self.files_by_tag.keys():
             self._load_files(key)
             all_.extend(self.files_by_tag[key])
         return all_
@@ -287,12 +287,12 @@ class SourcesDataSource(AbstractDataSource):
         if not key:
             key = 'Source0'
             self.log.warning('Retrieving default source as Source0')
-        if key not in self.sources_by_tag.iterkeys():
+        if key not in self.sources_by_tag.keys():
             return None
         return self.sources_by_tag[key]
 
     def get_keys(self):
-        return self.sources_by_tag.iterkeys()
+        return self.sources_by_tag.keys()
 
     def get_files_sources(self):
         ''' Compatibility, to be removed. '''
diff --git a/src/FedoraReview/review_helper.py b/src/FedoraReview/review_helper.py
index 07d6f20..b27d5fd 100644
--- a/src/FedoraReview/review_helper.py
+++ b/src/FedoraReview/review_helper.py
@@ -125,7 +125,7 @@ class ReviewHelper(object):
     def _list_flags():
         ''' List all flags in simple, user-friendly format. '''
         checks_lister = ChecksLister()
-        for flag in checks_lister.flags.itervalues():
+        for flag in checks_lister.flags.values():
             print(flag.name + ': ' + flag.doc)
 
     @staticmethod
@@ -144,7 +144,7 @@ class ReviewHelper(object):
             for f in sorted(files):
                 print('File:  ' + f)
                 flags_by_src = filter(lambda c: c.defined_in == f,
-                                      checks_lister.flags.itervalues())
+                                      checks_lister.flags.values())
                 for flag in flags_by_src:
                     print('Flag: ' + flag.name)
                 files_per_src = filter(lambda c: c.defined_in == f,
@@ -165,7 +165,7 @@ class ReviewHelper(object):
                 print()
 
         checks_lister = ChecksLister()
-        checks_list = list(checks_lister.get_checks().itervalues())
+        checks_list = list(checks_lister.get_checks().values())
         files = list(set([c.defined_in for c in checks_list]))
         list_data_by_file(files, checks_list)
         deps_list = filter(lambda c: c.needs != [] and
@@ -190,7 +190,7 @@ class ReviewHelper(object):
         print(('fedora-review version ' + __version__ + ' ' + BUILD_FULL))
         print('external plugins:')
         checks_lister = ChecksLister()
-        for registry in checks_lister.groups.itervalues():
+        for registry in checks_lister.groups.values():
             if registry.external_plugin:
                 print("{r.group} version {r.version} {r.build_id}".format(
                     r=registry))
diff --git a/src/FedoraReview/settings.py b/src/FedoraReview/settings.py
index 911e4de..bb1abb0 100644
--- a/src/FedoraReview/settings.py
+++ b/src/FedoraReview/settings.py
@@ -251,7 +251,7 @@ class _Settings(object):                         # pylint: disable=R0902
         This instanciate the Settings object and load into the _dict
         attributes the default configuration which each available option.
         '''
-        for key, value in self.defaults.iteritems():
+        for key, value in self.defaults.items():
             setattr(self, key, value)
         self._dict = self.defaults
         self.log = None
@@ -337,7 +337,7 @@ class _Settings(object):                         # pylint: disable=R0902
     def add_args(self, args):
         """ Load all command line options in args. """
         var_dict = vars(args)
-        for key, value in var_dict.iteritems():
+        for key, value in var_dict.items():
             setattr(self, key, value)
 
     @property
@@ -350,7 +350,7 @@ class _Settings(object):                         # pylint: disable=R0902
         if not self.log:
             return
         self.log.debug("Active settings after processing options")
-        for k, v in vars(self).iteritems():
+        for k, v in vars(self).items():
             if k in ['_dict', 'mock_config_options', 'log']:
                 continue
             try:
diff --git a/test/fr_testcase.py b/test/fr_testcase.py
index 7ebd775..c0f8819 100644
--- a/test/fr_testcase.py
+++ b/test/fr_testcase.py
@@ -145,7 +145,7 @@ class FR_TestCase(unittest.TestCase):
         self.assertEqual(rc, 0)
         sys.stdout = stdout
         checkdict = helper.checks.get_checks()
-        for check in checkdict.itervalues():
+        for check in checkdict.values():
             self.assertTrue(check.is_run)
             if check.is_passed or check.is_pending or check.is_failed:
                 self.assertIn(check.group, spec.groups_ok,
diff --git a/test/test_R_checks.py b/test/test_R_checks.py
index 931cd4c..954e347 100644
--- a/test/test_R_checks.py
+++ b/test/test_R_checks.py
@@ -102,7 +102,7 @@ class TestRChecks(FR_TestCase):
         self.bug.download_files()
         self.checks = Checks(self.bug.spec_file, self.bug.srpm_file)
         self.checks.run_checks(writedown=False)
-        for check in self.checks.checkdict.itervalues():
+        for check in self.checks.checkdict.values():
             #if not os.path.exists('test-R/rpms-unpacked'):
             #    os.mkdir('test-R/rpms-unpacked')
             if check.is_passed or check.is_pending or check.is_failed:

From a56343099d218cd8ce86df80574e4390e85d78d7 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 5/31] Use BeautifulSoup 4


---

diff --git a/src/FedoraReview/url_bug.py b/src/FedoraReview/url_bug.py
index 941a19c..50031d2 100644
--- a/src/FedoraReview/url_bug.py
+++ b/src/FedoraReview/url_bug.py
@@ -20,7 +20,7 @@ No xmlrpc involved, for better or worse.
 import os.path
 import urllib
 
-from BeautifulSoup import BeautifulSoup
+from bs4 import BeautifulSoup
 
 from abstract_bug import AbstractBug
 
@@ -46,7 +46,7 @@ class UrlBug(AbstractBug):
             tmpfile = self.bug_url.replace('file://', '')
         else:
             tmpfile = urllib.urlretrieve(self.bug_url)[0]
-        soup = BeautifulSoup(open(tmpfile))
+        soup = BeautifulSoup(open(tmpfile), features="lxml")
         links = soup.findAll('a')
         hrefs = [l.get('href') for l in links if l.get('href')]
         found = []

From 63106dde2639cd5787f4d4033dcaf1610dc6ec49 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 6/31] Relative imports


---

diff --git a/plugins/generic_should.py b/plugins/generic_should.py
index 2c10185..c67bc2d 100644
--- a/plugins/generic_should.py
+++ b/plugins/generic_should.py
@@ -31,7 +31,7 @@ from FedoraReview import CheckBase, Mock, ReviewDirs
 from FedoraReview import ReviewError             # pylint: disable=W0611
 from FedoraReview import RegistryBase, Settings
 
-from generic import in_list
+from .generic import in_list
 
 
 class Registry(RegistryBase):
diff --git a/src/FedoraReview/__init__.py b/src/FedoraReview/__init__.py
index 908a66c..1abbba8 100644
--- a/src/FedoraReview/__init__.py
+++ b/src/FedoraReview/__init__.py
@@ -20,14 +20,14 @@
 Tools for helping Fedora package reviewers
 '''
 
-from check_base   import AbstractCheck, GenericCheck, CheckBase
-from mock         import Mock
-from review_error import ReviewError
-from review_dirs  import ReviewDirs
-from registry     import AbstractRegistry, RegistryBase
-from rpm_file     import RpmFile
-from settings     import Settings
-from version      import __version__, BUILD_ID, BUILD_DATE, BUILD_FULL
-from xdg_dirs     import XdgDirs
+from .check_base   import AbstractCheck, GenericCheck, CheckBase
+from .mock         import Mock
+from .review_error import ReviewError
+from .review_dirs  import ReviewDirs
+from .registry     import AbstractRegistry, RegistryBase
+from .rpm_file     import RpmFile
+from .settings     import Settings
+from .version      import __version__, BUILD_ID, BUILD_DATE, BUILD_FULL
+from .xdg_dirs     import XdgDirs
 
 # vim: set expandtab ts=4 sw=4:
diff --git a/src/FedoraReview/abstract_bug.py b/src/FedoraReview/abstract_bug.py
index 01bb603..6022f14 100644
--- a/src/FedoraReview/abstract_bug.py
+++ b/src/FedoraReview/abstract_bug.py
@@ -26,11 +26,11 @@ from abc import ABCMeta, abstractmethod
 from glob import glob
 from six.moves.urllib.parse import urlparse
 
-from helpers_mixin import HelpersMixin
-from review_error import ReviewError
-from settings import Settings
-from srpm_file import SRPMFile
-from review_dirs import ReviewDirs
+from .helpers_mixin import HelpersMixin
+from .review_error import ReviewError
+from .settings import Settings
+from .srpm_file import SRPMFile
+from .review_dirs import ReviewDirs
 
 
 class AbstractBug(HelpersMixin):
diff --git a/src/FedoraReview/bugzilla_bug.py b/src/FedoraReview/bugzilla_bug.py
index a71081b..25b1320 100644
--- a/src/FedoraReview/bugzilla_bug.py
+++ b/src/FedoraReview/bugzilla_bug.py
@@ -18,9 +18,9 @@ Tools for helping Fedora package reviewers
 '''
 import os.path
 
-from url_bug import UrlBug
-from settings import Settings
-from abstract_bug import AbstractBug
+from .url_bug import UrlBug
+from .settings import Settings
+from .abstract_bug import AbstractBug
 
 
 class BugzillaBug(UrlBug):
diff --git a/src/FedoraReview/checks.py b/src/FedoraReview/checks.py
index a104624..6c412f0 100644
--- a/src/FedoraReview/checks.py
+++ b/src/FedoraReview/checks.py
@@ -27,13 +27,13 @@ import time
 from operator import attrgetter
 from straight.plugin import load                  # pylint: disable=F0401
 
-from datasrc import RpmDataSource, BuildFilesSource, SourcesDataSource
-from settings import Settings
-from srpm_file import SRPMFile
-from spec_file import SpecFile
-from review_error import ReviewError
-from xdg_dirs import XdgDirs
-from reports import write_xml_report, write_template
+from .datasrc import RpmDataSource, BuildFilesSource, SourcesDataSource
+from .settings import Settings
+from .srpm_file import SRPMFile
+from .spec_file import SpecFile
+from .review_error import ReviewError
+from .xdg_dirs import XdgDirs
+from .reports import write_xml_report, write_template
 
 
 _BATCH_EXCLUDED = 'CheckBuild,CheckPackageInstalls,CheckRpmlintInstalled,' \
diff --git a/src/FedoraReview/copr_bug.py b/src/FedoraReview/copr_bug.py
index 83ac3df..eadd99e 100644
--- a/src/FedoraReview/copr_bug.py
+++ b/src/FedoraReview/copr_bug.py
@@ -27,10 +27,10 @@ import re
 import os
 import platform
 
-from url_bug import UrlBug
-from review_error import ReviewError
-from review_dirs import ReviewDirs
-from settings import Settings
+from .url_bug import UrlBug
+from .review_error import ReviewError
+from .review_dirs import ReviewDirs
+from .settings import Settings
 
 COPR_API_URL_TEMPLATE = 'https://copr.fedoraproject.org/api/coprs/build/{}/'
 
diff --git a/src/FedoraReview/datasrc.py b/src/FedoraReview/datasrc.py
index 99ac27a..cb19f13 100644
--- a/src/FedoraReview/datasrc.py
+++ b/src/FedoraReview/datasrc.py
@@ -26,10 +26,10 @@ from abc import ABCMeta, abstractmethod
 from fnmatch import fnmatch
 from glob import glob
 
-from review_dirs import ReviewDirs
-from rpm_file import RpmFile
-from settings import Settings
-from source import Source
+from .review_dirs import ReviewDirs
+from .rpm_file import RpmFile
+from .settings import Settings
+from .source import Source
 
 
 class AbstractDataSource(object):
diff --git a/src/FedoraReview/deps.py b/src/FedoraReview/deps.py
index 59a0922..145c8b3 100644
--- a/src/FedoraReview/deps.py
+++ b/src/FedoraReview/deps.py
@@ -23,7 +23,7 @@ try:
 except ImportError:
     from FedoraReview.el_compat import check_output
 
-from settings import Settings
+from .settings import Settings
 
 
 def init():
diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index 1f112a7..50cd8db 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -27,8 +27,8 @@ import urllib
 from subprocess import Popen, PIPE
 import hashlib
 
-from settings import Settings
-from review_error import ReviewError
+from .settings import Settings
+from .review_error import ReviewError
 
 
 class DownloadError(ReviewError):
diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index ead4c43..ff1b03f 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -34,10 +34,10 @@ try:
 except ImportError:
     from FedoraReview.el_compat import check_output
 
-from helpers_mixin import HelpersMixin
-from review_dirs import ReviewDirs
-from settings import Settings
-from review_error import ReviewError
+from .helpers_mixin import HelpersMixin
+from .review_dirs import ReviewDirs
+from .settings import Settings
+from .review_error import ReviewError
 
 
 _RPMLINT_SCRIPT = " mock  @config@ --chroot " \
diff --git a/src/FedoraReview/name_bug.py b/src/FedoraReview/name_bug.py
index e470b96..e3d74e7 100644
--- a/src/FedoraReview/name_bug.py
+++ b/src/FedoraReview/name_bug.py
@@ -19,14 +19,13 @@ Handles -n, srpm and spec file already downloaded
 
 import os
 import os.path
-
 from glob import glob
-from review_dirs import ReviewDirs
-from settings import Settings
 
 from six.moves.urllib.parse import urlparse
 
-from abstract_bug import AbstractBug
+from .review_dirs import ReviewDirs
+from .settings import Settings
+from .abstract_bug import AbstractBug
 
 
 class NameBug(AbstractBug):
diff --git a/src/FedoraReview/registry.py b/src/FedoraReview/registry.py
index 1e61fc2..c4ab0b7 100644
--- a/src/FedoraReview/registry.py
+++ b/src/FedoraReview/registry.py
@@ -21,9 +21,9 @@ Test module registration support
 import inspect
 
 
-from review_error import ReviewError
-from settings import Settings
-from version import __version__, BUILD_ID
+from .review_error import ReviewError
+from .settings import Settings
+from .version import __version__, BUILD_ID
 
 
 class _Flag(object):
diff --git a/src/FedoraReview/reports.py b/src/FedoraReview/reports.py
index 14b9b89..fbd78e7 100644
--- a/src/FedoraReview/reports.py
+++ b/src/FedoraReview/reports.py
@@ -26,9 +26,9 @@ import xml.dom.minidom
 
 from glob import glob
 
-from review_dirs import ReviewDirs
-from settings import Settings
-from version import __version__, BUILD_FULL
+from .review_dirs import ReviewDirs
+from .settings import Settings
+from .version import __version__, BUILD_FULL
 
 # pylint: disable=W0611
 try:
diff --git a/src/FedoraReview/review_dirs.py b/src/FedoraReview/review_dirs.py
index 9b312d8..1d610fc 100644
--- a/src/FedoraReview/review_dirs.py
+++ b/src/FedoraReview/review_dirs.py
@@ -28,8 +28,8 @@ import os.path
 import shutil
 import tempfile
 
-from review_error import ReviewError
-from settings     import Settings
+from .review_error import ReviewError
+from .settings     import Settings
 
 SRPM              = 'srpm'
 SRPM_UNPACKED     = 'srpm-unpacked'
diff --git a/src/FedoraReview/review_helper.py b/src/FedoraReview/review_helper.py
index b27d5fd..f0eab02 100644
--- a/src/FedoraReview/review_helper.py
+++ b/src/FedoraReview/review_helper.py
@@ -26,20 +26,20 @@ from __future__ import print_function
 import os.path
 import sys
 import time
-import ansi
-
-from bugzilla_bug import BugzillaBug
-from check_base import SimpleTestResult
-from checks import Checks, ChecksLister
-from mock import Mock
-from name_bug import NameBug
-from review_dirs import ReviewDirs
-from review_error import ReviewError, SpecParseReviewError
-from settings import Settings
-from url_bug import UrlBug
-from copr_bug import CoprBug
-from version import __version__, BUILD_FULL
-from reports import write_xml_report
+
+from . import ansi
+from .bugzilla_bug import BugzillaBug
+from .check_base import SimpleTestResult
+from .checks import Checks, ChecksLister
+from .mock import Mock
+from .name_bug import NameBug
+from .review_dirs import ReviewDirs
+from .review_error import ReviewError, SpecParseReviewError
+from .settings import Settings
+from .url_bug import UrlBug
+from .copr_bug import CoprBug
+from .version import __version__, BUILD_FULL
+from .reports import write_xml_report
 
 
 _EXIT_MESSAGE = """\
diff --git a/src/FedoraReview/rpm_file.py b/src/FedoraReview/rpm_file.py
index 4302ed4..66ec1e0 100644
--- a/src/FedoraReview/rpm_file.py
+++ b/src/FedoraReview/rpm_file.py
@@ -22,8 +22,8 @@ Binary rpm file management.
 import os
 import rpm
 
-from mock import Mock
-from settings import Settings
+from .mock import Mock
+from .settings import Settings
 
 
 class RpmFile(object):
diff --git a/src/FedoraReview/settings.py b/src/FedoraReview/settings.py
index bb1abb0..593b650 100644
--- a/src/FedoraReview/settings.py
+++ b/src/FedoraReview/settings.py
@@ -31,9 +31,10 @@ import subprocess
 import sys
 
 from packaging import version
-import ansi
-from review_error import ReviewError
-from xdg_dirs import XdgDirs
+
+from . import ansi
+from .review_error import ReviewError
+from .xdg_dirs import XdgDirs
 
 PARSER_SECTION = 'review'
 
diff --git a/src/FedoraReview/source.py b/src/FedoraReview/source.py
index 9c61ee3..af571ff 100644
--- a/src/FedoraReview/source.py
+++ b/src/FedoraReview/source.py
@@ -25,10 +25,10 @@ import shutil
 
 from six.moves.urllib.parse import urlparse
 
-from helpers_mixin import HelpersMixin, DownloadError
-from review_dirs import ReviewDirs
-from review_error import ReviewError
-from settings import Settings
+from .helpers_mixin import HelpersMixin, DownloadError
+from .review_dirs import ReviewDirs
+from .review_error import ReviewError
+from .settings import Settings
 
 
 class Source(HelpersMixin):
diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index 9c407b4..b0d0534 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -24,9 +24,10 @@ import sys
 import urllib
 
 import rpm
-from review_error import ReviewError, SpecParseReviewError
-from settings import Settings
-from mock import Mock
+
+from .review_error import ReviewError, SpecParseReviewError
+from .settings import Settings
+from .mock import Mock
 
 
 def _lines_in_string(s, raw):
diff --git a/src/FedoraReview/srpm_file.py b/src/FedoraReview/srpm_file.py
index 5d8c9a0..31b9b4c 100644
--- a/src/FedoraReview/srpm_file.py
+++ b/src/FedoraReview/srpm_file.py
@@ -26,9 +26,9 @@ import shutil
 from glob import glob
 from subprocess import call
 
-from helpers_mixin import HelpersMixin
-from review_dirs import ReviewDirs
-from settings import Settings
+from .helpers_mixin import HelpersMixin
+from .review_dirs import ReviewDirs
+from .settings import Settings
 
 
 class SRPMFile(HelpersMixin):
diff --git a/src/FedoraReview/url_bug.py b/src/FedoraReview/url_bug.py
index 50031d2..4d82316 100644
--- a/src/FedoraReview/url_bug.py
+++ b/src/FedoraReview/url_bug.py
@@ -22,7 +22,7 @@ import urllib
 
 from bs4 import BeautifulSoup
 
-from abstract_bug import AbstractBug
+from .abstract_bug import AbstractBug
 
 
 class UrlBug(AbstractBug):
diff --git a/src/FedoraReview/version.py b/src/FedoraReview/version.py
index 1136aa6..3adc981 100755
--- a/src/FedoraReview/version.py
+++ b/src/FedoraReview/version.py
@@ -14,7 +14,7 @@ try:
 except ImportError:
     from FedoraReview.el_compat import check_output
 
-from review_error import ReviewError
+from .review_error import ReviewError
 
 __version__ = 'Unknown'
 BUILD_FULL = 'Unknown (no version file nor git info)'

From 525e7dd4413319e6632ac6c7a1e24105b81de623 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 7/31] String output from subprocess


---

diff --git a/src/FedoraReview/settings.py b/src/FedoraReview/settings.py
index 593b650..cbc374a 100644
--- a/src/FedoraReview/settings.py
+++ b/src/FedoraReview/settings.py
@@ -60,7 +60,8 @@ def _check_mock_grp():
 def _check_mock_ver():
     """ Returns mock version """
     try:
-        mock_ver = subprocess.check_output(['mock', '--version'])
+        mock_ver = subprocess.check_output(['mock', '--version'],
+                                           universal_newlines=True)
     except subprocess.CalledProcessError:
         mock_ver = '0'
     return mock_ver

From 9a16b96181b05c27aabf7fb397cc2a07cd932894 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 8/31] urlretrieve six.moves


---

diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index 50cd8db..b92acb7 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -23,7 +23,7 @@ Tools for helping Fedora package reviewers
 import logging
 import os.path
 import re
-import urllib
+from six.moves.urllib.request import FancyURLopener
 from subprocess import Popen, PIPE
 import hashlib
 
@@ -82,7 +82,7 @@ class HelpersMixin(object):
             import socket
             socket.setdefaulttimeout(30)
 
-            istream = urllib.FancyURLopener().open(url)
+            istream = FancyURLopener().open(url)
             if istream.getcode() and istream.getcode() != 200:
                 raise DownloadError(istream.getcode(), url)
             with open(path, 'w') as ostream:
diff --git a/src/FedoraReview/url_bug.py b/src/FedoraReview/url_bug.py
index 4d82316..f3110fe 100644
--- a/src/FedoraReview/url_bug.py
+++ b/src/FedoraReview/url_bug.py
@@ -18,7 +18,7 @@ Tools handling resources identified with an url (download only).
 No xmlrpc involved, for better or worse.
 '''
 import os.path
-import urllib
+from six.moves.urllib.request import urlretrieve
 
 from bs4 import BeautifulSoup
 
@@ -45,7 +45,7 @@ class UrlBug(AbstractBug):
         if self.bug_url.startswith('file://'):
             tmpfile = self.bug_url.replace('file://', '')
         else:
-            tmpfile = urllib.urlretrieve(self.bug_url)[0]
+            tmpfile = urlretrieve(self.bug_url)[0]
         soup = BeautifulSoup(open(tmpfile), features="lxml")
         links = soup.findAll('a')
         hrefs = [l.get('href') for l in links if l.get('href')]

From 0a25f50afb75b66a88301df776a30cf37979317c Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 9/31] Work with strings, not bytes


---

diff --git a/src/FedoraReview/url_bug.py b/src/FedoraReview/url_bug.py
index f3110fe..f1ade07 100644
--- a/src/FedoraReview/url_bug.py
+++ b/src/FedoraReview/url_bug.py
@@ -51,7 +51,7 @@ class UrlBug(AbstractBug):
         hrefs = [l.get('href') for l in links if l.get('href')]
         found = []
         for href in reversed(hrefs):
-            href = href.encode('ascii', 'ignore')
+            href = href.encode('ascii', 'ignore').decode('ascii')
             if '?' in href:
                 href = href[0: href.find('?')]
             if not href.startswith('http://') and \

From e425d3c4c433ab32282a4e7fb55860e166de089d Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 10/31] Donwload files as binary


---

diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index b92acb7..efb27cf 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -85,9 +85,9 @@ class HelpersMixin(object):
             istream = FancyURLopener().open(url)
             if istream.getcode() and istream.getcode() != 200:
                 raise DownloadError(istream.getcode(), url)
-            with open(path, 'w') as ostream:
+            with open(path, 'wb') as ostream:
                 octets = istream.read(32767)
-                while octets != '':
+                while octets:
                     ostream.write(octets)
                     octets = istream.read(32767)
         except IOError as err:

From dad99a58826c875b1913810f837b4abc4f33f9f3 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 11/31] Octal Literals


---

diff --git a/src/FedoraReview/deps.py b/src/FedoraReview/deps.py
index 145c8b3..7a514b9 100644
--- a/src/FedoraReview/deps.py
+++ b/src/FedoraReview/deps.py
@@ -162,7 +162,7 @@ def list_dirs(pkg_filename):
             # E. g., when given '(contains no files)'
             continue
         mode = int(mode, 8)
-        if mode & 040000:
+        if mode & 0o40000:
             dirs.append(path)
 
 
@@ -252,7 +252,7 @@ def listpaths(pkg_filename):
             # E. g., when given '(contains no files)'
             continue
         mode = int(mode, 8)
-        if mode & 040000:
+        if mode & 0o40000:
             dirs.append(path)
         else:
             files.append(path)

From af4e6fde166cd94d2ba540015a3c241d5e346fbf Mon Sep 17 00:00:00 2001
From: Riccardo Schirone <rschiron@redhat.com>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 12/31] Make urllib uses python2&3 compatible


---

diff --git a/plugins/R.py b/plugins/R.py
index c12da03..818627d 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -7,7 +7,7 @@
 
 import re
 import os
-import urllib
+from six.moves.urllib.request import urlopen
 
 import rpm
 
@@ -56,7 +56,7 @@ class RCheckBase(CheckBase):
         version = None
         for url in self.URLS:
             try:
-                stream = urllib.urlopen(url)
+                stream = urlopen(url)
                 content = stream.read()
                 stream.close()
             except IOError as err:
diff --git a/src/FedoraReview/copr_bug.py b/src/FedoraReview/copr_bug.py
index eadd99e..8505d93 100644
--- a/src/FedoraReview/copr_bug.py
+++ b/src/FedoraReview/copr_bug.py
@@ -20,7 +20,7 @@ Tool to generate review from a COPR build.
 
 from __future__ import print_function
 
-import urllib
+from six.moves.urllib.request import urlopen
 import json
 import sys
 import re
@@ -37,7 +37,7 @@ COPR_API_URL_TEMPLATE = 'https://copr.fedoraproject.org/api/coprs/build/{}/'
 
 def get_build_data(build_id):
     """ Fetch build data from COPR and return them as json """
-    build_data_file = urllib.urlopen(
+    build_data_file = urlopen(
         COPR_API_URL_TEMPLATE.format(build_id))
     return json.load(build_data_file)
 
diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index b0d0534..f5546c5 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -21,7 +21,7 @@ Spec file management.
 
 import re
 import sys
-import urllib
+from six.moves.urllib.parse import unquote
 
 import rpm
 
@@ -166,7 +166,7 @@ class SpecFile(object):
             tag = srctype + str(num)
             try:
                 result[tag] = \
-                    self.spec.sourceHeader.format(urllib.unquote(url))
+                    self.spec.sourceHeader.format(unquote(url))
             except Exception:
                 raise SpecParseReviewError("Cannot parse %s url %s"
                                            % (tag, url))
diff --git a/test/fr_testcase.py b/test/fr_testcase.py
index c0f8819..9ef0ebb 100644
--- a/test/fr_testcase.py
+++ b/test/fr_testcase.py
@@ -28,7 +28,7 @@ import subprocess
 import sys
 import unittest2 as unittest
 
-from urllib import urlopen
+from six.moves.urllib.request import urlopen
 
 import srcpath                                   # pylint: disable=W0611
 from FedoraReview import Mock, ReviewDirs, Settings

From 0adbd1a30aa4fc52706f1f001589ec6c049f591b Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 13/31] Replace a filter call  with list comprehension


Cherry-picked from https://pagure.io/fork/ret2libc/FedoraReview/c/499ecfea3399a8c07eb74276f16262a3bfce5319?branch=python3-progresses

Co-authored-by: Riccardo Schirone <rschiron@redhat.com>

---

diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index ff1b03f..d882faa 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -298,8 +298,8 @@ class _Mock(HelpersMixin):
         '''
         pattern = '%s-%s*' % (nvr.name, nvr.version)
         paths = self._get_rpm_paths(pattern)
-        paths = filter(lambda p: p.endswith('.rpm') and
-                       not p.endswith('.src.rpm'), paths)
+        paths = [p for p in paths if p.endswith('.rpm') and
+                 not p.endswith('.src.rpm')]
         if not paths:
             raise ReviewError('No built package found for ' + nvr.name)
         elif len(paths) > 1:

From 4d21db04f0e8e710e4f37864ef5f139221445669 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 14/31] Use six.moves.input() instead of raw_input()


    python-modernize -wnf libmodernize.fixes.fix_input_six .

Rearranged the imports manually.

---

diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 5f53fc1..00703b6 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -42,6 +42,7 @@ from subprocess import Popen, check_output
 import sys
 
 import six.moves.configparser
+from six.moves import input
 from six.moves.xmlrpc_client import Fault
 
 from bugzilla.rhbugzilla import RHBugzilla
@@ -281,13 +282,13 @@ class ReviewRequest(object):
             print("\t", bug.url)
 
         if bugbz:
-            usr_inp = raw_input('Do you want to proceed anyway? [Y/N]')
+            usr_inp = input('Do you want to proceed anyway? [Y/N]')
             if usr_inp.lower() == ''  or usr_inp.lower().startswith('n'):
                 raise FedoraCreateReviewError()
 
     def login_bz(self):
         """ Login into the bugzilla. """
-        username = raw_input('Bugzilla username: ')
+        username = input('Bugzilla username: ')
         self.bzclient.login(user=username,
                             password=getpass.getpass())
 
@@ -308,7 +309,7 @@ class ReviewRequest(object):
                 self.username = read_fas_user()
             except:
                 self.log.debug('Could not determine FAS username')
-                self.username = raw_input('FAS username: ')
+                self.username = input('FAS username: ')
 
         self.bzclient = RHBugzilla(url="%s/xmlrpc.cgi" % bzurl)
         self.srpmfile = os.path.expanduser(args.srpmfile)

From 1cfe624fbbe69ba9208d8599c2c5d1964ccf41e0 Mon Sep 17 00:00:00 2001
From: Riccardo Schirone <rschiron@redhat.com>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 15/31] Other python3 compatibility fixes


---

diff --git a/plugins/R.py b/plugins/R.py
index 818627d..ea38a5c 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -159,7 +159,7 @@ class RCheckLatestVersionIsPackaged(RCheckBase):
 
     def run_on_applicable(self):
         """ Run the check """
-        cur_version = self.spec.expand_tag('Version')
+        cur_version = self.spec.expand_tag('Version').decode('utf-8')
         up_version = self.get_upstream_r_package_version()
         if up_version is None:
             self.set_passed(
diff --git a/plugins/ccpp.py b/plugins/ccpp.py
index 1cda556..4159066 100644
--- a/plugins/ccpp.py
+++ b/plugins/ccpp.py
@@ -16,7 +16,7 @@ class Registry(RegistryBase):
         if self.is_user_enabled():
             return self.user_enabled_value()
         archs = self.checks.spec.expand_tag('BuildArchs')
-        if len(archs) == 1 and archs[0].lower() == 'noarch':
+        if len(archs) == 1 and archs[0].lower() == b'noarch':
             return False
         if self.checks.buildsrc.is_available:
             src = self.checks.buildsrc
diff --git a/plugins/generic.py b/plugins/generic.py
index 8535876..3bcc364 100644
--- a/plugins/generic.py
+++ b/plugins/generic.py
@@ -144,7 +144,7 @@ class CheckBuildCompilerFlags(GenericCheckBase):
 
     def run(self):
         archs = self.checks.spec.expand_tag('BuildArchs')
-        if len(archs) == 1 and archs[0].lower() == 'noarch':
+        if len(archs) == 1 and archs[0].lower() == b'noarch':
             self.set_passed(self.NA)
             return
         self.set_passed(self.PENDING)
@@ -253,7 +253,7 @@ class CheckDaemonCompileFlags(GenericCheckBase):
         try:
             cmd = 'find %s -perm -4000' % rpms_dir
             with open('/dev/null', 'w') as f:
-                suids = check_output(cmd.split(), stderr=f).strip().split()
+                suids = check_output(cmd.split(), stderr=f, universal_newlines=True).strip().split()
         except CalledProcessError:
             self.log.info('Cannot run find command: %s', cmd)
             suids = []
@@ -321,9 +321,9 @@ class CheckDescMacros(GenericCheckBase):
     def run(self):
         bad_tags = []
         for pkg_name in self.spec.packages:
-            if '%' in self.spec.expand_tag('Summary', pkg_name):
+            if '%' in self.spec.expand_tag('Summary', pkg_name).decode('utf-8'):
                 bad_tags.append(pkg_name + ' (summary)')
-            if '%' in self.spec.expand_tag('Description', pkg_name):
+            if '%' in self.spec.expand_tag('Description', pkg_name).decode('utf-8'):
                 bad_tags.append(pkg_name + ' (description)')
         if bad_tags:
             self.set_passed(self.PENDING,
@@ -654,7 +654,7 @@ class CheckLicenseField(GenericCheckBase):
             if os.path.exists(source_dir):
                 cmd = 'licensecheck -r ' + source_dir
                 try:
-                    out = check_output(cmd, shell=True)
+                    out = check_output(cmd, shell=True, universal_newlines=True)
                 except (OSError, CalledProcessError) as err:
                     self.set_passed(self.PENDING,
                                     "Cannot run licensecheck: " + str(err))
@@ -733,7 +733,7 @@ class CheckLicensInDoc(GenericCheckBase):
             nvr = self.spec.get_package_nvr(pkg)
             rpm_path = Mock.get_package_rpm_path(nvr)
             cmd = 'rpm -qp%s %s' % (self._license_flag, rpm_path)
-            doclist = check_output(cmd.split())
+            doclist = check_output(cmd.split(), universal_newlines=True)
             flagged_files.extend(doclist.split('\n'))
         flagged_files = map(lambda f: f.split('/')[-1], flagged_files)
 
@@ -742,7 +742,7 @@ class CheckLicensInDoc(GenericCheckBase):
                 nvr = self.spec.get_package_nvr(pkg)
                 rpm_path = Mock.get_package_rpm_path(nvr)
                 cmd = 'rpm -qpL %s' % rpm_path
-                qpL_list = check_output(cmd.split())
+                qpL_list = check_output(cmd.split(), universal_newlines=True)
                 qpL_files.extend(qpL_list.split('\n'))
             qpL_files = map(lambda f: f.split('/')[-1], qpL_files)
 
@@ -871,7 +871,7 @@ class CheckMultipleLicenses(GenericCheckBase):
         self.type = 'MUST'
 
     def is_applicable(self):
-        license_ = self.spec.expand_tag('License').lower().split()
+        license_ = self.spec.expand_tag('License').decode('utf-8').lower().split()
         return 'and' in license_ or 'or' in license_
 
 
@@ -1214,7 +1214,7 @@ class CheckSourceMD5(GenericCheckBase):
             cmd = '/usr/bin/diff -U2 -r %s %s' % (upstream, local)
             self.log.debug(' Diff cmd: %s', cmd)
             try:
-                p = Popen(cmd.split(), stdout=PIPE, stderr=PIPE)
+                p = Popen(cmd.split(), stdout=PIPE, stderr=PIPE, universal_newlines=True)
                 output = p.communicate()[0]
             except OSError:
                 self.log.error("Cannot run diff", exc_info=True)
diff --git a/plugins/generic_autotools.py b/plugins/generic_autotools.py
index be68aff..47d3732 100644
--- a/plugins/generic_autotools.py
+++ b/plugins/generic_autotools.py
@@ -171,7 +171,8 @@ class CheckAutotoolsObsoletedMacros(AutotoolsCheckBase):
 
             try:
                 self.log.debug("running: %s", ' '.join(cmd))
-                p = Popen(cmd, stdout=PIPE, stderr=PIPE)
+                p = Popen(cmd, stdout=PIPE, stderr=PIPE,
+                          universal_newlines=True)
                 stdout, stderr = p.communicate()
             except IOError:
                 self.set_passed(self.PENDING,
diff --git a/plugins/generic_build.py b/plugins/generic_build.py
index f3c0433..1be0ab5 100644
--- a/plugins/generic_build.py
+++ b/plugins/generic_build.py
@@ -34,13 +34,6 @@ import subprocess
 import sys
 import tempfile
 
-# pylint: disable=W0611
-try:
-    from subprocess import check_output
-except ImportError:
-    from FedoraReview.el_compat import check_output
-# pylint: enable=W0611
-
 
 import FedoraReview.deps as deps
 from FedoraReview import CheckBase, Mock, ReviewDirs, Settings
diff --git a/plugins/generic_should.py b/plugins/generic_should.py
index c67bc2d..c9cab2e 100644
--- a/plugins/generic_should.py
+++ b/plugins/generic_should.py
@@ -254,7 +254,7 @@ class CheckFullVerReqSub(GenericShouldCheckBase):
         if len(self.spec.packages) == 1:
             self.set_passed(self.NA)
             return
-        if len(archs) == 1 and archs[0].lower() == 'noarch':
+        if len(archs) == 1 and archs[0].lower() == b'noarch':
             isa = ''
         else:
             isa = Mock.get_macro('%_isa', self.spec, self.flags)
@@ -294,7 +294,7 @@ class CheckIllegalSpecTags(GenericShouldCheckBase):
         for tag in ('Packager', 'Vendor', 'PreReq', 'Copyright'):
             if not self.spec.find_re(r'^\s*' + tag + r'\s*:'):
                 continue
-            value = self.spec.expand_tag(tag)
+            value = self.spec.expand_tag(tag).decode('utf-8')
             if value:
                 passed = False
                 output += 'Found : %s: %s\n' % (tag, value)
@@ -544,7 +544,7 @@ class CheckSpecAsInSRPM(GenericShouldCheckBase):
         url_spec_file = self.spec.filename
         cmd = ["diff", '-U2', url_spec_file, srpm_spec_file]
         try:
-            p = Popen(cmd, stdout=PIPE, stderr=PIPE)
+            p = Popen(cmd, stdout=PIPE, stderr=PIPE, universal_newlines=True)
             output = p.communicate()[0]
         except OSError:
             self.log.error("Cannot run diff", exc_info=True)
@@ -596,7 +596,7 @@ class CheckSupportAllArchs(GenericShouldCheckBase):
         build_ok = self.checks.checkdict['CheckBuild'].is_passed
 
         arch = self.spec.expand_tag('BuildArch')
-        noarch = arch and arch[0].lower() == 'noarch'
+        noarch = arch and arch[0].lower() == b'noarch'
         one_arch = self.spec.expand_tag('ExclusiveArch')
         if build_ok and (one_arch or noarch):
             self.set_passed(self.PASS)
diff --git a/plugins/ruby.py b/plugins/ruby.py
index 7c061fb..8420eaf 100644
--- a/plugins/ruby.py
+++ b/plugins/ruby.py
@@ -127,7 +127,7 @@ class RubyCheckBuildArchitecture(RubyCheckBase):
         self.automatic = True
 
     def run_on_applicable(self):
-        arch = self.spec.expand_tag('arch')
+        arch = self.spec.expand_tag('arch').decode('utf-8')
         if _has_extension(self):
             self.set_passed('noarch' not in arch,
                             "Package with binary extension can't be built"
diff --git a/plugins/shell_api.py b/plugins/shell_api.py
index 4686b85..fb675c3 100644
--- a/plugins/shell_api.py
+++ b/plugins/shell_api.py
@@ -248,7 +248,7 @@ def _write_section(spec, env, s):
 
 def _write_tag(spec, env, tag):
     ''' Substitute a spec tag into env. '''
-    value = spec.expand_tag(tag.upper())
+    value = spec.expand_tag(tag.upper()).decode('utf-8')
     if value:
         env = env.replace('@' + tag + '@', value)
     else:
@@ -432,7 +432,7 @@ class ShellCheck(GenericCheck):
         Actually invoke the external script, returning
         (retcode, stdout, stderr)
         '''
-        p = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE)
+        p = Popen(cmd, shell=True, stdout=PIPE, stderr=PIPE, universal_newlines=True)
         try:
             stdout, stderr = p.communicate()
         except OSError:
diff --git a/src/FedoraReview/check_base.py b/src/FedoraReview/check_base.py
index d918e41..634618b 100644
--- a/src/FedoraReview/check_base.py
+++ b/src/FedoraReview/check_base.py
@@ -53,6 +53,24 @@ class _Attachment(object):
         s += self.text
         return s
 
+    def __eq__(self, other):
+        return self.__cmp__(other) == 0
+
+    def __ne__(self, other):
+        return self.__cmp__(other) != 0
+
+    def __lt__(self, other):
+        return self.__cmp__(other) < 0
+
+    def __le__(self, other):
+        return self.__cmp__(other) <= 0
+
+    def __gt__(self, other):
+        return self.__cmp__(other) > 0
+
+    def __ge__(self, other):
+        return self.__cmp__(other) >= 0
+
     def __cmp__(self, other):
         if not hasattr(other, 'order_hint'):
             return NotImplemented
diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 00703b6..2001445 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -241,7 +241,7 @@ class ReviewRequest(object):
         cmd = ['koji', 'build', '--scratch', target, self.srpmfile]
         self.log.debug(cmd)
         try:
-            proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
             output = proc.communicate()[0]
         except OSError as err:
             print("OSError : %s" % str(err))
@@ -368,7 +368,7 @@ class ReviewRequest(object):
                self.username + "@" + self.settings.upload_target]
         self.log.debug(cmd)
         try:
-            proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            proc = Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
             output = proc.communicate()[0]
         except OSError as err:
             print("OSError : %s" % str(err))
diff --git a/src/FedoraReview/deps.py b/src/FedoraReview/deps.py
index 7a514b9..93c5f3f 100644
--- a/src/FedoraReview/deps.py
+++ b/src/FedoraReview/deps.py
@@ -58,7 +58,7 @@ def list_deps(pkgs):
            ' '.join(list(set(pkgs)))]
     Settings.get_logger().debug("Running: %s", ' '.join(cmd))
     try:
-        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run %s", " ".join(cmd))
         return []
@@ -85,14 +85,14 @@ def list_provides(pkgs):
            + ' '.join(list(set(pkgs)))]
     Settings.get_logger().debug("Running: %s", ' '.join(cmd))
     try:
-        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run %s", " ".join(cmd))
         return []
     provides = []
     while True:
         try:
-            line = dnf.stdout.next().strip()
+            line = next(dnf.stdout).strip()
         except StopIteration:
             return list(set(provides))
         provides.append(line)
@@ -121,7 +121,7 @@ def resolve_one(req):
     Settings.get_logger().debug("Running: %s", ' '.join(cmd))
 
     try:
-        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        dnf = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run %s", " ".join(cmd))
         return []
@@ -146,7 +146,7 @@ def list_dirs(pkg_filename):
 
     cmd = ['rpm', '-ql', '--dump', '-p', pkg_filename]
     try:
-        rpm = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        rpm = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run %s", " ".join(cmd))
         return []
@@ -180,7 +180,8 @@ def list_owners(paths):
                               '--enable-network', 'shell',
                               'rpm --qf %{NAME}\n -qf ' + path],
                              stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE)
+                             stderr=subprocess.PIPE,
+                             universal_newlines=True)
         path_owners = p.communicate()[0].split()
         if p.returncode != 0:
             continue
@@ -197,7 +198,7 @@ def list_owners(paths):
                'shell', 'dnf repoquery -C --quiet --file ' + path]
         Settings.get_logger().debug("Running: %s", ' '.join(cmd))
         try:
-            lines = check_output(cmd).split()
+            lines = check_output(cmd, universal_newlines=True).split()
             lines = [l.strip() for l in lines]
             if not lines or not lines[0]:
                 continue
@@ -222,7 +223,7 @@ def list_paths(pkgs):
 
     Settings.get_logger().debug("Running: %s", ' '.join(cmd))
     try:
-        paths = check_output(cmd)
+        paths = check_output(cmd, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run dnf repoquery")
         return []
@@ -235,7 +236,7 @@ def listpaths(pkg_filename):
     cmd = ['rpm', '-ql', '--dump', '--nosignature', '-p', pkg_filename]
     Settings.get_logger().debug("Running: %s", ' '.join(cmd))
     try:
-        rpm = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        rpm = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
     except OSError:
         Settings.get_logger().warning("Cannot run %s", " ".join(cmd))
         return []
diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index efb27cf..f84ba4b 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -51,7 +51,7 @@ class HelpersMixin(object):
         ''' Run a command using using subprocess, return output. '''
         self.log.debug(header + ': ' + cmd)
         cmd = cmd.split(' ')
-        proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
+        proc = Popen(cmd, stdout=PIPE, stderr=PIPE, universal_newlines=True)
         output, error = '', 'undefined'
         try:
             output, error = proc.communicate()
@@ -70,7 +70,7 @@ class HelpersMixin(object):
         '''
         ck = hashlib.new(Settings.checksum)
         with open(path, 'rb') as f:
-            for chunk in iter(lambda: f.read(8192), ''):
+            for chunk in iter(lambda: f.read(8192), b''):
                 ck.update(chunk)
         return ck.hexdigest()
 
@@ -116,7 +116,7 @@ class HelpersMixin(object):
         """
         cmd = 'rpmdev-extract -qC ' + extract_dir + ' ' + archive
         cmd += ' &>/dev/null'
-        p = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=True)
+        p = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=True, universal_newlines=True)
         stdout, stderr = p.communicate()
         if p.returncode != 0:
             log = Settings.get_logger()
diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index d882faa..c269ae9 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -47,7 +47,7 @@ _RPMLINT_SCRIPT = " mock  @config@ --chroot " \
 def _run_script(script):
     """ Run a script,  return (ok, output). """
     try:
-        p = Popen(script, stdout=PIPE, stderr=STDOUT, shell=True)
+        p = Popen(script, stdout=PIPE, stderr=STDOUT, shell=True, universal_newlines=True)
         output, error = p.communicate()
     except OSError as e:
         return False, e.strerror + ' stderr: ' + error
@@ -143,7 +143,7 @@ class _Mock(HelpersMixin):
         macros = _add_disttag_macros({}, tag)
         buildarch, macros = _add_buildarch_macros(macros, paths)
         try:
-            _arch = check_output('rpm --eval %_arch'.split()).strip()
+            _arch = check_output('rpm --eval %_arch'.split(), universal_newlines=True).strip()
         except CalledProcessError:
             raise ReviewError("Can't evaluate 'rpm --eval %_arch")
         if buildarch == 'x86_64' and _arch != 'x86_64':
@@ -217,7 +217,7 @@ class _Mock(HelpersMixin):
         header = header if header else ""
         self.log.debug(header + ' command: ' + ', '.join(cmd))
         try:
-            p = Popen(cmd, stdout=PIPE, stderr=STDOUT)
+            p = Popen(cmd, stdout=PIPE, stderr=STDOUT, universal_newlines=True)
             output, error = p.communicate()
             logging.debug(log_text(output, error), exc_info=True)
         except OSError:
@@ -235,7 +235,7 @@ class _Mock(HelpersMixin):
         cmd = self._mock_cmd()
         cmd.extend(['-q', '--chroot', '--', 'rpm --eval %_topdir'])
         try:
-            self._topdir = check_output(cmd).strip()
+            self._topdir = check_output(cmd, universal_newlines=True).strip()
             self.log.debug("_topdir: %s", str(self._topdir))
         except (CalledProcessError, OSError):
             self.log.info("Cannot evaluate %topdir in mock, using"
@@ -260,7 +260,7 @@ class _Mock(HelpersMixin):
         ''' Run rpm --eval <arg> inside mock, return output. '''
         cmd = self._mock_cmd()
         cmd.extend(['--quiet', '--chroot', '--', 'rpm --eval "' + arg + '"'])
-        return check_output(cmd).decode('utf-8').strip()
+        return check_output(cmd, universal_newlines=True).strip()
 
 # Last (cached?) output from rpmlint, list of lines.
     rpmlint_output = property(_get_rpmlint_output)
diff --git a/src/FedoraReview/reports.py b/src/FedoraReview/reports.py
index fbd78e7..e41ab45 100644
--- a/src/FedoraReview/reports.py
+++ b/src/FedoraReview/reports.py
@@ -72,7 +72,7 @@ def _get_specfile():
     if len(spec) != 1:
         return '?', '?'
     path = spec[0].strip()
-    cs = check_output(['sha224sum', path]).split()[0]
+    cs = check_output(['sha224sum', path], universal_newlines=True).split()[0]
     path = path.rsplit('/', 1)[1]
     return path, cs
 
@@ -182,7 +182,10 @@ def write_xml_report(spec, results):
 
         def txt2utf(txt):
             ''' Convert txt to utf-8. '''
-            return unicode(txt, encoding='utf-8', errors='replace')
+            if isinstance(txt, bytes):
+                return txt.decode('utf-8', errors='replace')
+
+            return txt
 
         path = root.find('metadata/file').attrib['given-path']
         results = root.find('results')
@@ -203,5 +206,5 @@ def write_xml_report(spec, results):
             root = add_xml_result(root, result)
     dom = xml.dom.minidom.parseString(ET.tostring(root))
     prettyxml = dom.toprettyxml(indent='    ', encoding='utf-8')
-    with open('report.xml', 'w') as f:
+    with open('report.xml', 'wb') as f:
         f.write(prettyxml)
diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index f5546c5..1c97034 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -86,12 +86,12 @@ class SpecFile(object):
         parse_spec()
         sys.stdout = stdout
         self._packages = None
-        self.name_vers_rel = [self.expand_tag(rpm.RPMTAG_NAME),
-                              self.expand_tag(rpm.RPMTAG_VERSION),
+        self.name_vers_rel = [self.expand_tag(rpm.RPMTAG_NAME).decode('utf-8'),
+                              self.expand_tag(rpm.RPMTAG_VERSION).decode('utf-8'),
                               '*']
         update_macros()
         parse_spec()
-        self.name_vers_rel[2] = self.expand_tag(rpm.RPMTAG_RELEASE)
+        self.name_vers_rel[2] = self.expand_tag(rpm.RPMTAG_RELEASE).decode('utf-8')
 
     name = property(lambda self: self.name_vers_rel[0])
     version = property(lambda self: self.name_vers_rel[1])
@@ -265,12 +265,14 @@ class SpecFile(object):
     @property
     def build_requires(self):
         ''' Return the list of build requirements. '''
-        return self.spec.sourceHeader[rpm.RPMTAG_REQUIRES]
+        return [x.decode('utf-8')
+                for x in self.spec.sourceHeader[rpm.RPMTAG_REQUIRES]]
 
     def get_requires(self, pkg_name=None):
         ''' Return list of requirements i. e., Requires: '''
         package = self._get_pkg_by_name(pkg_name)
-        return package.header[rpm.RPMTAG_REQUIRES]
+        return [x.decode('utf-8')
+                for x in package.header[rpm.RPMTAG_REQUIRES]]
 
     def get_package_nvr(self, pkg_name=None):
         ''' Return object with name, version, release for a package. '''
@@ -282,9 +284,9 @@ class SpecFile(object):
 
         package = self._get_pkg_by_name(pkg_name)
         nvr = NVR()
-        nvr.version = package.header[rpm.RPMTAG_VERSION]
-        nvr.release = package.header[rpm.RPMTAG_RELEASE]
-        nvr.name = package.header[rpm.RPMTAG_NAME]
+        nvr.version = package.header[rpm.RPMTAG_VERSION].decode('utf-8')
+        nvr.release = package.header[rpm.RPMTAG_RELEASE].decode('utf-8')
+        nvr.name = package.header[rpm.RPMTAG_NAME].decode('utf-8')
         return nvr
 
     def get_files(self, pkg_name=None):
@@ -293,8 +295,8 @@ class SpecFile(object):
         '''
         try:
             files = self._get_pkg_by_name(pkg_name).fileList
-            return \
-                [l for l in [f.strip() for f in files.split('\n')] if l]
+            return [l for l in [f.decode('utf-8').strip()
+                                for f in files.split(b'\n')] if l]
         except AttributeError:
             # No fileList attribute...
             # https://bugzilla.redhat.com/show_bug.cgi?id=857653
diff --git a/src/FedoraReview/version.py b/src/FedoraReview/version.py
index 3adc981..91f623c 100755
--- a/src/FedoraReview/version.py
+++ b/src/FedoraReview/version.py
@@ -30,7 +30,7 @@ class VersionError(ReviewError):
 def _setup():
     ''' Setup  the 'version' file from version.tmpl. '''
     try:
-        octets = check_output(['git', 'log', '--pretty=format:%h %ci', '-1'])
+        octets = check_output(['git', 'log', '--pretty=format:%h %ci', '-1'], universal_newlines=True)
     except:
         raise VersionError("No version file and git not available.")
     line = octets.decode('utf-8')
diff --git a/test/test_dist.py b/test/test_dist.py
index bb60ad1..f381f8c 100644
--- a/test/test_dist.py
+++ b/test/test_dist.py
@@ -56,7 +56,7 @@ class TestDist(FR_TestCase):
         self.assertEqual(len(glob('dist/*')), 4)
         lint = check_output(
                     'rpmlint -f test/rpmlint.conf dist/*spec dist/*rpm',
-                    shell=True)
+                    shell=True, universal_newlines=True)
         self.assertIn('0 error', lint)
         self.assertIn('0 warning', lint)
 
diff --git a/test/test_misc.py b/test/test_misc.py
index 781029e..d14c055 100644
--- a/test/test_misc.py
+++ b/test/test_misc.py
@@ -530,11 +530,11 @@ class TestMisc(FR_TestCase):
         self.assertEqual(spec.version, '1.0')
         dist = Mock.get_macro('%dist', None, {})
         self.assertEqual(spec.release, '1' + dist)
-        self.assertEqual(spec.expand_tag('Release'), '1' + dist)
-        self.assertEqual(spec.expand_tag('License'), 'GPLv2+')
-        self.assertEqual(spec.expand_tag('Group'), 'Development/Languages')
+        self.assertEqual(spec.expand_tag('Release').decode('utf-8'), '1' + dist)
+        self.assertEqual(spec.expand_tag('License').decode('utf-8'), 'GPLv2+')
+        self.assertEqual(spec.expand_tag('Group').decode('utf-8'), 'Development/Languages')
         # Test rpm value not there
-        self.assertEqual(spec.expand_tag('PreReq'), None)
+        self.assertEqual(spec.expand_tag('PreReq').decode('utf-8'), None)
         # Test get sections
         expected = ['rm -rf $RPM_BUILD_ROOT']
         self.assertEqual(spec.get_section('%clean'), expected)

From 997133bb4ad7d9ab9ac23c834c580787b4c04ddd Mon Sep 17 00:00:00 2001
From: Riccardo Schirone <rschiron@redhat.com>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 16/31] Use compregensions instead of map/reduce/filter + various bytes/str fixes


---

diff --git a/plugins/generic.py b/plugins/generic.py
index 3bcc364..cc3839d 100644
--- a/plugins/generic.py
+++ b/plugins/generic.py
@@ -720,9 +720,8 @@ class CheckLicensInDoc(GenericCheckBase):
                               'COPYRIGHT', 'copyright']:
             pattern = '*' + potentialfile + '*'
             licenses.extend(self.rpms.find_all(pattern))
-        licenses = filter(lambda l: not self.rpms.find(l + '/*'),
-                          licenses)
-        licenses = map(lambda f: f.split('/')[-1], licenses)
+        licenses = [l.split('/')[-1] for l in licenses
+                    if not self.rpms.find(l + '/*')]
         if licenses == []:
             self.set_passed(self.PENDING)
             return
@@ -735,7 +734,7 @@ class CheckLicensInDoc(GenericCheckBase):
             cmd = 'rpm -qp%s %s' % (self._license_flag, rpm_path)
             doclist = check_output(cmd.split(), universal_newlines=True)
             flagged_files.extend(doclist.split('\n'))
-        flagged_files = map(lambda f: f.split('/')[-1], flagged_files)
+        flagged_files = [f.split('/')[-1] for f in flagged_files]
 
         if self._license_flag == 'L':
             for pkg in self.spec.packages:
@@ -744,7 +743,7 @@ class CheckLicensInDoc(GenericCheckBase):
                 cmd = 'rpm -qpL %s' % rpm_path
                 qpL_list = check_output(cmd.split(), universal_newlines=True)
                 qpL_files.extend(qpL_list.split('\n'))
-            qpL_files = map(lambda f: f.split('/')[-1], qpL_files)
+            qpL_files = [f.split('/')[-1] for f in qpL_files]
 
         for _license in licenses:
             if self._license_flag == 'L' and _license not in qpL_files:
diff --git a/plugins/generic_autotools.py b/plugins/generic_autotools.py
index 47d3732..cd32d25 100644
--- a/plugins/generic_autotools.py
+++ b/plugins/generic_autotools.py
@@ -50,7 +50,7 @@ _OBSOLETE_CHECKS = {
 def _prepend_indent(text):
     ''' add the paragraph indentation '''
     lines = text.splitlines()
-    return '\n'.join(map(lambda x: "  " + x if x != "" else "", lines))
+    return '\n'.join(["  " + x if x != "" else "" for x in lines])
 
 
 class Registry(RegistryBase):
@@ -186,7 +186,7 @@ class CheckAutotoolsObsoletedMacros(AutotoolsCheckBase):
                 return
 
             m4_lines = stdout.splitlines(False)
-            m4_lines = map(lambda x: x.strip('('), m4_lines)
+            m4_lines = [x.strip('(') for x in m4_lines]
             for m4_line in m4_lines:
                 line, m4 = m4_line.split(':')
 
diff --git a/plugins/generic_build.py b/plugins/generic_build.py
index 1be0ab5..1991045 100644
--- a/plugins/generic_build.py
+++ b/plugins/generic_build.py
@@ -65,7 +65,7 @@ class BuildCheckBase(CheckBase):
         arg: filenames, list of filenames to run rpmlint on
         """
         rpmlint_cfg = tempfile.NamedTemporaryFile(delete=False)
-        rpmlint_cfg.write('addFilter("unknown-key")')
+        rpmlint_cfg.write(b'addFilter("unknown-key")')
         rpmlint_cfg.close()
 
         cmd = 'rpmlint -f ' + rpmlint_cfg.name + ' ' + ' '.join(filenames)
diff --git a/plugins/generic_should.py b/plugins/generic_should.py
index c9cab2e..82b1c14 100644
--- a/plugins/generic_should.py
+++ b/plugins/generic_should.py
@@ -175,8 +175,8 @@ class CheckFileRequires(GenericShouldCheckBase):
 
         def get_requires(rpm_name, requires):
             ''' Return printable requirements for a rpm. '''
-            requires = filter(lambda s: 'rpmlib' not in s, requires)
-            requires = filter(lambda s: 'GLIBC' not in s, requires)
+            requires = [s for s in requires
+                        if 'rpmlib' not in s and 'GLIBC' not in s]
             requires = sorted(list(set(requires)))
             hdr = rpm_name + ' (rpmlib, GLIBC filtered):'
             requires.insert(0, hdr)
diff --git a/src/FedoraReview/checks.py b/src/FedoraReview/checks.py
index 6c412f0..c399a0f 100644
--- a/src/FedoraReview/checks.py
+++ b/src/FedoraReview/checks.py
@@ -327,7 +327,7 @@ class Checks(_ChecksLoader):
     def _get_ready_to_run(self):
         ''' Return checks ready to run, deprecating checks first. '''
         names = list(self.checkdict.keys())
-        tests_to_run = filter(self._ready_to_run, names)
+        tests_to_run = [n for n in names if self._ready_to_run(n)]
         return sorted(tests_to_run,
                       key=lambda t: len(self.checkdict[t].deprecates),
                       reverse=True)
diff --git a/src/FedoraReview/deps.py b/src/FedoraReview/deps.py
index 93c5f3f..bffc951 100644
--- a/src/FedoraReview/deps.py
+++ b/src/FedoraReview/deps.py
@@ -65,7 +65,7 @@ def list_deps(pkgs):
     deps = []
     while True:
         try:
-            line = dnf.stdout.next().strip()
+            line = next(dnf.stdout).strip()
         except StopIteration:
             return list(set(deps))
         name = line.rsplit('.', 2)[0]
@@ -129,7 +129,7 @@ def resolve_one(req):
     pkgs = []
     while True:
         try:
-            line = dnf.stdout.next().strip()
+            line = next(dnf.stdout).strip()
         except StopIteration:
             return list(set(pkgs))
 
@@ -153,7 +153,7 @@ def list_dirs(pkg_filename):
     dirs = []
     while True:
         try:
-            line = rpm.stdout.next().strip()
+            line = next(rpm.stdout).strip()
         except StopIteration:
             return dirs
         try:
@@ -244,7 +244,7 @@ def listpaths(pkg_filename):
     dirs = []
     while True:
         try:
-            line = rpm.stdout.next().strip()
+            line = next(rpm.stdout).strip()
         except StopIteration:
             return dirs, files
         try:
diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index f84ba4b..bcbd18e 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -135,11 +135,9 @@ class HelpersMixin(object):
 
         problems = re.compile(r'(\d+)\serrors\,\s(\d+)\swarnings')
         lines = out.split('\n')[:-1]
-        err_lines = filter(lambda l: l.lower().find('error') != -1,
-                           lines)
+        err_lines = [l for l in lines if l.lower().find('error') != -1]
         if not err_lines:
-            Settings.get_logger().debug('Cannot parse rpmlint output: %s',
-                                        out)
+            Settings.get_logger().debug('Cannot parse rpmlint output: %s', out)
             return False, 'Cannot parse rpmlint output:'
 
         res = problems.search(err_lines[-1])
diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index c269ae9..fe069e3 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -484,7 +484,7 @@ class _Mock(HelpersMixin):
         cmd.append('--init')
         self._run_cmd(cmd, '--init')
         cmd = self._mock_cmd()
-        rpms = filter(is_not_installed, list(set(packages)))
+        rpms = [p for p in set(packages) if is_not_installed(p)]
         if not rpms:
             return None
         self._clear_rpm_db()
diff --git a/src/FedoraReview/registry.py b/src/FedoraReview/registry.py
index c4ab0b7..d1e4461 100644
--- a/src/FedoraReview/registry.py
+++ b/src/FedoraReview/registry.py
@@ -43,9 +43,11 @@ class _Flag(object):
         self.defined_in = defined_in
         self.value = False
 
-    def __nonzero__(self):
+    def __bool__(self):
         return bool(self.value)
 
+    __nonzero__ = __bool__
+
     def __str__(self):
         return self.value if self.value else ''
 
diff --git a/src/FedoraReview/reports.py b/src/FedoraReview/reports.py
index e41ab45..4d89408 100644
--- a/src/FedoraReview/reports.py
+++ b/src/FedoraReview/reports.py
@@ -100,7 +100,7 @@ def _write_section(results, output):
     groups = list(set([hdr(test.group) for test in results]))
     res = None
     for group in sorted(groups):
-        res = filter(lambda t: hdr(t.group) == group, results)
+        res = [r for r in results if hdr(r.group) == group]
         if not res:
             continue
         res = sorted(res, key=result_key)
@@ -133,15 +133,15 @@ def write_template(output, results, issues, attachments):
         results = [r for r in results if r not in issues]
 
     output.write("\n\n===== MUST items =====\n")
-    musts = filter(lambda r: r.type == 'MUST', results)
+    musts = [r for r in results if r.type == 'MUST']
     _write_section(musts, output)
 
     output.write("\n===== SHOULD items =====\n")
-    shoulds = filter(lambda r: r.type == 'SHOULD', results)
+    shoulds = [r for r in results if r.type == 'SHOULD']
     _write_section(shoulds, output)
 
     output.write("\n===== EXTRA items =====\n")
-    extras = filter(lambda r: r.type == 'EXTRA', results)
+    extras = [r for r in results if r.type == 'EXTRA']
     _write_section(extras, output)
 
     for a in sorted(attachments):
diff --git a/src/FedoraReview/review_helper.py b/src/FedoraReview/review_helper.py
index f0eab02..f531337 100644
--- a/src/FedoraReview/review_helper.py
+++ b/src/FedoraReview/review_helper.py
@@ -143,12 +143,12 @@ class ReviewHelper(object):
             ''' print filename + flags and checks defined in it. '''
             for f in sorted(files):
                 print('File:  ' + f)
-                flags_by_src = filter(lambda c: c.defined_in == f,
-                                      checks_lister.flags.values())
+                flags_by_src = [c for c in checks_lister.flags.values()
+                                if c.defined_in == f]
                 for flag in flags_by_src:
                     print('Flag: ' + flag.name)
-                files_per_src = filter(lambda c: c.defined_in == f,
-                                       checks_list)
+                files_per_src = [c for c in checks_list
+                                 if c.defined_in == f]
                 groups = list(set([c.group for c in files_per_src]))
                 for group in sorted(groups):
 
@@ -156,7 +156,7 @@ class ReviewHelper(object):
                         ''' check in correct group and file? '''
                         return c.group == group and c.defined_in == f
 
-                    checks = filter(check_match, checks_list)
+                    checks = [c for c in checks_list if check_match(c)]
                     if checks == []:
                         continue
                     print('Group: ' + group)
@@ -168,15 +168,14 @@ class ReviewHelper(object):
         checks_list = list(checks_lister.get_checks().values())
         files = list(set([c.defined_in for c in checks_list]))
         list_data_by_file(files, checks_list)
-        deps_list = filter(lambda c: c.needs != [] and
-                           c.needs != ['CheckBuildCompleted'],
-                           checks_list)
+        deps_list = [c for c in checks_list
+                     if c.needs != [] and c.needs != ['CheckBuildCompleted']]
         for dep in deps_list:
             print('Dependencies: ' + dep.name + ': ' +
                   os.path.basename(dep.defined_in))
             for needed in dep.needs:
                 print('     ' + needed)
-        deprecators = filter(lambda c: c.deprecates != [], checks_list)
+        deprecators = [c for c in checks_list if c.deprecates != []]
         for dep in deprecators:
             print('Deprecations: ' + dep.name + ': ' +
                   os.path.basename(dep.defined_in))
diff --git a/src/FedoraReview/rpm_file.py b/src/FedoraReview/rpm_file.py
index 66ec1e0..de5abe2 100644
--- a/src/FedoraReview/rpm_file.py
+++ b/src/FedoraReview/rpm_file.py
@@ -52,9 +52,9 @@ class RpmFile(object):
         eq = rpm.RPMSENSE_EQUAL
         for ix in range(0, len(names) - 1):
             if not versions[ix]:
-                dp.append(names[ix])
+                dp.append(names[ix].decode('utf-8'))
                 continue
-            s = names[ix] + ' '
+            s = names[ix].decode('utf-8') + ' '
             if sense[ix] and eq:
                 s += '= '
             elif sense[ix] and lt:
@@ -64,7 +64,7 @@ class RpmFile(object):
             else:
                 s += 'What? : ' + bin(sense[ix])
                 self.log.warning("Bad RPMSENSE: %s", bin(sense[ix]))
-            s += versions[ix]
+            s += versions[ix].decode('utf-8')
             dp.append(s)
         return dp
 
@@ -86,9 +86,11 @@ class RpmFile(object):
     def header_to_str(self, tag):
         ''' Convert header in a string, to cope with API changes in F18'''
         if isinstance(self.header[tag], (dict, list)):
-            return ' '.join(self.header[tag])
-
-        return self.header[tag]
+            return b' '.join(self.header[tag]).decode('utf-8')
+        elif self.header[tag]:
+            return self.header[tag].decode('utf-8')
+        else:
+            return None
 
     def _scriptlet(self, prog_tag, script_tag):
         ''' Return inline -p script, script or None. '''
@@ -141,19 +143,19 @@ class RpmFile(object):
     def filelist(self):
         ''' List of files in this rpm (expanded). '''
         self.init()
-        return self.header[rpm.RPMTAG_FILENAMES]
+        return [x.decode('utf-8') for x in self.header[rpm.RPMTAG_FILENAMES]]
 
     @property
     def requires(self):
         ''' List of requires, also auto-generated for rpm. '''
         self.init()
-        return self.header[rpm.RPMTAG_REQUIRES]
+        return [x.decode('utf-8') for x in self.header[rpm.RPMTAG_REQUIRES]]
 
     @property
     def provides(self):
         ''' List of provides, also auto-generated for rpm. '''
         self.init()
-        return self.header[rpm.RPMTAG_PROVIDES]
+        return [x.decode('utf-8') for x in self.header[rpm.RPMTAG_PROVIDES]]
 
     @property
     def format_requires(self):
diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index 1c97034..059b789 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -110,7 +110,7 @@ class SpecFile(object):
                                  nvr.name, nvr.version, nvr.release)
                 return None
 
-        pkgs = [p.header[rpm.RPMTAG_NAME] for p in self.spec.packages]
+        pkgs = [p.header[rpm.RPMTAG_NAME].decode('utf-8') for p in self.spec.packages]
         pkgs = [p for p in pkgs if not self.get_files(p) is None]
         return [p for p in pkgs if check_pkg_path(p)]
 
@@ -150,7 +150,7 @@ class SpecFile(object):
         if not pkg_name:
             return self.spec.packages[0]
         for p in self.spec.packages:
-            if p.header[rpm.RPMTAG_NAME] == pkg_name:
+            if p.header[rpm.RPMTAG_NAME].decode('utf-8') == pkg_name:
                 return p
         raise KeyError(pkg_name + ': no such package')
 
@@ -219,7 +219,7 @@ class SpecFile(object):
                     lines.append(line)
             elif line:
                 lines.append(line)
-        return lines
+        return [l.decode('utf-8') for l in lines]
 
     @property
     def base_package(self):
diff --git a/test/test_misc.py b/test/test_misc.py
index d14c055..9d2baf8 100644
--- a/test/test_misc.py
+++ b/test/test_misc.py
@@ -543,10 +543,10 @@ class TestMisc(FR_TestCase):
                    '/usr/bin/python setup.py build']
 
         build = spec.get_section('%build')
-        build = map(fix_usr_link, build)
+        build = [fix_usr_link(b) for b in build]
         self.assertIn(''.join(build), ''.join(expected))
         install = spec.get_section('%install')
-        install = map(fix_usr_link, install)
+        install = [fix_usr_link(x) for x in install]
         expected = ['LANG=C', 'export LANG', 'unset DISPLAY',
                     'rm -rf $RPM_BUILD_ROOT',
                     '/usr/bin/python setup.py install -O1 --skip-build'

From 06fae3b5e432f8be786000d1bdc1de1ee6594e3d Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 17/31] Avoid unnecessary list construction


---

diff --git a/src/FedoraReview/checks.py b/src/FedoraReview/checks.py
index c399a0f..1318d19 100644
--- a/src/FedoraReview/checks.py
+++ b/src/FedoraReview/checks.py
@@ -326,8 +326,8 @@ class Checks(_ChecksLoader):
 
     def _get_ready_to_run(self):
         ''' Return checks ready to run, deprecating checks first. '''
-        names = list(self.checkdict.keys())
-        tests_to_run = [n for n in names if self._ready_to_run(n)]
+        names = self.checkdict.keys()
+        tests_to_run = (n for n in names if self._ready_to_run(n))
         return sorted(tests_to_run,
                       key=lambda t: len(self.checkdict[t].deprecates),
                       reverse=True)

From 492316e6dde32e955ff069fa825c14ae9e456d58 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 18/31] TypeError: cannot use a string pattern on a bytes-like object


---

diff --git a/plugins/R.py b/plugins/R.py
index ea38a5c..adb3487 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -63,7 +63,8 @@ class RCheckBase(CheckBase):
                 self.log.warning('Could not retrieve info from %s', url)
                 self.log.debug('Error: %s', err, exc_info=True)
                 continue
-            res = re.search('Package: %s\nVersion:.*' % name, content)
+            res = re.search(('Package: %s\nVersion:.*' % name).encode(),
+                            content)
             if res is not None:
                 self.log.debug("Found in: %s", url)
                 versionok.append(url)

From 01a426a62dea9f1f0e48d34d8501238316dd1f4e Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 19/31] Don't decode text, that's not possible


---

diff --git a/src/FedoraReview/version.py b/src/FedoraReview/version.py
index 91f623c..1846d1e 100755
--- a/src/FedoraReview/version.py
+++ b/src/FedoraReview/version.py
@@ -30,10 +30,9 @@ class VersionError(ReviewError):
 def _setup():
     ''' Setup  the 'version' file from version.tmpl. '''
     try:
-        octets = check_output(['git', 'log', '--pretty=format:%h %ci', '-1'], universal_newlines=True)
+        line = check_output(['git', 'log', '--pretty=format:%h %ci', '-1'], universal_newlines=True)
     except:
         raise VersionError("No version file and git not available.")
-    line = octets.decode('utf-8')
     words = line.split()
     commit = words.pop(0)
     date = words.pop(0)

From dbf3688ad94c81a043c3d133ef37712c3d04aaff Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 20/31] Use absolute import in version.py to fix update-version


---

diff --git a/src/FedoraReview/version.py b/src/FedoraReview/version.py
index 1846d1e..8655428 100755
--- a/src/FedoraReview/version.py
+++ b/src/FedoraReview/version.py
@@ -14,7 +14,7 @@ try:
 except ImportError:
     from FedoraReview.el_compat import check_output
 
-from .review_error import ReviewError
+from FedoraReview.review_error import ReviewError
 
 __version__ = 'Unknown'
 BUILD_FULL = 'Unknown (no version file nor git info)'

From 293c577f430a537a8f12e547f0dc35d49663233d Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 21/31] Use print function in update-version


---

diff --git a/update-version b/update-version
index 1899493..2c33460 100755
--- a/update-version
+++ b/update-version
@@ -6,6 +6,7 @@
 #    $ cd .git/hooks
 #    $ ln -s ../../update-version post-commit
 
+from __future__ import print_function
 import os.path
 import os
 import sys
@@ -13,7 +14,7 @@ import sys
 here = os.path.dirname(os.path.realpath(__file__))
 os.chdir(os.path.join(here, 'src', 'FedoraReview'))
 if not os.access(os.getcwd(), os.W_OK):
-    print "I cannot write into " + os.getcwd() + ", giving up"
+    print("I cannot write into " + os.getcwd() + ", giving up")
     sys.exit(1)
 if os.path.exists('version'):
     os.remove('version')

From aaecab99ec7149ba03cad250ae4e07dadfba0fbf Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 22/31] Update spec for Python 3


---

diff --git a/fedora-review.spec b/fedora-review.spec
index e92c7f5..046d044 100644
--- a/fedora-review.spec
+++ b/fedora-review.spec
@@ -23,36 +23,26 @@ Source0:    https://releases.pagure.org/FedoraReview/%{name}-%{version}%{?git_ta
 
 BuildArch:  noarch
 
-BuildRequires:  python-BeautifulSoup
-BuildRequires:  python-bugzilla
-BuildRequires:  python-packaging
-BuildRequires:  python-straight-plugin
-%if 0%{?rhel} && 0%{?rhel} < 7
-BuildRequires:  python-unittest2
-%endif
-BuildRequires:  python2-devel
-BuildRequires:  rpm-python
+BuildRequires:  python3-beautifulsoup4
+BuildRequires:  python3-bugzilla
+BuildRequires:  python3-packaging
+BuildRequires:  python3-straight-plugin
+BuildRequires:  python3-devel
+BuildRequires:  python3-rpm
 
-Requires:       packagedb-cli
 Requires:       fedora-packager
-Requires:       python-BeautifulSoup
-Requires:       python-bugzilla
-Requires:       python-kitchen
-Requires:       python-packaging
-Requires:       python-straight-plugin
-Requires:       packagedb-cli
-Requires:       rpm-python
+Requires:       python3-beautifulsoup4
+Requires:       python3-bugzilla
+Requires:       python3-packaging
+Requires:       python3-straight-plugin
+Requires:       python3-rpm
 # licensecheck used to be in rpmdevtools, moved to devscripts later
 # this is compatible with both situations without ifdefs
 Requires:       %{_bindir}/licensecheck
-%if 0%{?fedora} > 21
 Requires:       dnf-plugins-core
-%else
-Requires:       yum-utils
-%endif
 
 # Let's be consistent with the name used on fedorahosted
-provides:       FedoraReview = %{version}-%{release}
+Provides:       FedoraReview = %{version}-%{release}
 
 Provides:       %{name}-php-phpci = %{version}-%{release}
 Obsoletes:      %{name}-php-phpci < %{version}-%{release}
@@ -87,7 +77,7 @@ fedora-review ruby-specific tests, not installed by default.
 %package tests
 Summary: Test and test data files for fedora-review
 Requires: %{name} = %{version}-%{release}
-Requires: python-nose
+Requires: python3-nose
 
 %description tests
 Tests are packaged separately due to space concerns.
@@ -98,20 +88,24 @@ Tests are packaged separately due to space concerns.
 
 
 %build
-%py2_build
+%py3_build
 
 
 %install
-%py2_install
-pkg_dir="$RPM_BUILD_ROOT/%{python_sitelib}/FedoraReview"
+%py3_install
+pkg_dir="%{buildroot}/%{python3_sitelib}/FedoraReview"
 ln -s %{_datadir}/%{name}/scripts $pkg_dir/scripts
 ln -s %{_datadir}/%{name}/plugins $pkg_dir/plugins
 cd test
 bash < restore-links.sh
 rm restore-links.sh remember-links
 cd ..
-cp -ar test "$RPM_BUILD_ROOT%{_datadir}/%{name}"
-cp -a pep8.conf pylint.conf "$RPM_BUILD_ROOT%{_datadir}/%{name}"
+cp -ar test "%{buildroot}%{_datadir}/%{name}"
+cp -a pep8.conf pylint.conf "%{buildroot}%{_datadir}/%{name}"
+
+# Fix shebangs in %%{_bindir}.
+chmod -c 0755 %{buildroot}%{_bindir}/*
+pathfix.py -pni "%{__python3} %{py3_shbang_opts}" %{buildroot}%{python3_sitelib} %{buildroot}%{_bindir}/*
 
 
 %check
@@ -122,15 +116,15 @@ export MAKE_RELEASE=1
 mock --quiet -r fedora-25-i386 --init
 mock --quiet -r fedora-26-i386 --init
 mock --quiet -r fedora-26-i386 --uniqueext=hugo --init
-%{__python2} -m unittest discover -f
+%{__python3} -m unittest discover -f
 %endif
 
 
 %files
 %doc README
 %license COPYING AUTHORS
-%{python2_sitelib}/FedoraReview
-%{python2_sitelib}/fedora_review-%{version}-py%{python2_version}.egg-info
+%{python3_sitelib}/FedoraReview
+%{python3_sitelib}/fedora_review-%{version}-py%{python3_version}.egg-info
 %{_bindir}/fedora-review
 %{_bindir}/fedora-create-review
 %{_bindir}/koji-download-scratch

From 6d1896d8025a538c44b79df7c398e070d618acb8 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 23/31] Fix bad string modulo


---

diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index fe069e3..aa96453 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -455,7 +455,7 @@ class _Mock(HelpersMixin):
             return None
         else:
             self.log.debug('Build failed rc = %s', rc)
-            error = ReviewError('mock build failed, see %s/build.log',
+            error = ReviewError('mock build failed, see %s/build.log' %
                                 self.resultdir)
             error.show_logs = False
             raise error

From 36fd9533dba9fecad442835588ea97a61b125016 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 24/31] Fix packages with no main %files section


---

diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index 059b789..3fef443 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -187,9 +187,9 @@ class SpecFile(object):
             elif token == '-f':
                 tokens.pop(0)
             else:
-                return self.base_package + '-' + token
+                return self.base_package.decode() + '-' + token
 
-        return self.base_package
+        return self.base_package.decode()
 
     def _parse_files(self, pkg_name):
         ''' Parse and return the %files section for pkg_name.
@@ -219,7 +219,7 @@ class SpecFile(object):
                     lines.append(line)
             elif line:
                 lines.append(line)
-        return [l.decode('utf-8') for l in lines]
+        return lines
 
     @property
     def base_package(self):
@@ -295,12 +295,14 @@ class SpecFile(object):
         '''
         try:
             files = self._get_pkg_by_name(pkg_name).fileList
-            return [l for l in [f.decode('utf-8').strip()
-                                for f in files.split(b'\n')] if l]
         except AttributeError:
             # No fileList attribute...
             # https://bugzilla.redhat.com/show_bug.cgi?id=857653
             return self._parse_files(pkg_name)
+        if files is None:
+            return None
+        return [l for l in [f.decode('utf-8').strip()
+                            for f in files.split(b'\n')] if l]
 
     def get_section(self, section, raw=False):
         '''

From bbb5668e961efbc841c71dc53118f3e83a5cd5e3 Mon Sep 17 00:00:00 2001
From: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 25/31] Fix fedora-create-review on Python 3.


---

diff --git a/src/fedora-create-review b/src/fedora-create-review
index 515ff6a..42cf047 100755
--- a/src/fedora-create-review
+++ b/src/fedora-create-review
@@ -47,9 +47,9 @@ srcdir = os.path.join(here, 'FedoraReview')
 if os.path.exists(srcdir):
     sys.path.insert(0, srcdir)
 
-import create_review
+from FedoraReview import create_review
 
 try:
     create_review.ReviewRequest().main()
 except Exception as err:
-    print err
+    print(err)

From 666f90f60b29c7db7b0ba3b508cece38f1c4004b Mon Sep 17 00:00:00 2001
From: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 26/31] Decode rpm information used to create review requests.


---

diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 2001445..3e2ebaa 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -339,19 +339,19 @@ class ReviewRequest(object):
 
     def retrieve_description(self):
         """ Retrieve the description tag from a spec file. """
-        description = self.spec.packages[0].header[1005]
+        description = self.spec.packages[0].header[1005].decode('utf-8')
         self.log.debug('Description: %s', description)
         return description
 
     def retrieve_name(self):
         """ Retrieve the name tag from a spec file. """
-        name = self.spec.packages[0].header[1000]
+        name = self.spec.packages[0].header[1000].decode('utf-8')
         self.log.debug('Name: %s', name)
         return name
 
     def retrieve_summary(self):
         """ Retrieve the summary tag from a spec file. """
-        summary = self.spec.packages[0].header[1004]
+        summary = self.spec.packages[0].header[1004].decode('utf-8')
         self.log.debug('Summary: %s', summary)
         return summary
 

From 61357b653e22e6788bdfd8e6e481eaecf126393b Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 14 2019 14:19:34 +0000
Subject: [PATCH 27/31] Split bytes by bytes


---

diff --git a/plugins/R.py b/plugins/R.py
index adb3487..685f8a3 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -69,7 +69,7 @@ class RCheckBase(CheckBase):
                 self.log.debug("Found in: %s", url)
                 versionok.append(url)
                 if version is None:
-                    ver = res.group().split('\n')[1]
+                    ver = res.group().split(b'\n')[1]
                     version = ver.replace('Version:', '').strip()
                 else:
                     self.log.warning(

From 5bb168d67c34e6a514f8a4ceac9fac936b08b726 Mon Sep 17 00:00:00 2001
From: Robert-André Mauchin <zebob.m@gmail.com>
Date: Mar 14 2019 14:19:35 +0000
Subject: [PATCH 28/31] Convert get_upstream_r_package_version to use bytes


and decode the version to utf-8 in RCheckLatestVersionIsPackaged

---

diff --git a/plugins/R.py b/plugins/R.py
index 685f8a3..33f3ffd 100644
--- a/plugins/R.py
+++ b/plugins/R.py
@@ -70,7 +70,7 @@ class RCheckBase(CheckBase):
                 versionok.append(url)
                 if version is None:
                     ver = res.group().split(b'\n')[1]
-                    version = ver.replace('Version:', '').strip()
+                    version = ver.replace(b'Version:', b'').strip()
                 else:
                     self.log.warning(
                         " * Found two version of the package in %s",
@@ -161,7 +161,7 @@ class RCheckLatestVersionIsPackaged(RCheckBase):
     def run_on_applicable(self):
         """ Run the check """
         cur_version = self.spec.expand_tag('Version').decode('utf-8')
-        up_version = self.get_upstream_r_package_version()
+        up_version = self.get_upstream_r_package_version().decode('utf-8')
         if up_version is None:
             self.set_passed(
                 self.PENDING,

From 4bb383d6641cbcc5647ad9bf6587fa0d020bc3e4 Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 16 2019 17:46:28 +0000
Subject: [PATCH 29/31] Apply pyupgrade tool


https://github.com/asottile/pyupgrade

    pyupgrade koji-download-scratch try-fedora-review update-version *.py \
        plugins/*.py src/fedora-create-review src/fedora-review src/*.py \
        src/FedoraReview/*.py test/*.py

---

diff --git a/plugins/ccpp.py b/plugins/ccpp.py
index 4159066..f861af6 100644
--- a/plugins/ccpp.py
+++ b/plugins/ccpp.py
@@ -125,7 +125,7 @@ class CheckHeaderFiles(CCppCheckBase):
                 # All other .h files should be in a -devel package.
                 if '-devel' not in pkg:
                     passed = False
-                    extra += "%s : %s\n" % (pkg, path)
+                    extra += "{} : {}\n".format(pkg, path)
         self.set_passed(passed, extra)
 
 
@@ -173,7 +173,7 @@ class CheckSoFiles(CCppCheckBase):
             if pkg.endswith('-devel'):
                 continue
             for path in self.rpms.find_all('*.so', pkg):
-                bad_list.append("%s: %s" % (pkg, path))
+                bad_list.append("{}: {}".format(pkg, path))
                 if self.bad_re.search(path):
                     in_libdir = True
                 else:
@@ -217,7 +217,7 @@ class CheckLibToolArchives(CCppCheckBase):
             extra = ""
             for pkg in self.spec.packages:
                 for path in self.rpms.find_all('*.la', pkg):
-                    extra += "%s : %s\n" % (pkg, path)
+                    extra += "{} : {}\n".format(pkg, path)
             self.set_passed(self.FAIL, extra)
 
 
diff --git a/plugins/generic.py b/plugins/generic.py
index cc3839d..bbaf16b 100644
--- a/plugins/generic.py
+++ b/plugins/generic.py
@@ -731,7 +731,7 @@ class CheckLicensInDoc(GenericCheckBase):
         for pkg in self.spec.packages:
             nvr = self.spec.get_package_nvr(pkg)
             rpm_path = Mock.get_package_rpm_path(nvr)
-            cmd = 'rpm -qp%s %s' % (self._license_flag, rpm_path)
+            cmd = 'rpm -qp{} {}'.format(self._license_flag, rpm_path)
             doclist = check_output(cmd.split(), universal_newlines=True)
             flagged_files.extend(doclist.split('\n'))
         flagged_files = [f.split('/')[-1] for f in flagged_files]
@@ -901,7 +901,7 @@ class CheckNameCharset(GenericCheckBase):
         if passed:
             self.set_passed(passed)
         else:
-            self.set_passed(passed, '%s\n%s' % (self.spec.name, output))
+            self.set_passed(passed, '{}\n{}'.format(self.spec.name, output))
 
 
 class CheckNaming(GenericCheckBase):
@@ -1111,7 +1111,7 @@ class CheckOwnOther(GenericCheckBase):
             items = []
             for d in owners_by_dir:
                 owners = ', '.join(owners_by_dir[d])
-                items.append("{0}({1})".format(d, owners))
+                items.append("{}({})".format(d, owners))
             return "Dirs in package are owned also by: " + \
                 ', '.join(items)
 
@@ -1210,7 +1210,7 @@ class CheckSourceMD5(GenericCheckBase):
                 self.log.warn(
                     "Cannot extract local source: %s", s.filename)
                 return(False, None)
-            cmd = '/usr/bin/diff -U2 -r %s %s' % (upstream, local)
+            cmd = '/usr/bin/diff -U2 -r {} {}'.format(upstream, local)
             self.log.debug(' Diff cmd: %s', cmd)
             try:
                 p = Popen(cmd.split(), stdout=PIPE, stderr=PIPE, universal_newlines=True)
@@ -1237,9 +1237,9 @@ class CheckSourceMD5(GenericCheckBase):
             local = self.srpm.check_source_checksum(source.filename)
             upstream = source.check_source_checksum()
             text += source.url + ' :\n'
-            text += '  CHECKSUM({0}) this package     : {1}\n'.\
+            text += '  CHECKSUM({}) this package     : {}\n'.\
                     format(Settings.checksum.upper(), local)
-            text += '  CHECKSUM({0}) upstream package : {1}\n'.\
+            text += '  CHECKSUM({}) upstream package : {}\n'.\
                     format(Settings.checksum.upper(), upstream)
             if local != upstream:
                 all_sources_passed = False
@@ -1870,8 +1870,8 @@ class CheckSystemdUnitdirScriplets(GenericCheckBase):
     def run(self):
         using = []
         failed = False
-        systemd_post_re = re.compile(re.escape(rpm.expandMacro('%systemd_post .*.service')).replace('\.\*', '.*')[2:-4], re.M)
-        systemd_preun_re = re.compile(re.escape(rpm.expandMacro('%systemd_preun .*.service')).replace('\.\*', '.*')[2:-4], re.M)
+        systemd_post_re = re.compile(re.escape(rpm.expandMacro('%systemd_post .*.service')).replace(r'\.\*', '.*')[2:-4], re.M)
+        systemd_preun_re = re.compile(re.escape(rpm.expandMacro('%systemd_preun .*.service')).replace(r'\.\*', '.*')[2:-4], re.M)
         for pkg in self.spec.packages:
             if self.rpms.find('/usr/lib/systemd/system/*', pkg):
                 using.append(pkg)
@@ -1903,8 +1903,8 @@ class CheckSystemdUserunitdirScriplets(GenericCheckBase):
     def run(self):
         using = []
         failed = False
-        systemd_user_post_re = re.compile(re.escape(rpm.expandMacro('%systemd_user_post .*.service')).replace('\.\*', '.*')[2:], re.M)
-        systemd_user_preun_re = re.compile(re.escape(rpm.expandMacro('%systemd_user_preun .*.service')).replace('\.\*', '.*')[2:], re.M)
+        systemd_user_post_re = re.compile(re.escape(rpm.expandMacro('%systemd_user_post .*.service')).replace(r'\.\*', '.*')[2:], re.M)
+        systemd_user_preun_re = re.compile(re.escape(rpm.expandMacro('%systemd_user_preun .*.service')).replace(r'\.\*', '.*')[2:], re.M)
         for pkg in self.spec.packages:
             if self.rpms.find('/usr/lib/systemd/user/*', pkg):
                 using.append(pkg)
diff --git a/plugins/generic_should.py b/plugins/generic_should.py
index 82b1c14..d410612 100644
--- a/plugins/generic_should.py
+++ b/plugins/generic_should.py
@@ -297,7 +297,7 @@ class CheckIllegalSpecTags(GenericShouldCheckBase):
             value = self.spec.expand_tag(tag).decode('utf-8')
             if value:
                 passed = False
-                output += 'Found : %s: %s\n' % (tag, value)
+                output += 'Found : {}: {}\n'.format(tag, value)
         if not passed:
             self.set_passed(passed, output)
         else:
@@ -419,7 +419,7 @@ class CheckPkgConfigFiles(GenericShouldCheckBase):
             for fn in files_by_pkg[pkg]:
                 if '-devel' not in pkg:
                     passed = 'pending'
-                    extra += '%s : %s\n' % (pkg, fn)
+                    extra += '{} : {}\n'.format(pkg, fn)
         self.set_passed(passed, extra)
 
 
diff --git a/plugins/shell_api.py b/plugins/shell_api.py
index fb675c3..e90226e 100644
--- a/plugins/shell_api.py
+++ b/plugins/shell_api.py
@@ -139,7 +139,7 @@ function attach()
     fi
     sort_hint=$1
     shift
-    title=${*//\/ }
+    title=${*//\\/ }
     file="$sort_hint;${title/;/:};$i"
     cat > .attachments/"$file"
     cd $startdir
@@ -182,7 +182,7 @@ def _settings_generator():
             value = ''
         if isinstance(value, str):
             value = value.replace('"', r'\"')
-        body += 'FR_SETTINGS[%s]="%s"\n' % (key, value)
+        body += 'FR_SETTINGS[{}]="{}"\n'.format(key, value)
     return body
 
 
@@ -207,7 +207,7 @@ def _files_generator(spec):
     body = 'declare -A FR_FILES\n'
     for pkg in spec.packages:
         item = _quote('\n'.join(spec.get_files(pkg)))
-        body += """FR_FILES[%s]='%s'\n""" % (pkg, item)
+        body += """FR_FILES[{}]='{}'\n""".format(pkg, item)
     return body
 
 
@@ -220,7 +220,7 @@ def _description_generator(spec):
         section = spec.get_section('%description', pkg)
         if section:
             item = _quote('\n'.join(section))
-            body += """FR_DESCRIPTION[%s]='%s'\n""" % (pkg, item)
+            body += """FR_DESCRIPTION[{}]='{}'\n""".format(pkg, item)
     return body
 
 
@@ -228,7 +228,7 @@ def _flags_generator(flags):
     ''' Bash code defining FR_FLAGS,reflecting checks.flags. '''
     body = 'declare -A FR_FLAGS\n'
     for flag in flags.values():
-        body += """FR_FLAGS[%s]='%s'\n""" % (flag.name, str(flag))
+        body += """FR_FLAGS[{}]='{}'\n""".format(flag.name, str(flag))
     return body
 
 
diff --git a/src/FedoraReview/checks.py b/src/FedoraReview/checks.py
index 1318d19..b6ae273 100644
--- a/src/FedoraReview/checks.py
+++ b/src/FedoraReview/checks.py
@@ -287,7 +287,7 @@ class Checks(_ChecksLoader):
         with open('.testlog.txt', 'w') as f:
             for r in results:
                 f.write('\n' + 24 * ' ' +
-                        "('%s', '%s')," % (r.state, r.name))
+                        "('{}', '{}'),".format(r.state, r.name))
 
     def _ready_to_run(self, name):
         """
@@ -389,7 +389,7 @@ class Checks(_ChecksLoader):
             with open('.testlog.txt', 'w') as f:
                 for r in results:
                     f.write('\n' + 24 * ' ' +
-                            "('%s', '%s')," % (r.state, r.name))
+                            "('{}', '{}'),".format(r.state, r.name))
 
 
 # vim: set expandtab ts=4 sw=4:
diff --git a/src/FedoraReview/create_review.py b/src/FedoraReview/create_review.py
index 3e2ebaa..a5e0602 100644
--- a/src/FedoraReview/create_review.py
+++ b/src/FedoraReview/create_review.py
@@ -210,7 +210,7 @@ class ReviewRequest(object):
             'product': 'Fedora',
             'component': 'Package Review',
             'version': 'rawhide',
-            'short_desc': '%s: %s - %s' % (review_type,
+            'short_desc': '{}: {} - {}'.format(review_type,
                                            self.info['name'],
                                            self.info['summary']),
             'comment': BUG_COMMENT % (self.info['specurl'],
@@ -252,7 +252,7 @@ class ReviewRequest(object):
         info in the settings.
         """
         complement_url = self.settings.upload_target.split('public_html/')[1]
-        url = 'https://%s.fedorapeople.org/%s/' % (
+        url = 'https://{}.fedorapeople.org/{}/'.format(
             self.username, complement_url)
         self.info['specurl'] = url + os.path.basename(self.specfile)
         self.info['srpmurl'] = url + os.path.basename(self.srpmfile)
@@ -270,7 +270,7 @@ class ReviewRequest(object):
         """
         bugbz = self.bzclient.query(
             #  'bug_status': ['CLOSED'],
-            {'short_desc': "Request: {0} -".format(self.info['name']),
+            {'short_desc': "Request: {} -".format(self.info['name']),
              'short_desc_type': 'allwordssubstr',
              'query_format': 'advanced',
              'component': 'Package Review'})
@@ -333,7 +333,7 @@ class ReviewRequest(object):
         bug = self.create_review_request(args.rename_request)
         if not args.no_build:
             add_comment_build(output_build, bug)
-        print('Review created at: %s/show_bug.cgi?id=%s' % (bzurl,
+        print('Review created at: {}/show_bug.cgi?id={}'.format(bzurl,
                                                             bug.id))
         print(bug)
 
diff --git a/src/FedoraReview/helpers_mixin.py b/src/FedoraReview/helpers_mixin.py
index bcbd18e..bf5f647 100644
--- a/src/FedoraReview/helpers_mixin.py
+++ b/src/FedoraReview/helpers_mixin.py
@@ -35,7 +35,7 @@ class DownloadError(ReviewError):
     ''' Error in urlretrieve(). '''
     def __init__(self, code, url):
         ReviewError.__init__(
-            self, "Error %s downloading %s" % (code, url))
+            self, "Error {} downloading {}".format(code, url))
 
 
 class HelpersMixin(object):
diff --git a/src/FedoraReview/mock.py b/src/FedoraReview/mock.py
index aa96453..d2c9fc0 100644
--- a/src/FedoraReview/mock.py
+++ b/src/FedoraReview/mock.py
@@ -96,7 +96,7 @@ def _add_buildarch_macros(macros, paths):
     if not paths:
         return '', macros
     arches = [p.rsplit('.', 2)[1] for p in paths]
-    if set(arches) == set(['noarch']):
+    if set(arches) == {'noarch'}:
         buildarch = 'noarch'
     else:
         buildarch = [a for a in arches if a != 'noarch'][0]
@@ -296,7 +296,7 @@ class _Mock(HelpersMixin):
         on missing or multiple matches. Argument should have
         have name, version and release attributes.
         '''
-        pattern = '%s-%s*' % (nvr.name, nvr.version)
+        pattern = '{}-{}*'.format(nvr.name, nvr.version)
         paths = self._get_rpm_paths(pattern)
         paths = [p for p in paths if p.endswith('.rpm') and
                  not p.endswith('.src.rpm')]
@@ -315,7 +315,7 @@ class _Mock(HelpersMixin):
 
         def get_package_srpm_path(spec):
             ''' Return path to srpm given a spec. '''
-            pattern = '*%s-%s*' % (spec.name, spec.version)
+            pattern = '*{}-{}*'.format(spec.name, spec.version)
             paths = self._get_rpm_paths(pattern)
             paths = [p for p in paths if p.endswith('.src.rpm')]
             if not paths:
@@ -335,7 +335,7 @@ class _Mock(HelpersMixin):
 
     def get_package_debuginfo_paths(self, nvr):
         ''' Return paths to debuginfo rpms for given nvr.  '''
-        pattern = '%s-*debuginfo*-%s-*' % (nvr.name, nvr.version)
+        pattern = '{}-*debuginfo*-{}-*'.format(nvr.name, nvr.version)
         return self._get_rpm_paths(pattern)
 
     def get_builddir(self, subdir=None):
diff --git a/src/FedoraReview/reports.py b/src/FedoraReview/reports.py
index 4d89408..91540d5 100644
--- a/src/FedoraReview/reports.py
+++ b/src/FedoraReview/reports.py
@@ -97,7 +97,7 @@ def _write_section(results, output):
 
         return '3' + str(result.check.sort_key)
 
-    groups = list(set([hdr(test.group) for test in results]))
+    groups = list({hdr(test.group) for test in results})
     res = None
     for group in sorted(groups):
         res = [r for r in results if hdr(r.group) == group]
diff --git a/src/FedoraReview/review_helper.py b/src/FedoraReview/review_helper.py
index f531337..14ff2b4 100644
--- a/src/FedoraReview/review_helper.py
+++ b/src/FedoraReview/review_helper.py
@@ -149,7 +149,7 @@ class ReviewHelper(object):
                     print('Flag: ' + flag.name)
                 files_per_src = [c for c in checks_list
                                  if c.defined_in == f]
-                groups = list(set([c.group for c in files_per_src]))
+                groups = list({c.group for c in files_per_src})
                 for group in sorted(groups):
 
                     def check_match(c):
@@ -161,12 +161,12 @@ class ReviewHelper(object):
                         continue
                     print('Group: ' + group)
                     for c in sorted(checks):
-                        print('    %s: %s' % (c.name, c.text))
+                        print('    {}: {}'.format(c.name, c.text))
                 print()
 
         checks_lister = ChecksLister()
         checks_list = list(checks_lister.get_checks().values())
-        files = list(set([c.defined_in for c in checks_list]))
+        files = list({c.defined_in for c in checks_list})
         list_data_by_file(files, checks_list)
         deps_list = [c for c in checks_list
                      if c.needs != [] and c.needs != ['CheckBuildCompleted']]
diff --git a/src/FedoraReview/spec_file.py b/src/FedoraReview/spec_file.py
index 3fef443..2ed8289 100644
--- a/src/FedoraReview/spec_file.py
+++ b/src/FedoraReview/spec_file.py
@@ -116,7 +116,7 @@ class SpecFile(object):
 
     # pylint: disable=W1401
     def _get_lines(self, filename):
-        ''' Read line from specfile, fold \ continuation lines. '''
+        r''' Read line from specfile, fold \ continuation lines. '''
         with open(filename, "r") as f:
             lines = f.readlines()
         last = None
diff --git a/test/test_deps.py b/test/test_deps.py
index 60bdc2e..a0dc4d2 100644
--- a/test/test_deps.py
+++ b/test/test_deps.py
@@ -29,7 +29,7 @@ import FedoraReview.deps as deps
 
 from fr_testcase import FEDORA
 
-DEPLIST_OK = set(["bash",
+DEPLIST_OK = {"bash",
                   "python",
                   "fedora-packager",
                   "python",
@@ -40,16 +40,16 @@ DEPLIST_OK = set(["bash",
                   "python-straight-plugin",
                   "packagedb-cli",
                   "rpm-python",
-                  "devscripts-minimal"])
+                  "devscripts-minimal"}
 
 
-DIRLIST_OK = set(["/usr/share/doc/ruby-racc-1.4.5",
+DIRLIST_OK = {"/usr/share/doc/ruby-racc-1.4.5",
                   "/usr/share/doc/ruby-racc-1.4.5/doc.en",
                   "/usr/share/doc/ruby-racc-1.4.5/doc.ja",
-                  "/usr/share/ruby/vendor_ruby/racc"])
+                  "/usr/share/ruby/vendor_ruby/racc"}
 DIRLIST_PKG = 'ruby-racc/ruby-racc/results/ruby-racc-1.4.5-9.fc17.noarch.rpm'
 
-OWNERS_OK = set(['rpm', 'yum', 'fedora-repos'])
+OWNERS_OK = {'rpm', 'yum', 'fedora-repos'}
 
 
 class TestDeps(unittest.TestCase):
@@ -67,7 +67,7 @@ class TestDeps(unittest.TestCase):
     def test_resolve(self):
         ''' Test resolving symbols -> packages. '''
         pkgs = deps.resolve(['config(rpm)', 'perl'])
-        self.assertEqual(set(['rpm', 'perl']), set(pkgs))
+        self.assertEqual({'rpm', 'perl'}, set(pkgs))
 
     @unittest.skipIf(not FEDORA, 'Fedora-only test')
     def test_list_dirs(self):
diff --git a/test/test_misc.py b/test/test_misc.py
index 9d2baf8..8882349 100644
--- a/test/test_misc.py
+++ b/test/test_misc.py
@@ -394,7 +394,7 @@ class TestMisc(FR_TestCase):
         self.assertTrue(check.is_passed)
         paths = check.checks.sources.find_all_re('.*[.]py')
         files = [os.path.basename(p) for p in paths]
-        self.assertEqual(set(files), set(['setup.py', '__init__.py']))
+        self.assertEqual(set(files), {'setup.py', '__init__.py'})
         files = check.checks.sources.get_filelist()
         self.assertEqual(len(files), 10)
 
@@ -452,7 +452,7 @@ class TestMisc(FR_TestCase):
                              '--no-build'])
         wdir = Mock.get_builddir('BUILD')
         len1 = len(glob.glob(os.path.join(wdir, "*")))
-        s = "cd {0}; ln -sf foo bar || :".format(wdir)
+        s = "cd {}; ln -sf foo bar || :".format(wdir)
         check_output(s, shell=True)
         Mock.builddir_cleanup()
         len2 = len(glob.glob(os.path.join(wdir, "*")))

From 9af34e463a26b0e9a27401c8775b7a6ac4303d2f Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 16 2019 17:53:01 +0000
Subject: [PATCH 30/31] Send the commit hooks to a farm upstate


---

diff --git a/git-hooks/post-commit b/git-hooks/post-commit
deleted file mode 100755
index ef5e431..0000000
--- a/git-hooks/post-commit
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/bin/bash
-#
-# fedora-review post-commit
-#
-cd $( dirname $( readlink -fn $0 ))/..
-commit=$( git rev-parse HEAD )
-commit=${commit:0:7}
-host=$( uname -n )
-isodate=( $( git show -s --pretty=format:%ci HEAD ) )
-cd src/FedoraReview
-sed -e "s/@commit@/$commit/g" \
-    -e "s/@host@/$host/g" \
-    -e "s/@date@/${isodate[0]}/g" \
-    -e "s/@time@/${isodate[1]}/g" < version.tmpl > version
diff --git a/git-hooks/pre-commit b/git-hooks/pre-commit
deleted file mode 100755
index 9e2cd32..0000000
--- a/git-hooks/pre-commit
+++ /dev/null
@@ -1,56 +0,0 @@
-#!/bin/bash
-#
-# fedora-review git pre-commit hook: run pep8 and pylint
-
-branch_name=$(git symbolic-ref -q HEAD)
-branch_name=${branch_name##refs/heads/}
-branch_name=${branch_name:-HEAD}
-
-if [ "$branch_name" = "master" ];then
-    echo "You are on master branch. If you want to contribute patches "
-    echo "you should work on devel branch. Please read CONTRIBUTE file. "
-    echo "To ignore this warning run 'git commit --no-verify'."
-    exit 1
-fi
-
-ALL_FILES=$( git diff --cached --name-status |
-              grep -v ^D | awk '$1 $2 { print $2}' | grep -e '.py$' )
-
-if [ -z "$ALL_FILES" ]; then
-    exit 0
-fi
-
-pep8 --config pep8.conf  $ALL_FILES || {
-    echo "pep8 failed. Fix your code!"
-    echo "*If* you know what you are doing, use 'git commit --no-verify'"
-    exit 1
-}
-
-SRC_FILES=''
-TEST_FILES=''
-for file in $( echo $ALL_FILES ); do
-    if [[ $file == test/* ]]; then
-        TEST_FILES="$TEST_FILES ${file##test/}"
-    else
-        SRC_FILES="$SRC_FILES $file"
-    fi
-done
-
-if [ "$TEST_FILES" ]; then
-    cd test
-    export PYTHONPATH=../src
-    pylint --rcfile=../pylint.conf -f text $TEST_FILES > pylint.log || {
-        echo "Pylint failed for test files, log in test/pylint.log"
-        echo "Fix test code or use --no-verify."
-        exit 3
-    }
-    cd $OLDPWD
-fi
-if [ "$SRC_FILES" ]; then
-    export PYTHONPATH=src
-    pylint --rcfile=pylint.conf -f text $SRC_FILES > pylint.log || {
-        echo "Pylint failed, log in pylint.log."
-        echo "Fix code or use --no-verify."
-        exit 3
-    }
-fi
diff --git a/try-fedora-review b/try-fedora-review
index 3376671..f90e165 100755
--- a/try-fedora-review
+++ b/try-fedora-review
@@ -38,16 +38,6 @@ if cur_branch == "master":
           "latest stable release. For development/bugreporting use "
           "'devel' branch. See CONTRIBUTE file for more details\033[0m")
 
-# let's setup a proper git commit hooks so people don't commit ugly code
-# accidentally
-devnull = open('/dev/null', 'w')
-subprocess.call(['ln', '-s', '../../git-hooks/pre-commit',
-                 os.path.join(main_dir, '.git/hooks/pre-commit')],
-                 stderr=devnull)
-subprocess.call(['ln', '-s', '../../git-hooks/post-commit',
-                 os.path.join(main_dir, '.git/hooks/post-commit')],
-                 stderr=devnull)
-
 review = ReviewHelper()
 rc = review.run()
 sys.exit(rc)

From 04a60c355d3e47f4f2398be19fd199e1927f35db Mon Sep 17 00:00:00 2001
From: Miro Hrončok <miro@hroncok.cz>
Date: Mar 17 2019 13:59:29 +0000
Subject: [PATCH 31/31] Update README


---

diff --git a/README b/README
index d6f325d..0c4cc91 100644
--- a/README
+++ b/README
@@ -58,8 +58,8 @@ Running from git
 After downloading (above) use try-fedora-review:
 
     $ cd FedoraReview
-    $ ./update-version
-    $ ./try-fedora-review --help
+    $ python3 update-version
+    $ python3 try-fedora-review --help
 
 To run from any directory, install try-fedora-review according to
 instructions in that script. update-version only needs to run once.