Bug 22722 - Make fedabipkgdiff and its tests support both python 3 and 2
authorChenxiong Qi <cqi@redhat.com>
Sun, 25 Mar 2018 07:34:59 +0000 (15:34 +0800)
committerDodji Seketeli <dodji@redhat.com>
Thu, 29 Mar 2018 11:46:50 +0000 (13:46 +0200)
This patch makes fedabipkgdiff Python 3 compatible.  All tests written
in Python are updated and compatible with Python 3 as well.

The patch looks for a Python 3 interperter.  If it finds one then it
runs the tests using that interpreter.  Otherwise it just tries to use
the Python 2 interpreter.

This behaviour can be disabled by the new --disable-python3 option.

* configure.ac: Add new option --enable-python3. Add new
test runner file tests/runtestdefaultsupprs-py3 and
tests/runtestfedabipkgdiffpy3.sh. Add required six Python module.
* tests/Makefile.am: Add new test files
tests/runtestdefaultsupprspy3.sh and
tests/runtestfedabipkgdiffpy3.sh accordingly.
* tests/mockfedabipkgdiff.in: Convert print statement to
six.print_. Replace call to function filter with list
comprehension. Replace basestring with six.string_types.
* tests/runtestdefaultsupprspy3.sh.in: New shell script to run
test runtestdefaultsupprs with Python 3.
* tests/runtestdefaultsupprs.py.in: Repalce a few tabs with
proper number of spaces which is detected by Python 3
interpreter.
* tests/runtestfedabipkgdiffpy3.sh.in: New shell script to run
test runtestfedabipkgdiff with Python 3.
* tests/runtestfedabipkgdiff.py.in: Use python from env in
shebang instead of a fixed path to a Python interpreter.
* tools/fedabipkgdiff: Globally replace print statement with a
function call to print which is available by importing
print_function from __future__ module. Use six.print_ to output
string to stderr instead. Convert function call to map to
for-loop. (cmp_nvr): Change argument to handle a Koji build
mapping instead of only the nvr. (Brew.listBuilds): use the new
cmp_nvr to sort builds.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
.gitignore
configure.ac
tests/Makefile.am
tests/mockfedabipkgdiff.in
tests/runtestdefaultsupprs.py.in
tests/runtestdefaultsupprspy3.sh.in [new file with mode: 0644]
tests/runtestfedabipkgdiff.py.in
tests/runtestfedabipkgdiffpy3.sh.in [new file with mode: 0644]
tools/fedabipkgdiff

index a60cadb..ed4f43c 100644 (file)
@@ -22,4 +22,5 @@ Makefile.in
 .tags
 build/
 TAGS
-fedabipkgdiffc
\ No newline at end of file
+fedabipkgdiffc
+tools/__pycache__/
\ No newline at end of file
index 179174e..5ae6583 100644 (file)
@@ -97,6 +97,12 @@ AC_ARG_ENABLE([fedabipkgdiff],
              ENABLE_FEDABIPKGDIFF=$enableval,
              ENABLE_FEDABIPKGDIFF=auto)
 
+AC_ARG_ENABLE([python3],
+             AS_HELP_STRING([--enable-python3=yes|no|auto],
+                            [enable running abigail tools with python3 (default is auto)]),
+             ENABLE_PYTHON3=$enableval,
+             ENABLE_PYTHON3=auto)
+
 dnl *************************************************
 dnl check for dependencies
 dnl *************************************************
@@ -334,29 +340,84 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then
     fi
   fi
 
-  # The minimal python version we want to support is 2.6.6 because EL6
+  # The minimal python version we want to support is 2.6.6 because EL6
   # distributions have that version installed.
-  MINIMAL_PYTHON_VERSION="2.6.6"
+  MINIMAL_PYTHON2_VERSION="2.6.6"
 
   AC_PATH_PROG(PYTHON, python, no)
