From 1ac0c055fa1892b4f4565fca3b13ecc3d7cf2391 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Sun, 25 Mar 2018 15:34:59 +0800 Subject: [PATCH] Bug 22722 - Make fedabipkgdiff and its tests support both python 3 and 2 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 Signed-off-by: Dodji Seketeli --- .gitignore | 3 +- configure.ac | 84 ++++++++++++++++++++++++++--- tests/Makefile.am | 19 +++++++ tests/mockfedabipkgdiff.in | 21 ++++---- tests/runtestdefaultsupprs.py.in | 24 ++++----- tests/runtestdefaultsupprspy3.sh.in | 2 + tests/runtestfedabipkgdiff.py.in | 2 +- tests/runtestfedabipkgdiffpy3.sh.in | 22 ++++++++ tools/fedabipkgdiff | 63 +++++++++++++--------- 9 files changed, 182 insertions(+), 58 deletions(-) create mode 100644 tests/runtestdefaultsupprspy3.sh.in create mode 100644 tests/runtestfedabipkgdiffpy3.sh.in diff --git a/.gitignore b/.gitignore index a60cadb9..ed4f43c2 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/configure.ac b/configure.ac index 179174ee..5ae6583c 100644 --- a/configure.ac +++ b/configure.ac @@ -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 2 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} diff --git a/tests/Makefile.am b/tests/Makefile.am index 0a7f4b99..229bc1f8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -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 diff --git a/tests/mockfedabipkgdiff.in b/tests/mockfedabipkgdiff.in index aac99d12..47c8cc82 100644 --- a/tests/mockfedabipkgdiff.in +++ b/tests/mockfedabipkgdiff.in @@ -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) diff --git a/tests/runtestdefaultsupprs.py.in b/tests/runtestdefaultsupprs.py.in index 41b0b284..10b71d1b 100644 --- a/tests/runtestdefaultsupprs.py.in +++ b/tests/runtestdefaultsupprs.py.in @@ -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 index 00000000..4985206f --- /dev/null +++ b/tests/runtestdefaultsupprspy3.sh.in @@ -0,0 +1,2 @@ +#!/bin/bash +@PYTHON3_INTERPRETER@ "@abs_top_builddir@/tests/runtestdefaultsupprs.py" diff --git a/tests/runtestfedabipkgdiff.py.in b/tests/runtestfedabipkgdiff.py.in index 78898f3c..04429b64 100755 --- a/tests/runtestfedabipkgdiff.py.in +++ b/tests/runtestfedabipkgdiff.py.in @@ -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 index 00000000..209037ce --- /dev/null +++ b/tests/runtestfedabipkgdiffpy3.sh.in @@ -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" diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff index da303ad0..2191613f 100755 --- a/tools/fedabipkgdiff +++ b/tools/fedabipkgdiff @@ -22,13 +22,17 @@ # # 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: -- 2.43.5