* [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 @ 2018-01-01 0:00 cqi 2018-01-01 0:00 ` Dodji Seketeli 0 siblings, 1 reply; 4+ messages in thread From: cqi @ 2018-01-01 0:00 UTC (permalink / raw) To: libabigail; +Cc: Chenxiong Qi From: Chenxiong Qi <cqi@redhat.com> This patch makes fedabipkgdiff Python 3 compatible. All tests written in Python are updated and compatible with Python 3 as well. Tests run with Python 3 by default. It could be disabled by a new option --disable-py3-tests if it is unnecessary. * configure.ac: Add new option --disable-py3-tests. Add new test runner file tests/runtestdefaultsupprs-py3 and tests/runtestfedabipkgdiff-py3. Add required six Python module. * tests/Makefile.am: Add new test files tests/runtestdefaultsupprs-py3 and tests/runtestfedabipkgdiff-py3 according to the new option --disable-py3-tests. * tests/mockfedabipkgdiff.in: Convert print statement to six.print_. * tests/runtestdefaultsupprs-py3.in: New shell script to run test runtestdefaultsupprs with Python 3. * tests/runtestdefaultsupprs.py.in: Remove trailing semicolons. Repalce tabs with proper number of spaces. * tests/runtestfedabipkgdiff-py3.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. * tests/update-test-output.py: Likewise. And convert print statement to the print function call which is available after importing print_function from __future__ module. * 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. Signed-off-by: Chenxiong Qi <cqi@redhat.com> --- configure.ac | 20 ++++++++++++- tests/Makefile.am | 14 +++++++++ tests/mockfedabipkgdiff.in | 19 ++++++------ tests/runtestdefaultsupprs-py3.in | 2 ++ tests/runtestdefaultsupprs.py.in | 62 ++++++++++++++++++++------------------- tests/runtestfedabipkgdiff-py3.in | 2 ++ tests/runtestfedabipkgdiff.py.in | 2 +- tests/update-test-output.py | 8 +++-- tools/fedabipkgdiff | 33 ++++++++++++--------- 9 files changed, 104 insertions(+), 58 deletions(-) create mode 100644 tests/runtestdefaultsupprs-py3.in create mode 100644 tests/runtestfedabipkgdiff-py3.in diff --git a/configure.ac b/configure.ac index 4963ee5..9bc92f2 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,12 @@ AC_ARG_ENABLE([fedabipkgdiff], ENABLE_FEDABIPKGDIFF=$enableval, ENABLE_FEDABIPKGDIFF=auto) +AC_ARG_ENABLE([py3-tests], + AS_HELP_STRING([--disable-py3-tests=yes|no|auto], + [disable running tests with Python 3]), + DISABLE_PY3_TESTS=$enableval, + DISABLE_PY3_TESTS=auto) + dnl ************************************************* dnl check for dependencies dnl ************************************************* @@ -351,7 +357,7 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\ argparse logging os re subprocess sys urlparse \ - xdg koji mock rpm imp tempfile mimetypes shutil" + 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], @@ -399,6 +405,14 @@ fi AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes) + +AC_MSG_NOTICE(Run tests with Python 3... $DISABLE_PY3_TESTS) +if test x$DISABLE_PY3_TESTS = xauto -o x$DISABLE_PY3_TESTS = xyes; then + ENABLE_PY3_TESTS=yes +fi + +AM_CONDITIONAL(ENABLE_PY3_TESTS, test x$ENABLE_PY3_TESTS = xyes) + dnl Check for dependency: libzip LIBZIP_VERSION=0.10.1 @@ -556,8 +570,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/runtestfedabipkgdiff-py3], + [chmod +x tests/runtestfedabipkgdiff-py3]) AC_CONFIG_FILES([tests/runtestdefaultsupprs.py], [chmod +x tests/runtestdefaultsupprs.py]) +AC_CONFIG_FILES([tests/runtestdefaultsupprs-py3], + [chmod +x tests/runtestdefaultsupprs-py3]) AC_OUTPUT diff --git a/tests/Makefile.am b/tests/Makefile.am index 0a7f4b9..9b31e98 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -20,6 +20,9 @@ FEDABIPKGDIFF_TEST = if ENABLE_FEDABIPKGDIFF FEDABIPKGDIFF_TEST += runtestfedabipkgdiff.py endif +if ENABLE_PY3_TESTS +FEDABIPKGDIFF_TEST += runtestfedabipkgdiff-py3 +endif TESTS= \ $(FEDABIPKGDIFF_TEST) \ @@ -43,10 +46,15 @@ runtestabidiffexit \ runtestdefaultsupprs.py \ $(CXX11_TESTS) +if ENABLE_PY3_TESTS +TESTS += runtestdefaultsupprs-py3 +endif + EXTRA_DIST = \ runtestcanonicalizetypes.sh.in \ runtestfedabipkgdiff.py.in \ +runtestfedabipkgdiff-py3.in \ mockfedabipkgdiff.in \ test-valgrind-suppressions.supp @@ -136,9 +144,15 @@ runtestcanonicalizetypes.sh$(EXEEXT): runtestfedabipkgdiff_py_SOURCES = runtestfedabipkgdiff.py$(EXEEXT): +runtestfedabipkgdiff_py3_SOURCES = +runtestfedabipkgdiff-py3$(EXEEXT): + runtestdefaultsupprs_py_SOURCES = runtestdefaultsupprs.py$(EXEEXT): +runtestdefaultsupprs_py3_SOURCES = +runtestdefaultsupprs-py3$(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 aac99d1..1168b92 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' @@ -246,29 +247,29 @@ rpms = [ }, # RPMs of build vte291-0.39.1-1.fc22 {'build_id': 600011, - 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', + 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.1-1.fc22', }, {'build_id': 600011, - 'name': 'vte291-devel', 'version': '0.39.1', 'release': '1.fc22', + 'name': 'vte291-devel', 'version': '0.39.1', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.1-1.fc22', }, {'build_id': 600011, - 'name': 'vte291-debuginfo', 'version': '0.39.1', 'release': '1.fc22', + 'name': 'vte291-debuginfo', 'version': '0.39.1', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.1-1.fc22', }, # RPMs of build vte291-0.39.90-1.fc22 {'build_id': 612610, - 'name': 'vte291', 'version': '0.39.90', 'release': '1.fc22', + 'name': 'vte291', 'version': '0.39.90', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.90-1.fc22', }, {'build_id': 612610, - 'name': 'vte291-devel', 'version': '0.39.90', 'release': '1.fc22', + 'name': 'vte291-devel', 'version': '0.39.90', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.90-1.fc22', }, {'build_id': 612610, - 'name': 'vte291-debuginfo', 'version': '0.39.90', 'release': '1.fc22', + 'name': 'vte291-debuginfo', 'version': '0.39.90', 'release': '1.fc22', 'arch': 'x86_64', 'nvr': 'vte291-0.39.90-1.fc22', }, ] diff --git a/tests/runtestdefaultsupprs-py3.in b/tests/runtestdefaultsupprs-py3.in new file mode 100644 index 0000000..7b856d0 --- /dev/null +++ b/tests/runtestdefaultsupprs-py3.in @@ -0,0 +1,2 @@ +#!/bin/bash +python3 "@abs_top_builddir@/tests/runtestdefaultsupprs.py" \ No newline at end of file diff --git a/tests/runtestdefaultsupprs.py.in b/tests/runtestdefaultsupprs.py.in index 41b0b28..9feafe7 100644 --- a/tests/runtestdefaultsupprs.py.in +++ b/tests/runtestdefaultsupprs.py.in @@ -67,7 +67,8 @@ def ensure_output_dir_created(): pass if not os.path.exists(output_dir): - sys.exit(1); + sys.exit(1) + def run_abidiff_tests(): """Run the abidiff default suppression tests. @@ -90,9 +91,9 @@ def run_abidiff_tests(): default_suppression = output_dir + "/default.abignore" with open(default_suppression, 'w') as out: - out.write('\n'); + out.write('\n') - result = 0; + result = 0 for test_spec in abidiff_test_specs: binary1 = test_spec[0] binary2 = test_spec[1] @@ -111,40 +112,40 @@ def run_abidiff_tests(): # The environment variables that say where to find the default # suppression specifications loaded by libabigail. envs = { - 'LIBABIGAIL_DEFAULT_SYSTEM_SUPPRESSION_FILE' : default_suppression, - 'LIBABIGAIL_DEFAULT_USER_SUPPRESSION_FILE' : default_suppression + 'LIBABIGAIL_DEFAULT_SYSTEM_SUPPRESSION_FILE': default_suppression, + 'LIBABIGAIL_DEFAULT_USER_SUPPRESSION_FILE': default_suppression } # Initialize the environment variables above to their default # value. for name, value in envs.items(): - os.environ[name] = value; + os.environ[name] = value for env_name in envs: env_vars = os.environ 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"); + "for env var '" + e + "'\n") - del env_vars[env_name]; + del env_vars[env_name] try: os.remove(default_suppression) except: pass - return result; + return result + def run_abipkgdiff_tests(): """Run the abipkgdiff default suppression tests. @@ -167,9 +168,9 @@ def run_abipkgdiff_tests(): default_suppression = output_dir + "/default.abignore" with open(default_suppression, 'w') as out: - out.write('\n'); + out.write('\n') - result = 0; + result = 0 for test_spec in abipkgdiff_test_specs: pkg1 = test_spec[0] pkg2 = test_spec[1] @@ -188,40 +189,40 @@ def run_abipkgdiff_tests(): # The environment variables that say where to find the default # suppression specifications loaded by libabigail. envs = { - 'LIBABIGAIL_DEFAULT_SYSTEM_SUPPRESSION_FILE' : default_suppression, - 'LIBABIGAIL_DEFAULT_USER_SUPPRESSION_FILE' : default_suppression + 'LIBABIGAIL_DEFAULT_SYSTEM_SUPPRESSION_FILE': default_suppression, + 'LIBABIGAIL_DEFAULT_USER_SUPPRESSION_FILE': default_suppression } # Initialize the environment variables above to their default # value. for name, value in envs.items(): - os.environ[name] = value; + os.environ[name] = value for env_name in envs: env_vars = os.environ 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"); + "for env var '" + e + "'\n") - del env_vars[env_name]; + del env_vars[env_name] try: os.remove(default_suppression) except: pass - return result; + return result + def main(): """The main entry point of this program. @@ -235,12 +236,13 @@ def main(): """ ensure_output_dir_created() - result = 0; + result = 0 result = run_abidiff_tests() if result: - return result; + return result result = run_abipkgdiff_tests() - return result; + return result + if __name__ == '__main__': exit_code = main() diff --git a/tests/runtestfedabipkgdiff-py3.in b/tests/runtestfedabipkgdiff-py3.in new file mode 100644 index 0000000..b4cbe78 --- /dev/null +++ b/tests/runtestfedabipkgdiff-py3.in @@ -0,0 +1,2 @@ +#!/bin/bash +python3 "@abs_top_builddir@/tests/runtestfedabipkgdiff.py" \ No newline at end of file diff --git a/tests/runtestfedabipkgdiff.py.in b/tests/runtestfedabipkgdiff.py.in index 78898f3..04429b6 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/update-test-output.py b/tests/update-test-output.py index 4017dd0..84afd4e 100755 --- a/tests/update-test-output.py +++ b/tests/update-test-output.py @@ -1,14 +1,16 @@ -#!/bin/python +#!/usr/bin/env python # This program generates the copy commands you should use to update # the reference data for tests <build-dir>/tests/runtest* that emit an # output that is compared against a reference output. -# +# # It takes in argument the diff result emitted by # <build-dir>/tests/runtest*, and emits on standard output a series of # 'cp <src> <dest>' commands to execute to update reference data of # the test. +from __future__ import print_function + import fileinput import re import sys @@ -31,7 +33,7 @@ def main(): try: opts, args = getopt.getopt(sys.argv[1:], "hi", ["help"]) except getopt.GetoptError as err: - print str(err) + print(str(err)) display_usage() for opt, arg in opts: diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff index da303ad..5500104 100755 --- a/tools/fedabipkgdiff +++ b/tools/fedabipkgdiff @@ -22,6 +22,8 @@ # # Author: Chenxiong Qi +from __future__ import print_function + import argparse import glob import logging @@ -39,6 +41,7 @@ import xdg.BaseDirectory import rpm import koji +import six # @file # @@ -461,7 +464,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): @@ -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""" @@ -974,7 +978,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:', cmd) return return_code = subprocess.call(cmd, shell=True) @@ -997,7 +1001,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 +1024,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 @@ -1124,14 +1129,14 @@ def abipkgdiff(cmp_half1, cmp_half2): cmd = filter(lambda s: s != '', cmd) if global_config.dry_run: - print 'DRY-RUN:', ' '.join(cmd) + print('DRY-RUN:', ' '.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) @@ -1160,9 +1165,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 +1560,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 +1573,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 +1584,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.14.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 2018-01-01 0:00 [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 cqi @ 2018-01-01 0:00 ` Dodji Seketeli 2018-01-01 0:00 ` Chenxiong Qi 0 siblings, 1 reply; 4+ messages in thread From: Dodji Seketeli @ 2018-01-01 0:00 UTC (permalink / raw) To: cqi; +Cc: libabigail Hello Chenxiong, Thank you very much for putting this patch together, it's really appreciated. I have a few comments about the patch though, please find them below. [...] > diff --git a/configure.ac b/configure.ac > index 4963ee5..9bc92f2 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -97,6 +97,12 @@ AC_ARG_ENABLE([fedabipkgdiff], > ENABLE_FEDABIPKGDIFF=$enableval, > ENABLE_FEDABIPKGDIFF=auto) > > +AC_ARG_ENABLE([py3-tests], > + AS_HELP_STRING([--disable-py3-tests=yes|no|auto], > + [disable running tests with Python 3]), What AC_ARG_ENABLE does here is to define both --enable-py3-test *and* --disable-py3-test options. So, I'd prefer that in the help string we refer to the positive option, which is --enable-py3-test. That positive option is of course "applied" by default in this case. > + DISABLE_PY3_TESTS=$enableval, > + DISABLE_PY3_TESTS=auto) So here, I'd prefer that we use a "positive" variable name, which would be ENABLE_PY3_TESTS, rather than the DISABLE_PY3_TESTS that you are using. This would be just like what is done above for the other options. > + > dnl ************************************************* > dnl check for dependencies > dnl ************************************************* > @@ -351,7 +357,7 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then > > REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\ > argparse logging os re subprocess sys urlparse \ > - xdg koji mock rpm imp tempfile mimetypes shutil" > + xdg koji mock rpm imp tempfile mimetypes shutil six" Just for my own education, is the python six package available on el6 systems? > > if test x$ENABLE_FEDABIPKGDIFF != xno; then > AX_CHECK_PYTHON_MODULES([$REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF], > @@ -399,6 +405,14 @@ fi > > AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes) > > + > +AC_MSG_NOTICE(Run tests with Python 3... $DISABLE_PY3_TESTS) > +if test x$DISABLE_PY3_TESTS = xauto -o x$DISABLE_PY3_TESTS = xyes; then > + ENABLE_PY3_TESTS=yes Hmmh. I disagree with the logic here. I think the logic should be that if we are in 'auto' mode and if python3 is available, then we should enable the python3 tests. If we are in auto mode and if python3 is not available, then we should *NOT* enable the python3 tests. We should thus assume python2 mode. If on the other hand the user chose to explicitly disable or enable the python3 mode (i.e, not choosing 'auto') then we should disable or enable python3 as per the user request. [...] > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 0a7f4b9..9b31e98 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -20,6 +20,9 @@ FEDABIPKGDIFF_TEST = > if ENABLE_FEDABIPKGDIFF > FEDABIPKGDIFF_TEST += runtestfedabipkgdiff.py > endif > +if ENABLE_PY3_TESTS > +FEDABIPKGDIFF_TEST += runtestfedabipkgdiff-py3 > +endif I think this should be included in the body of the "if ENABLE_FEDABIPKGDIFF" clause above. > > TESTS= \ > $(FEDABIPKGDIFF_TEST) \ > @@ -43,10 +46,15 @@ runtestabidiffexit \ > runtestdefaultsupprs.py \ > $(CXX11_TESTS) > > +if ENABLE_PY3_TESTS > +TESTS += runtestdefaultsupprs-py3 > +endif > + I think this is unnecessary if my previous comment is right. [...] > diff --git a/tests/mockfedabipkgdiff.in b/tests/mockfedabipkgdiff.in > index aac99d1..1168b92 100644 > --- a/tests/mockfedabipkgdiff.in > +++ b/tests/mockfedabipkgdiff.in [...] > {'build_id': 600011, > - 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', > + 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', I believe these are (white space) changes that are unrelated to the purpose of the patch, which is to move to python3. I am correct? If so, please file a separate patch that contains these white space changes. There are plenty of hunks following this one, which similar white space changes, please include them in that white-space/style patch. [...] > diff --git a/tests/runtestdefaultsupprs.py.in b/tests/runtestdefaultsupprs.py.in > index 41b0b28..9feafe7 100644 > --- a/tests/runtestdefaultsupprs.py.in > +++ b/tests/runtestdefaultsupprs.py.in > @@ -67,7 +67,8 @@ def ensure_output_dir_created(): > pass > > if not os.path.exists(output_dir): > - sys.exit(1); > + sys.exit(1) > + Just like I said above concerning the white space changes, I think this change would be better suited to a separate "style-only" patch. There are similar other changes after this one that should be included in that separate patch as well. [...] Thanks! -- Dodji ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 2018-01-01 0:00 ` Dodji Seketeli @ 2018-01-01 0:00 ` Chenxiong Qi 2018-01-01 0:00 ` Dodji Seketeli 0 siblings, 1 reply; 4+ messages in thread From: Chenxiong Qi @ 2018-01-01 0:00 UTC (permalink / raw) To: Dodji Seketeli; +Cc: libabigail [-- Attachment #1: Type: text/plain, Size: 5205 bytes --] Hi Dodji, My patch is updated. Please take another look. This time it also contains a few replacement from tab to spaces which are detected during running in Python 3. On Wed, Jan 31, 2018 at 7:37 PM, Dodji Seketeli <dodji@seketeli.org> wrote: > Hello Chenxiong, > > Thank you very much for putting this patch together, it's really > appreciated. > > I have a few comments about the patch though, please find them below. > > [...] > >> diff --git a/configure.ac b/configure.ac >> index 4963ee5..9bc92f2 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -97,6 +97,12 @@ AC_ARG_ENABLE([fedabipkgdiff], >> ENABLE_FEDABIPKGDIFF=$enableval, >> ENABLE_FEDABIPKGDIFF=auto) >> >> +AC_ARG_ENABLE([py3-tests], >> + AS_HELP_STRING([--disable-py3-tests=yes|no|auto], >> + [disable running tests with Python 3]), > > What AC_ARG_ENABLE does here is to define both --enable-py3-test *and* > --disable-py3-test options. So, I'd prefer that in the help string we > refer to the positive option, which is --enable-py3-test. That > positive option is of course "applied" by default in this case. > > >> + DISABLE_PY3_TESTS=$enableval, >> + DISABLE_PY3_TESTS=auto) > > So here, I'd prefer that we use a "positive" variable name, which > would be ENABLE_PY3_TESTS, rather than the DISABLE_PY3_TESTS that you > are using. This would be just like what is done above for the other > options. > >> + >> dnl ************************************************* >> dnl check for dependencies >> dnl ************************************************* >> @@ -351,7 +357,7 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then >> >> REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\ >> argparse logging os re subprocess sys urlparse \ >> - xdg koji mock rpm imp tempfile mimetypes shutil" >> + xdg koji mock rpm imp tempfile mimetypes shutil six" > > Just for my own education, is the python six package available on el6 > systems? > >> >> if test x$ENABLE_FEDABIPKGDIFF != xno; then >> AX_CHECK_PYTHON_MODULES([$REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF], >> @@ -399,6 +405,14 @@ fi >> >> AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes) >> >> + >> +AC_MSG_NOTICE(Run tests with Python 3... $DISABLE_PY3_TESTS) >> +if test x$DISABLE_PY3_TESTS = xauto -o x$DISABLE_PY3_TESTS = xyes; then >> + ENABLE_PY3_TESTS=yes > > Hmmh. I disagree with the logic here. > > I think the logic should be that if we are in 'auto' mode and if > python3 is available, then we should enable the python3 tests. If we > are in auto mode and if python3 is not available, then we should *NOT* > enable the python3 tests. We should thus assume python2 mode. > > If on the other hand the user chose to explicitly disable or enable > the python3 mode (i.e, not choosing 'auto') then we should disable or > enable python3 as per the user request. > > [...] > >> diff --git a/tests/Makefile.am b/tests/Makefile.am >> index 0a7f4b9..9b31e98 100644 >> --- a/tests/Makefile.am >> +++ b/tests/Makefile.am >> @@ -20,6 +20,9 @@ FEDABIPKGDIFF_TEST = >> if ENABLE_FEDABIPKGDIFF >> FEDABIPKGDIFF_TEST += runtestfedabipkgdiff.py >> endif >> +if ENABLE_PY3_TESTS >> +FEDABIPKGDIFF_TEST += runtestfedabipkgdiff-py3 >> +endif > > I think this should be included in the body of the "if > ENABLE_FEDABIPKGDIFF" clause above. > >> >> TESTS= \ >> $(FEDABIPKGDIFF_TEST) \ >> @@ -43,10 +46,15 @@ runtestabidiffexit \ >> runtestdefaultsupprs.py \ >> $(CXX11_TESTS) >> >> +if ENABLE_PY3_TESTS >> +TESTS += runtestdefaultsupprs-py3 >> +endif >> + > > I think this is unnecessary if my previous comment is right. > > [...] > >> diff --git a/tests/mockfedabipkgdiff.in b/tests/mockfedabipkgdiff.in >> index aac99d1..1168b92 100644 >> --- a/tests/mockfedabipkgdiff.in >> +++ b/tests/mockfedabipkgdiff.in > > [...] > >> {'build_id': 600011, >> - 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', >> + 'name': 'vte291', 'version': '0.39.1', 'release': '1.fc22', > > I believe these are (white space) changes that are unrelated to the purpose of > the patch, which is to move to python3. I am correct? > > If so, please file a separate patch that contains these white space > changes. There are plenty of hunks following this one, which similar > white space changes, please include them in that white-space/style > patch. > > [...] > >> diff --git a/tests/runtestdefaultsupprs.py.in b/tests/runtestdefaultsupprs.py.in >> index 41b0b28..9feafe7 100644 >> --- a/tests/runtestdefaultsupprs.py.in >> +++ b/tests/runtestdefaultsupprs.py.in >> @@ -67,7 +67,8 @@ def ensure_output_dir_created(): >> pass >> >> if not os.path.exists(output_dir): >> - sys.exit(1); >> + sys.exit(1) >> + > > Just like I said above concerning the white space changes, I think > this change would be better suited to a separate "style-only" patch. > There are similar other changes after this one that should be included > in that separate patch as well. > > [...] > > Thanks! > > > -- > Dodji -- Regards, Chenxiong Qi [-- Attachment #2: 0001-Bug-22722-Make-fedabipkgdiff-and-its-tests-support-b.patch --] [-- Type: text/x-patch, Size: 19452 bytes --] From d53b8dd2920c46af58131072a5296121a0b55978 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi <cqi@redhat.com> 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. It defaults to look for a Python 3 interpreter, if there is such one, run tests with found interpreter, if there is no one, tests will run only with Python 2. But, this behavior can be disabled by --enable-py3-tests=no. * configure.ac: Add new option --enable-py3-tests. Add new test runner file tests/runtestdefaultsupprs-py3 and tests/runtestfedabipkgdiff-py3. Add required six Python module. * tests/Makefile.am: Add new test files tests/runtestdefaultsupprs-py3 and tests/runtestfedabipkgdiff-py3 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/runtestdefaultsupprs-py3.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/runtestfedabipkgdiff-py3.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> --- .gitignore | 3 +- configure.ac | 39 +++++++++++++++++++++++- tests/Makefile.am | 20 +++++++++++++ tests/mockfedabipkgdiff.in | 21 ++++++------- tests/runtestdefaultsupprs-py3.in | 2 ++ tests/runtestdefaultsupprs.py.in | 24 +++++++-------- tests/runtestfedabipkgdiff-py3.in | 22 ++++++++++++++ tests/runtestfedabipkgdiff.py.in | 2 +- tools/fedabipkgdiff | 63 +++++++++++++++++++++++---------------- 9 files changed, 144 insertions(+), 52 deletions(-) create mode 100644 tests/runtestdefaultsupprs-py3.in create mode 100644 tests/runtestfedabipkgdiff-py3.in diff --git a/.gitignore b/.gitignore index a60cadb..ed4f43c 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 4738faa..762c8e8 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,12 @@ AC_ARG_ENABLE([fedabipkgdiff], ENABLE_FEDABIPKGDIFF=$enableval, ENABLE_FEDABIPKGDIFF=auto) +AC_ARG_ENABLE([py3-tests], + AS_HELP_STRING([--enable-py3-tests=yes|no|auto], + [enable to run tests with Python 3 (default is auto)]), + ENABLE_PY3_TESTS=$enableval, + ENABLE_PY3_TESTS=auto) + dnl ************************************************* dnl check for dependencies dnl ************************************************* @@ -351,7 +357,7 @@ if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then REQUIRED_PYTHON_MODULES_FOR_FEDABIPKGDIFF="\ argparse logging os re subprocess sys urlparse \ - xdg koji mock rpm imp tempfile mimetypes shutil" + 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], @@ -399,6 +405,33 @@ fi AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes) + +# The minimal python 3 version we want to support is 3.5, which is +# available in Fedora releases and the version that is available in EL7. +AC_CHECK_PROGS(PYTHON3_INTERPRETER, [python3 python3.5 python3.6 python3.7], no) + +AC_MSG_NOTICE(Run tests with Python 3... $ENABLE_PY3_TESTS) +if test x$ENABLE_PY3_TESTS = xauto; then + if test x$PYTHON3_INTERPRETER != xno; then + RUN_TESTS_WITH_PY3=yes + else + # When enabling py3 tests is auto, tests only run if python3 + # interpreter is found from system. Otherwise, just ignore it. + AC_MSG_NOTICE([Python 3 is not found. Skip to run tests with Python 3.]) + fi +fi +if test x$ENABLE_PY3_TESTS = xyes; then + if test x$PYTHON3_INTERPRETER != xno; then + RUN_TESTS_WITH_PY3=yes + else + AC_MSG_ERROR([Python 3 is not found.]) + fi +fi + +AM_CONDITIONAL(RUN_TESTS_WITH_PY3, test x$RUN_TESTS_WITH_PY3 = xyes) +AM_CONDITIONAL(PYTHON3_INTERPRETER, test x$PYTHON3_INTERPRETER != xno) + + dnl Check for dependency: libzip LIBZIP_VERSION=0.10.1 @@ -556,8 +589,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/runtestfedabipkgdiff-py3], + [chmod +x tests/runtestfedabipkgdiff-py3]) AC_CONFIG_FILES([tests/runtestdefaultsupprs.py], [chmod +x tests/runtestdefaultsupprs.py]) +AC_CONFIG_FILES([tests/runtestdefaultsupprs-py3], + [chmod +x tests/runtestdefaultsupprs-py3]) AC_OUTPUT diff --git a/tests/Makefile.am b/tests/Makefile.am index 0a7f4b9..e528aa4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -19,6 +19,9 @@ endif FEDABIPKGDIFF_TEST = if ENABLE_FEDABIPKGDIFF FEDABIPKGDIFF_TEST += runtestfedabipkgdiff.py +if RUN_TESTS_WITH_PY3 +FEDABIPKGDIFF_TEST += runtestfedabipkgdiff-py3 +endif endif TESTS= \ @@ -43,6 +46,9 @@ runtestabidiffexit \ runtestdefaultsupprs.py \ $(CXX11_TESTS) +if RUN_TESTS_WITH_PY3 +TESTS += runtestdefaultsupprs-py3 +endif EXTRA_DIST = \ runtestcanonicalizetypes.sh.in \ @@ -50,6 +56,12 @@ runtestfedabipkgdiff.py.in \ mockfedabipkgdiff.in \ test-valgrind-suppressions.supp +if RUN_TESTS_WITH_PY3 +EXTRA_DIST += \ +runtestfedabipkgdiff-py3.in \ +runtestdefaultsupprs-py3.in +endif + CLEANFILES = \ runtestcanonicalizetypes.output.txt \ runtestcanonicalizetypes.output.final.txt @@ -139,6 +151,14 @@ runtestfedabipkgdiff.py$(EXEEXT): runtestdefaultsupprs_py_SOURCES = runtestdefaultsupprs.py$(EXEEXT): +if RUN_TESTS_WITH_PY3 +runtestfedabipkgdiff_py3_SOURCES = +runtestfedabipkgdiff-py3$(EXEEXT): + +runtestdefaultsupprs_py3_SOURCES = +runtestdefaultsupprs-py3$(EXEEXT): +endif + 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 aac99d1..47c8cc8 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-py3.in b/tests/runtestdefaultsupprs-py3.in new file mode 100644 index 0000000..4985206 --- /dev/null +++ b/tests/runtestdefaultsupprs-py3.in @@ -0,0 +1,2 @@ +#!/bin/bash +@PYTHON3_INTERPRETER@ "@abs_top_builddir@/tests/runtestdefaultsupprs.py" diff --git a/tests/runtestdefaultsupprs.py.in b/tests/runtestdefaultsupprs.py.in index 41b0b28..10b71d1 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/runtestfedabipkgdiff-py3.in b/tests/runtestfedabipkgdiff-py3.in new file mode 100644 index 0000000..209037c --- /dev/null +++ b/tests/runtestfedabipkgdiff-py3.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/tests/runtestfedabipkgdiff.py.in b/tests/runtestfedabipkgdiff.py.in index 78898f3..04429b6 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/tools/fedabipkgdiff b/tools/fedabipkgdiff index da303ad..2191613 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.14.3 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 2018-01-01 0:00 ` Chenxiong Qi @ 2018-01-01 0:00 ` Dodji Seketeli 0 siblings, 0 replies; 4+ messages in thread From: Dodji Seketeli @ 2018-01-01 0:00 UTC (permalink / raw) To: Chenxiong Qi; +Cc: libabigail [-- Attachment #1: Type: text/plain, Size: 1522 bytes --] Hello Chenxiong, Chenxiong Qi <qcxhome@gmail.com> a écrit: > Hi Dodji, > > My patch is updated. Please take another look. This time it also > contains a few replacement from tab to spaces which are detected > during running in Python 3. Thanks! So I reviewed it, found it great and so I committed it! I just made some changes in the autotool-related parts to make it so that the logic is to "enable fedabpkgdiff to run on python3". Not just the tests. So the option is now --enable-python3. When that option is fired, if python3 is found, then we enable the tests to run on python3 too. I also made a change to test for the presence of required python modules using either python2 or python3, depending on the version of python that were selected. Because without my change, only python2 was being used to check for the presence of the python modules required by fedabipkgdiff. Doing that I realized, for instance, that the package urlparse was renamed in python3 as urllib.parse. So I handled that too. On a more cosmetic note, I renamed some automake conditionnal variables to make them look like the other ones we already have in place. I also updated the summary that is displayed at the end of the configure script to make it mention the path to the python binary we are using and if we are using python3 or not. So below is the patch I committed. Thank you so much for the time, effort and dedication you are showing. It really is appreciated. Cheers, [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: The committed patch --] [-- Type: text/x-patch, Size: 22714 bytes --] From 974d6b8745c425e30ca6aafa92338ccfacfededb Mon Sep 17 00:00:00 2001 From: Chenxiong Qi <cqi@redhat.com> 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.py. Add required six Python module. * tests/Makefile.am: Add new test files tests/runtestdefaultsupprspy3.py and tests/runtestfedabipkgdiffpy3.py 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.py.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.py.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 | 3 +- configure.ac | 84 ++++++++++++++++++++++++++--- tests/Makefile.am | 21 ++++++++ tests/mockfedabipkgdiff.in | 21 ++++---- tests/runtestdefaultsupprs.py.in | 24 ++++----- tests/runtestdefaultsupprspy3.py.in | 2 + tests/runtestfedabipkgdiff.py.in | 2 +- tests/runtestfedabipkgdiffpy3.py.in | 22 ++++++++ tools/fedabipkgdiff | 63 +++++++++++++--------- 9 files changed, 184 insertions(+), 58 deletions(-) create mode 100644 tests/runtestdefaultsupprspy3.py.in create mode 100644 tests/runtestfedabipkgdiffpy3.py.in diff --git a/.gitignore b/.gitignore index a60cadb..ed4f43c 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 4738faa..df495e4 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 ************************************************* @@ -333,29 +339,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]) @@ -398,6 +459,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 @@ -556,8 +620,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.py], + [chmod +x tests/runtestfedabipkgdiffpy3.py]) AC_CONFIG_FILES([tests/runtestdefaultsupprs.py], [chmod +x tests/runtestdefaultsupprs.py]) +AC_CONFIG_FILES([tests/runtestdefaultsupprspy3.py], + [chmod +x tests/runtestdefaultsupprspy3.py]) AC_OUTPUT @@ -573,6 +641,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} @@ -583,6 +652,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 0a7f4b9..30bf46b 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.py +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.py +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.py.in \ +runtestdefaultsupprspy3.py.in +endif + CLEANFILES = \ runtestcanonicalizetypes.output.txt \ runtestcanonicalizetypes.output.final.txt @@ -139,6 +152,14 @@ runtestfedabipkgdiff.py$(EXEEXT): runtestdefaultsupprs_py_SOURCES = runtestdefaultsupprs.py$(EXEEXT): +if ENABLE_RUNNING_TESTS_WITH_PY3 +runtestfedabipkgdiffpy3_py_SOURCES = +runtestfedabipkgdiffpy3.py$(EXEEXT): + +runtestdefaultsupprspy3_py_SOURCES = +runtestdefaultsupprspy3.py$(EXEEXT): +endif + 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 aac99d1..47c8cc8 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 41b0b28..10b71d1 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.py.in b/tests/runtestdefaultsupprspy3.py.in new file mode 100644 index 0000000..4985206 --- /dev/null +++ b/tests/runtestdefaultsupprspy3.py.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 78898f3..04429b6 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.py.in b/tests/runtestfedabipkgdiffpy3.py.in new file mode 100644 index 0000000..209037c --- /dev/null +++ b/tests/runtestfedabipkgdiffpy3.py.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 da303ad..2191613 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.17.0.rc1 [-- Attachment #3: Type: text/plain, Size: 14 bytes --] -- Dodji ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-29 12:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-01 0:00 [PATCH] Bug 22722 - Make fedabidiff and its tests support both python 3 and 2 cqi 2018-01-01 0:00 ` Dodji Seketeli 2018-01-01 0:00 ` Chenxiong Qi 2018-01-01 0:00 ` Dodji Seketeli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).