-  AX_PROG_PYTHON_VERSION($MINIMAL_PYTHON_VERSION,
+  AX_PROG_PYTHON_VERSION($MINIMAL_PYTHON2_VERSION,
                         [MINIMAL_PYTHON_VERSION_FOUND=yes],
                         [MINIMAL_PYTHON_VERSION_FOUND=no])
 
+  # The minimal python 3 version we want to support is 3.5, which is
+  # available in Fedora releases and in EL7.
+  if test x$ENABLE_PYTHON3 != xno; then
+    AC_CHECK_PROGS(PYTHON3_INTERPRETER, [python3 python3.5 python3.6 python3.7], no)
+  else
+    PYTHON3_INTERPRETER=no
+  fi
+
+  if test x$ENABLE_PYTHON3 = xauto; then
+      if test x$PYTHON3_INTERPRETER != xno; then
+        ENABLE_PYTHON3=yes
+      else
+        # When enabling python3 is auto, tests only run if the
+        # python3 interpreter was found on the system. Otherwise,
+        # just ignore it.
+       ENABLE_PYTHON3=no
+        AC_MSG_NOTICE([Python 3 was not found. Skip running tests with Python 3.])
+      fi
+  fi
+
+  if test x$ENABLE_PYTHON3 = xyes; then
+      if  test x$PYTHON3_INTERPRETER != xno; then
+        # We were asked to enable python3 implicitely (auto and
+        # python3 was found) or explicitly.  So enable running tests
+        # using python3 then.
+        RUN_TESTS_WITH_PY3=yes
+      else
+         AC_MSG_ERROR([Python 3 was not found])
+      fi
+  fi
+
+  if test x$PYTHON3_INTERPRETER = xyes; then
+     MINIMAL_PYTHON_VERSION_FOUND=yes
+  fi
+
   if test x$MINIMAL_PYTHON_VERSION_FOUND = xno; then
     MISSING_FEDABIPKGDIFF_DEP=yes
     if test x$MISSING_FEDABIPKGDIFF_DEP_FATAL = xyes; then
-      AC_MSG_ERROR([could not find a python program of version at least $MINIMAL_PYTHON_VERSION])
+      AC_MSG_ERROR([could not find a python program of version at least $MINIMAL_PYTHON2_VERSION])
+    fi
+  else
+    if test x$PYTHON3_INTERPRETER != xno; then
+     # We were instructed to use python3 and it's present on the
+     # system.  Let's update the PYTHON variable that points to the
+     # actual python interpreter we are going to be using
+     PYTHON=$PYTHON3_INTERPRETER
     fi
   fi
 
+  ###################################################################
+  # Now we are going to check the presence of the required python
+  # modules using either python2 or python3 as required until now.
+  ###################################################################
+
+  # Grrr, the urlparse python2 module got renamed in python3 as
+  # urllib.parse.  Oh well.
+  if test x$PYTHON = xpython3; then
+     URLPARSE_MODULE=urllib.parse
+  else
+     URLPARSE_MODULE=urlparse
+  fi
+
   REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\
-   argparse logging os re subprocess sys urlparse \
-   xdg koji mock rpm imp tempfile mimetypes shutil"
+   argparse logging os re subprocess sys $URLPARSE_MODULE \
+   xdg koji mock rpm imp tempfile mimetypes shutil six"
 
   if test x$ENABLE_FEDABIPKGDIFF != xno; then
     AX_CHECK_PYTHON_MODULES([$REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF],
-                           [python2],
+                           [$PYTHON],
                            [FOUND_ALL_PYTHON_MODULES=yes],
                            [FOUND_ALL_PYTHON_MODULES=no])
 
@@ -399,6 +460,9 @@ koji.read_config('koji')"
 fi
 
 AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes)
+AM_CONDITIONAL(ENABLE_RUNNING_TESTS_WITH_PY3, test x$RUN_TESTS_WITH_PY3 = xyes)
+AM_CONDITIONAL(ENABLE_PYTHON3_INTERPRETER, test x$PYTHON3_INTERPRETER != xno)
+
 
 dnl Check for dependency: libzip
 LIBZIP_VERSION=0.10.1
@@ -557,8 +621,12 @@ AC_CONFIG_FILES([tests/mockfedabipkgdiff],
                [chmod +x tests/mockfedabipkgdiff])
 AC_CONFIG_FILES([tests/runtestfedabipkgdiff.py],
                [chmod +x tests/runtestfedabipkgdiff.py])
+AC_CONFIG_FILES([tests/runtestfedabipkgdiffpy3.sh],
+               [chmod +x tests/runtestfedabipkgdiffpy3.sh])
 AC_CONFIG_FILES([tests/runtestdefaultsupprs.py],
                [chmod +x tests/runtestdefaultsupprs.py])
+AC_CONFIG_FILES([tests/runtestdefaultsupprspy3.sh],
+               [chmod +x tests/runtestdefaultsupprspy3.sh])
 
 AC_OUTPUT
 
@@ -574,6 +642,7 @@ AC_MSG_NOTICE([
     C Compiler                                     : ${CC}
     C++ Compiler                                  : ${CXX}
     GCC visibility attribute supported             : ${SUPPORTS_GCC_VISIBILITY_ATTRIBUTE}
+    Python                                        : ${PYTHON}
 
  OPTIONAL FEATURES:
     Enable zip archives                            : ${ENABLE_ZIP_ARCHIVE}
@@ -584,6 +653,7 @@ AC_MSG_NOTICE([
     Enable GNU tar archive support in abipkgdiff   : ${ENABLE_TAR}
     Enable bash completion                        : ${ENABLE_BASH_COMPLETION}
     Enable fedabipkgdiff                           : ${ENABLE_FEDABIPKGDIFF}
+    Enable python 3                               : ${ENABLE_PYTHON3}
     Enable running tests under Valgrind            : ${enable_valgrind}
     Generate html apidoc                          : ${ENABLE_APIDOC}
     Generate html manual                          : ${ENABLE_MANUAL}
index 0a7f4b9..229bc1f 100644 (file)
@@ -17,9 +17,13 @@ AM_CXXFLAGS += "-std=gnu++11"
 endif
 
 FEDABIPKGDIFF_TEST =
+if ENABLE_RUNNING_TESTS_WITH_PY3
+FEDABIPKGDIFF_TEST += runtestfedabipkgdiffpy3.sh
+else
 if ENABLE_FEDABIPKGDIFF
 FEDABIPKGDIFF_TEST += runtestfedabipkgdiff.py
 endif
+endif
 
 TESTS=                         \
 $(FEDABIPKGDIFF_TEST)          \
@@ -43,6 +47,9 @@ runtestabidiffexit            \
 runtestdefaultsupprs.py                \
 $(CXX11_TESTS)
 
+if ENABLE_RUNNING_TESTS_WITH_PY3
+TESTS += runtestdefaultsupprspy3.sh
+endif
 
 EXTRA_DIST = \
 runtestcanonicalizetypes.sh.in \
@@ -50,6 +57,12 @@ runtestfedabipkgdiff.py.in \
 mockfedabipkgdiff.in \
 test-valgrind-suppressions.supp
 
+if ENABLE_RUNNING_TESTS_WITH_PY3
+EXTRA_DIST += \
+runtestfedabipkgdiffpy3.sh.in  \
+runtestdefaultsupprspy3.sh.in
+endif
+
 CLEANFILES = \
 runtestcanonicalizetypes.output.txt \
 runtestcanonicalizetypes.output.final.txt
@@ -139,6 +152,12 @@ runtestfedabipkgdiff.py$(EXEEXT):
 runtestdefaultsupprs_py_SOURCES =
 runtestdefaultsupprs.py$(EXEEXT):
 
+runtestfedabipkgdiffpy3_sh_SOURCES =
+runtestfedabipkgdiffpy3.sh$(EXEEXT):
+
+runtestdefaultsupprspy3_sh_SOURCES =
+runtestdefaultsupprspy3.sh$(EXEEXT):
+
 AM_CPPFLAGS=-I${abs_top_srcdir}/include \
 -I${abs_top_builddir}/include -I${abs_top_srcdir}/tools -fPIC
 
index aac99d1..47c8cc8 100644 (file)
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 # -*- Mode: Python
 #
@@ -55,13 +55,14 @@ variables.
 import os
 import tempfile
 import imp
+import six
 
 try:
     from mock import patch
 except ImportError:
     import sys
-    print >>sys.stderr, \
-        'mock is required to run tests. Please install before running tests.'
+    six.print_('mock is required to run tests. Please install before running'
+               ' tests.', file=sys.stderr)
     sys.exit(1)
 
 ABIPKGDIFF = '@abs_top_builddir@/tools/abipkgdiff'
@@ -307,12 +308,12 @@ class MockClientSession(object):
         :return: the found package
         :rtype: dict
         """
-        assert isinstance(name, basestring)
+        assert isinstance(name, six.string_types)
 
         def selector(package):
             return package['name'] == name
 
-        return filter(selector, packages)[0]
+        return [p for p in packages if selector(p)][0]
 
     def getBuild(self, build_id):
         """Mock kojihub.getBuild
@@ -326,7 +327,7 @@ class MockClientSession(object):
         def selector(build):
             return build['build_id'] == build_id
 
-        return filter(selector, builds)[0]
+        return [b for b in builds if selector(b)][0]
 
     def listBuilds(self, packageID, state=None):
         """Mock kojihub.listBuilds
@@ -362,7 +363,7 @@ class MockClientSession(object):
                 rpm['release'] == rpminfo['release'] and \
                 rpm['arch'] == rpminfo['arch']
 
-        return filter(selector, rpms)[0]
+        return [rpm for rpm in rpms if selector(rpm)][0]
 
     def listRPMs(self, buildID, arches=None):
         """Mock kojihub.listRPMs
@@ -376,9 +377,9 @@ class MockClientSession(object):
         """
         assert isinstance(buildID, int)
         if arches is not None:
-            assert isinstance(arches, (tuple, list, basestring))
+            assert isinstance(arches, (tuple, list, six.string_types))
 
-        if arches is not None and isinstance(arches, basestring):
+        if arches is not None and isinstance(arches, six.string_types):
             arches = [arches]
 
         def selector(rpm):
@@ -387,7 +388,7 @@ class MockClientSession(object):
                 selected = selected and rpm['arch'] in arches
             return selected
 
-        return filter(selector, rpms)
+        return [rpm for rpm in rpms if selector(rpm)]
 
 
 @patch('koji.ClientSession', new=MockClientSession)
index 41b0b28..10b71d1 100644 (file)
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 '''Runs tests for the default suppression specifications of libabigail.
 
@@ -125,19 +125,18 @@ def run_abidiff_tests():
             if suppressions:
                 env_vars[env_name] = suppressions
 
-           with open(output_path, 'w') as out_file:
-               subprocess.call(cmd, env=env_vars, stdout=out_file)
+            with open(output_path, 'w') as out_file:
+                subprocess.call(cmd, env=env_vars, stdout=out_file)
 
-           diffcmd = ['diff', '-u', reference_report_path,
-           output_path]
+            diffcmd = ['diff', '-u', reference_report_path, output_path]
 
-           return_code = subprocess.call(diffcmd)
+            return_code = subprocess.call(diffcmd)
             if return_code:
                 result = return_code
                 sys.stderr.write("failed abidiff test "
                                  "for env var '" + e + "'\n");
 
-           del env_vars[env_name];
+            del env_vars[env_name];
 
         try:
             os.remove(default_suppression)
@@ -202,19 +201,18 @@ def run_abipkgdiff_tests():
             if suppressions:
                 env_vars[env_name] = suppressions
 
-           with open(output_path, 'w') as out_file:
-               subprocess.call(cmd, env=env_vars, stdout=out_file)
+            with open(output_path, 'w') as out_file:
+                subprocess.call(cmd, env=env_vars, stdout=out_file)
 
-           diffcmd = ['diff', '-u', reference_report_path,
-           output_path]
+            diffcmd = ['diff', '-u', reference_report_path, output_path]
 
-           return_code = subprocess.call(diffcmd)
+            return_code = subprocess.call(diffcmd)
             if return_code:
                 result = return_code
                 sys.stderr.write("failed abipkgdiff test "
                                  "for env var '" + e + "'\n");
 
-           del env_vars[env_name];
+            del env_vars[env_name];
 
         try:
             os.remove(default_suppression)
diff --git a/tests/runtestdefaultsupprspy3.sh.in b/tests/runtestdefaultsupprspy3.sh.in
new file mode 100644 (file)
index 0000000..4985206
--- /dev/null
@@ -0,0 +1,2 @@
+#!/bin/bash
+@PYTHON3_INTERPRETER@ "@abs_top_builddir@/tests/runtestdefaultsupprs.py"
index 78898f3..04429b6 100755 (executable)
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 # -*- Mode: Python
 #
diff --git a/tests/runtestfedabipkgdiffpy3.sh.in b/tests/runtestfedabipkgdiffpy3.sh.in
new file mode 100644 (file)
index 0000000..209037c
--- /dev/null
@@ -0,0 +1,22 @@
+#!/bin/bash -e
+
+# Either tests runner script or the tools/fedabipkgdiff has shebang
+# `/usr/bin/env python`, as a result, to run tests in Python 3, we have to
+# change PATH in order to make sure python can be found before the current
+# $PATH.
+
+PY3_TEMP=$(mktemp -d --tmpdir libabigail-py3-temp-XXXXXXXX)
+
+ln -s $(which @PYTHON3_INTERPRETER@) $PY3_TEMP/python
+
+export PATH=$PY3_TEMP:$PATH
+
+function clean_env
+{
+    unlink $PY3_TEMP/python
+    rmdir $PY3_TEMP
+}
+
+trap "clean_env" EXIT
+
+@PYTHON3_INTERPRETER@ "@abs_top_builddir@/tests/runtestfedabipkgdiff.py"
index da303ad..2191613 100755 (executable)
 #
 # Author: Chenxiong Qi
 
+from __future__ import print_function
+
 import argparse
+import functools
 import glob
 import logging
 import mimetypes
 import os
 import re
 import shutil
+import six
 import subprocess
 import sys
 
@@ -226,8 +230,8 @@ def cmp_nvr(left, right):
     equal to, or larger than the right individually.
     :rtype: int
     """
-    left_nvr = koji.parse_NVR(left)
-    right_nvr = koji.parse_NVR(right)
+    left_nvr = koji.parse_NVR(left['nvr'])
+    right_nvr = koji.parse_NVR(right['nvr'])
     return rpm.labelCompare(
         (left_nvr['epoch'], left_nvr['version'], left_nvr['release']),
         (right_nvr['epoch'], right_nvr['version'], right_nvr['release']))
@@ -461,7 +465,8 @@ class RPMCollection(object):
         self.ancillary_rpms = {}
 
         if rpms:
-            map(self.add, rpms)
+            for rpm in rpms:
+                self.add(rpm)
 
     @classmethod
     def gather_from_dir(cls, rpm_file, all_rpms=None):
@@ -517,8 +522,7 @@ class RPMCollection(object):
 
     def rpms_iter(self, arches=None, default_behavior=True):
         """Iterator of RPMs to go through RPMs with specific arches"""
-        arches = self.rpms.keys()
-        arches.sort()
+        arches = sorted(self.rpms.keys())
 
         for arch in arches:
             for _rpm in self.rpms[arch]:
@@ -561,7 +565,7 @@ class RPMCollection(object):
             if r.is_debuginfo:
                 result.append(r)
         return result
-    
+
 
 def generate_comparison_halves(rpm_col1, rpm_col2):
     """Iterate RPM collection and peer's to generate comparison halves"""
@@ -739,7 +743,7 @@ class Brew(object):
             raise TypeError(
                 '{0} is not a callable object.'.format(str(selector)))
 
-        if order_by is not None and not isinstance(order_by, basestring):
+        if order_by is not None and not isinstance(order_by, six.string_types):
             raise TypeError('order_by {0} is invalid.'.format(order_by))
 
         builds = self.session.listBuilds(packageID=packageID, state=state)
@@ -748,11 +752,16 @@ class Brew(object):
         if order_by is not None:
             # FIXME: is it possible to sort builds by using opts parameter of
             # listBuilds
-            cmp_func = cmp_nvr if order_by == 'nvr' else None
-            builds = sorted(builds,
-                            key=lambda item: item[order_by],
-                            cmp=cmp_func,
-                            reverse=reverse)
+            if order_by == 'nvr':
+                if six.PY2:
+                    builds = sorted(builds, cmp=cmp_nvr, reverse=reverse)
+                else:
+                    builds = sorted(builds,
+                                    key=functools.cmp_to_key(cmp_nvr),
+                                    reverse=reverse)
+            else:
+                builds = sorted(
+                    builds, key=lambda b: b[order_by], reverse=reverse)
         if topone:
             builds = builds[0:1]
 
@@ -974,7 +983,7 @@ def download_rpm(url):
         url, os.path.join(get_download_dir(),
                           os.path.basename(url)))
     if global_config.dry_run:
-        print 'DRY-RUN:', cmd
+        print('DRY-RUN: {0}'.format(cmd))
         return
 
     return_code = subprocess.call(cmd, shell=True)
@@ -997,7 +1006,8 @@ def download_rpms(rpms):
             logger.debug('Download %s', rpm.download_url)
             download_rpm(rpm.download_url)
 
-    map(_download, rpms)
+    for rpm in rpms:
+        _download(rpm)
 
 
 @log_call
@@ -1019,7 +1029,7 @@ def build_path_to_abipkgdiff():
 def format_debug_info_pkg_options(option, debuginfo_list):
     """Given a list of debug info package descriptors return an option
     string that looks like:
-    
+
        option dbg.rpm1 option dbgrpm2 ...
 
     :param: list debuginfo_list a list of instances of the RPM class
@@ -1121,20 +1131,21 @@ def abipkgdiff(cmp_half1, cmp_half2):
         cmp_half1.subject.downloaded_file,
         cmp_half2.subject.downloaded_file,
     ]
-    cmd = filter(lambda s: s != '', cmd)
+    cmd = [s for s in cmd if s != '']
 
     if global_config.dry_run:
-        print 'DRY-RUN:', ' '.join(cmd)
+        print('DRY-RUN: {0}'.format(' '.join(cmd)))
         return
 
     logger.debug('Run: %s', ' '.join(cmd))
 
-    print 'Comparing the ABI of binaries between {0} and {1}:'.format(
-        cmp_half1.subject.filename, cmp_half2.subject.filename)
-    print
+    print('Comparing the ABI of binaries between {0} and {1}:'.format(
+        cmp_half1.subject.filename, cmp_half2.subject.filename))
+    print()
 
     proc = subprocess.Popen(' '.join(cmd), shell=True,
-                            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                            universal_newlines=True)
     # So we could have done: stdout, stderr = proc.communicate()
     # But then the documentatin of proc.communicate says:
     #
@@ -1160,9 +1171,9 @@ def abipkgdiff(cmp_half1, cmp_half2):
     has_abi_change = proc.returncode & ABIDIFF_ABI_CHANGE
 
     if is_internal_error:
-        print >>sys.stderr, stderr
+        six.print_(stderr, file=sys.stderr)
     elif is_ok or has_abi_change:
-        print stdout
+        print(stdout)
 
     return proc.returncode
 
@@ -1555,7 +1566,7 @@ def main():
             if both_nvr or both_nvra:
                 return diff_two_nvras_from_koji()
 
-    print >>sys.stderr, 'Unknown arguments. Please refer to --help.'
+    six.print_('Unknown arguments. Please refer to --help.', file=sys.stderr)
     return 1
 
 
@@ -1568,7 +1579,7 @@ if __name__ == '__main__':
         if global_config.debug:
             logger.debug('Terminate by user')
         else:
-            print >>sys.stderr, 'Terminate by user'
+            six.print_('Terminate by user', file=sys.stderr)
         if global_config.show_traceback:
             raise
         else:
@@ -1579,7 +1590,7 @@ if __name__ == '__main__':
         if global_config.debug:
             logger.debug(str(e))
         else:
-            print >>sys.stderr, str(e)
+            six.print_(str(e), file=sys.stderr)
         if global_config.show_traceback:
             raise
         else: