public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Chenxiong Qi <qcxhome@yahoo.com>
To: Dodji Seketeli <dodji@redhat.com>, Chenxiong Qi <cqi@redhat.com>
Cc: "libabigail@sourceware.org" <libabigail@sourceware.org>
Subject: Re: a new tool fedabipkgdiff
Date: Fri, 01 Jan 2016 00:00:00 -0000	[thread overview]
Message-ID: <1620325890.3371217.1461940400376.JavaMail.yahoo@mail.yahoo.com> (raw)
In-Reply-To: <868u0yzz89.fsf@seketeli.org>

[-- Attachment #1: Type: text/plain, Size: 25242 bytes --]






> On Thursday, March 31, 2016 7:03 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> > Hello Chenxiong,
> 
> Sorry for my being late to review this.  It just felt through the cracks
> while I was looking as something else.  Oh well.
> 
> So I reviewed it and I must say this is good stuff :-)
> 
> I looked at it in details and the comments I have now are mostly about
> documentations and the like.
> 
> Please find my comments below.
> 
>>      * autoconf-archive/ax_compare_version.m4: New file copied from the
> 
> Each of these lines should start with a 'tab' character (you know, the
> \t character), as specified in the file COMMIT-LOG-GUIDELINES:
> 
>     The subsequent lines should have the form of the Body of a GNU ChangeLog
>     entry, i.e:
> 
>         * file1.c (func1): Changed foo in this function.
>         (func2): Changed blah in that function
>         * file2.c (func_foo): Changed something here.
> 
>     Note that before the '*', there is a tab that is 8 spaces long.  
> Also
>     note that right after the '*', there is a space.
> 
> To ease the task, I have edited your ChangeLog.  Here is the result,
> that you can add to your patch:
> 
>     * autoconf-archive/ax_compare_version.m4: New file copied from the
>     autoconf-archive project.
>     * autoconf-archive/ax_prog_python_version.m4: Likewise.
>     * autoconf-archive/ax_python_module.m4: Likewise.
>     * Makefile.am: Add the new files above to the source distribution.
>     * configure.ac: Include the new m4 macros from the autoconf
>     archive. Add a new --enable-fedabipkgdiff option. Update the
>     report at the end of the configure process to show the status of
>     the fedabipkgdiff feature. Add check for prerequisite python
>     modules itertools, shutil, unittest and mock.  These are necessary
>     for the unit test of fedabipkgdiff. Generate
>     tests/runtestfedabipkgdiff.py into the build directory, from the
>     tests/runtestfedabipkgdiff.py.in input file.
>     * tools/Makefile.am: Include the fedabipkgdiff to the source
>     distribution and install it if the "fedabipkgdiff" feature is
>     enabled.
>     * tests/Makefile.am: Rename runtestfedabipkgdiff.sh into
>     runtestfedabipkgdiff.py.  Add the new runtestfedabipkgdiff.py.in
>     autoconf template file in here.
>     * tests/runtestfedabipkgdiff.py.in: New unit test file.
>     * tools/fedabipkgdiff: New tool fedabipkgdiff.
> [...]
> 
>>  diff --git a/tests/runtestfedabipkgdiff.py.in 
> b/tests/runtestfedabipkgdiff.py.in
>>  new file mode 100755
>>  index 0000000..3087212
>>  --- /dev/null
>>  +++ b/tests/runtestfedabipkgdiff.py.in
>>  @@ -0,0 +1,450 @@
>>  +#!/usr/bin/python
>>  +# -*- coding: utf-8 -*-
>>  +# -*- Mode: Python
>>  +#
>>  +# This file is part of the GNU Application Binary Interface Generic
>>  +# Analysis and Instrumentation Library.  This program is free
>>  +# software; you can redistribute it and/or modify it under the terms
>>  +# of the GNU General Public License as published by the Free Software
>>  +# Foundation; either version 3, or (at your option) any later version.
>>  +#
>>  +# This program is distributed in the hope that it will be useful, but
>>  +# WITHOUT ANY WARRANTY; without even the implied warranty of
>>  +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>  +# General Lesser Public License for more details.
>>  +#
>>  +# You should have received a copy of the GNU Lesser General Public
>>  +# License along with this program; see the file COPYING-LGPLV3.  If
>>  +# not, see <http://www.gnu.org/licenses/>.
> 
> This is a program, not a library.  In the project, by default,
> programs are GPL.  It's only the library that is LGPL.  So if this is
> an oversight, I guess we should fix it and change this license to GPL.
> 
>>  +#
>>  +# Author: Chenxiong Qi
>>  +
>>  +import os
>>  +import itertools
>>  +import shutil
>>  +import unittest
>>  +
>>  +import koji
> 
> In general, I find that this file lacks comments.  There should be a
> general comment at the beginning of the file that introduces the kind
> of tests that are performed here.  For instance,
> tests/test-diff-pkg.cc has:
> 
>     /// @file
>     ///
>     /// This test harness program computes the ABI changes between ELF
>     /// binaries present inside input packages.  Some of the input
>     /// packages have debuginfo, some don't.  The resulting ABI change
>     /// report is then compared with a reference one.
>     ///
>     /// The set of input files and reference reports to consider should be
>     /// present in the source distribution, which means they must be
>     /// referenced in tests/data/Makefile.am by the EXTRA_DIST variable.
> 
> And if you look into that file, there are various other comments that
> describe how the test is organized etc.  I think we should try and do
> the same for all the tests that we add.
> 
> For instance:
> 
> [...]
> 
>>  +counter = itertools.count(0)
> 
> What is this global variable for?  We should have a comment for this.
> 
> 
>>  +class UtilsTest(unittest.TestCase):
> 
> We should have an introductory comment for this, I believe.
> 
>>  +
>>  +    def test_is_fedora_distro(self):
> 
> Same for this test.  What is the philosophy behind this?  What is the
> kind of "distro strings" we want to accept? etc.
> 
>>  +        distro = 'fc5'
>>  +        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'f5'
>>  +        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'fc23'
>>  +        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'fc'
>>  +        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'fc234'
>>  +        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'el7'
>>  +        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +        distro = 'el7_2'
>>  +        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
>>  +
>>  +
>>  +class RPMTest(unittest.TestCase):
>>  +    """Test case for RPM class"""
> 
> For instance, I would have expected this comment to say something
> introductory about the tests performed on the fedabipkgdiff.RPM
> class.
> 
>>  +
>>  +    def setUp(self):
> 
> Here, I would have expected a comment about what this
> debuginfo_rpm_info data member is, saying that it must be the same
> type as the parameter type expected by the constructor of the
> fedabipkgdiff.RPM class.  So if that constructor's parameter type
> changes, this test must be updated.  Something like that.
> 
>>  +        self.debuginfo_rpm_info = {
>>  +            'arch': 'i686',
>>  +            'name': 'httpd-debuginfo',
>>  +            'release': '1.fc22',
>>  +            'version': '2.4.18'
>>  +            }
> 
> Likewise.
> 
>>  +        self.rpm_info = {
>>  +            'arch': 'x86_64',
>>  +            'name': 'httpd',
>>  +            'release': '1.fc22',
>>  +            'version': '2.4.18'
>>  +            }
>>  +
>>  +    def test_attribute_access(self):
> 
> Please add a comment saying what this test does.  At least its
> intent.  For instance:
> 
>     This test enforces the exported interface of the fedabipkgdiff.RPM
>     class.  If a new data member is added to that type, removed or
>     changed in that class, please update this test accordingly.
> 
> Maybe repeat here that the data members of the RPM class are the same
> as the data members of the poorly documented return type of the koji
> getRPM request, and so whenever that return type changes, we need to
> update this test.  You know, something like that.  Otherwise, there is
> too much implicit knowledge here, and this makes maintenance painful
> for anyone would would have to take over tomorrow.  And this is
> especially true for a program in python where types are so loosely
> defined and thus where type mis-matches won't be caught until the
> program's runtime.
> 
> etc.
> 
>>  +        rpm = fedabipkgdiff_mod.RPM(self.debuginfo_rpm_info)
>>  +        self.assertEquals(self.debuginfo_rpm_info['arch'], 
> rpm.arch)
>>  +        self.assertEquals(self.debuginfo_rpm_info['name'], 
> rpm.name)
>>  +        self.assertEquals(self.debuginfo_rpm_info['release'], 
> rpm.release)
>>  +        self.assertEquals(self.debuginfo_rpm_info['version'], 
> rpm.version)
> 
> [...]
> 
> I think all test cases should be documented similarly.  And also, all
> the classes that are helper classes for the tests should be
> documented.
> 
> 
>>  --- /dev/null
>>  +++ b/tools/fedabipkgdiff
>>  @@ -0,0 +1,805 @@
>>  +#!/usr/bin/env python
>>  +# -*- coding: utf-8 -*-
>>  +# -*- Mode: Python
>>  +#
>>  +# This file is part of the GNU Application Binary Interface Generic
>>  +# Analysis and Instrumentation Library.  This program is free
>>  +# software; you can redistribute it and/or modify it under the terms
>>  +# of the GNU General Public License as published by the Free Software
>>  +# Foundation; either version 3, or (at your option) any later version.
>>  +#
>>  +# This program is distributed in the hope that it will be useful, but
>>  +# WITHOUT ANY WARRANTY; without even the implied warranty of
>>  +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>  +# General Lesser Public License for more details.
>>  +#
>>  +# You should have received a copy of the GNU Lesser General Public
>>  +# License along with this program; see the file COPYING-LGPLV3.  If
>>  +# not, see <http://www.gnu.org/licenses/>.
> 
> Likewise, I believe this should be GPL.
> 
>>  +#
>>  +# Author: Chenxiong Qi
>>  +
> 
> Please add an introductory comment here, explaining the intend of the
> tool.  Just like what is done for in tools/abidw.cc:
> 
>     /// @file
>     ///
>     /// This program reads an elf file, try to load its debug info (in
>     /// DWARF format) and emit it back in a set of "text sections" in 
> native
>     /// libabigail XML format.
> 
> 
> [...]
> 
> Generally speaking, I would like to see all the function parameter
> *types* and return values to be documented.
> 
> [...]
> 
>>  +def log_call(func):
>>  +    def proxy(*args, **kwargs):
> 
> Please add a comment that says that this is a decorator.  And please
> tell what we expect *args and **kwargs to be.  (the arguments of func,
> as well as the key word arguments, if any).
> 
> [...]
> 
> 
>>  +class RPM(object):
>>  +    """Represeting a RPM"""
>>  +
>>  +    def __init__(self, data):
>>  +        """Initialize a RPM object
>>  +
>>  +        :param dict data: a dict representing a RPM information got from 
> koji
>>  +            API, either listRPMs or getRPM
> 
> What are the keys and values expected in that dict?  Granted the koji
> API is so "lightly" documented, it doesn't really help.  But I 
> think
> we could write an example of the dict keys/values that *we* expect
> here, to ease maintenance.
> 
> Also, you could say something about the fact that the set of data
> members expected by this class is enforced in the unit test case 
> 
> [...]
> 
>>  +    @property
>>  +    def nvra(self):
>>  +        return '%(name)s-%(version)s-%(release)s.%(arch)s' % 
> self.data
>>  +
>>  +    @property
>>  +    def filename(self):
>>  +        return '{0}.rpm'.format(self.nvra)
>>  +
>>  +    @property
>>  +    def is_debuginfo(self):
>>  +        """Check if a RPM is a debuginfo"""
> 
> I would have said:
> 
>     "Check if the name of the current RPM denotes a debug info
>      package"
> 
>>  +        return koji.is_debuginfo(self.data['name'])
>>  +
>>  +    @property
>>  +    def download_url(self):
>>  +        """Get the URL from where to download from 
> koji"""
>>  +        build = session.getBuild(self.build_id)
>>  +        return os.path.join(pathinfo.build(build), 
> pathinfo.rpm(self.data))
>>  +
>>  +    @property
>>  +    def downloaded_file(self):
>>  +        """Get a pridictable downloaded file name with 
> absolute path"""
>>  +        # arch should be removed from the result returned from 
> PathInfo.rpm
>>  +        filename = os.path.basename(pathinfo.rpm(self.data))
>>  +        return os.path.join(get_download_dir(), filename)
>>  +
>>  +    @property
>>  +    def is_downloaded(self):
> 
> Please add a comment here.
> 
>>  +        return os.path.exists(self.downloaded_file)
>>  +
>>  +
>>  +class LocalRPM(RPM):
>>  +    """Representing a local RPM
>>  +
>>  +    Local RPM means the one that could be already downloaded or built from
>>  +    where I can find it
>>  +    """
>>  +
>>  +    def __init__(self, filename):
> 
> Please add a comment here.  What is the type of the "filename"
> parameter etc...
> 
>>  +        self.local_filename = filename
>>  +        self.data = koji.parse_NVRA(os.path.basename(filename))
>>  +
>>  +    @property
>>  +    def downloaded_file(self):
> 
> Comments.
> 
>>  +
>>  +class Brew(object):
>>  +    """Proxy to kojihub XMLRPC with additional extensions 
> to fedabipkgdiff
>>  +
>>  +    kojihub XMLRPC APIs are well-documented in koji's source code. For 
> more
>>  +    details information, please refer to class RootExports within 
> kojihub.py.
>>  +    """
> 
> Please add an html link to the koji source code where the XMLRPC APIs
> are documented.
> 
>>  +
>>  +    def __init__(self, baseurl):
>>  +        """Initialize Brew that is a proxy to 
> koji.ClientSession"""
>>  +        self.session = koji.ClientSession(baseurl)
>>  +
>>  +    @log_call
>>  +    def listRPMs(self, selector=None, **kwargs):
>>  +        """Proxy to kojihub.listRPMs
>>  +
>>  +        :param selector: to adjust if a RPM should be selected
>>  +        :type selector: a callable object
>>  +        :param kwargs: keyword parameters accepted by kojihub.listRPMs
> 
> Here if you could give examples of the keywords (and their arguments)
> that we'd pass, it would be good.  I know the reader can always go dig
> into kojihub.listRPMs, but examples would help.  I think redundancy is
> good here.
> 
>>  +        :type kwargs: dict
>>  +        :return: a list of RPMs, each of them is a dict object
> 
> Again, an example of the dict object that we expect would be good.
> 
>>  +        :rtype: list
>>  +        """
>>  +        if selector:
>>  +            assert hasattr(selector, '__call__'), 'selector 
> should be callable'
>>  +        rpms = self.session.listRPMs(**kwargs)
>>  +        if selector:
>>  +            rpms = [rpm for rpm in rpms if selector(rpm)]
>>  +        return rpms
>>  +
>>  +    @log_call
>>  +    def getRPM(self, *args, **kwargs):
> 
> Comments.
> 
>>  +        rpm = self.session.getRPM(*args, **kwargs)
>>  +        if rpm is None:
>>  +            raise RpmNotFound('Cannot find RPM 
> {0}'.format(args[0]))
>>  +        return rpm
>>  +
>>  +    @log_call
>>  +    def listBuilds(self, topone=None, selector=None, order_by=None,
>>  +                   reverse=None, **kwargs):
>>  +        """Proxy to kojihub.listBuilds to list completed 
> builds
> 
> Please document here (yeah I know it's redundant) the parameters that
> kojihub.listBuilds expects.
> 
>>  +
>>  +        Suport additional two keyword parameters:
>>  +
>>  +        :param bool topone: whether to return the top first one
>>  +        :param selector: a callable object used to select specific subset 
> of
>>  +            builds
>>  +        :type selector: callable object
>>  +        :param str order_by: the attribute name by which to order the 
> builds,
>>  +            for example, name, version, or nvr.
>>  +        :param bool reverse: whether to order builds reversely
>>  +        :param dict kwargs: keyword parameters accepted by 
> kojihub.listBuilds
>>  +        :return: a list of builds, even if just return only one build
>>  +        :rtype: list
>>  +        """
>>  +        if 'state' not in kwargs:
>>  +            kwargs['state'] = 
> koji.BUILD_STATES['COMPLETE']
>>  +
>>  +        if selector is not None and not hasattr(selector, 
> '__call__'):
>>  +            raise TypeError(
>>  +                '{0} is not a callable 
> object.'.format(str(selector)))
>>  +
>>  +        if order_by is not None and not isinstance(order_by, basestring):
>>  +            raise TypeError('order_by {0} is 
> invalid.'.format(order_by))
>>  +
>>  +        builds = self.session.listBuilds(**kwargs)
>>  +        if selector is not None:
>>  +            builds = [build for build in builds if selector(build)]
>>  +        if order_by is not None:
>>  +            # FIXME: is it possible to sort builds by using opts parameter 
> of
>>  +            # listBuilds
>>  +            builds = sorted(builds,
>>  +                            key=lambda item: item[order_by],
>>  +                            reverse=reverse)
>>  +        if topone:
>>  +            builds = builds[0:1]
>>  +
>>  +        return builds
>>  +
>>  +    @log_call
>>  +    def getPackage(self, name):
>>  +        """Proxy to kojihub.getPackage
>>  +
>>  +        :param str name: package name
> 
> Please give example of the package name that we expect, so that a user
> of this function has an idea of the correct form expected.
> 
>>  +        :return: a dict object representing a package
>>  +        :rtype: dict
>>  +        """
>>  +        package = self.session.getPackage(name)
>>  +        if package is None:
>>  +            package = self.session.getPackage(name.rsplit('-', 
> 1)[0])
>>  +            if package is None:
>>  +                raise KojiPackageNotFound(
>>  +                    'Cannot find package {0}.'.format(name))
>>  +        return package
>>  +
>>  +    @log_call
>>  +    def getBuild(self, *args, **kwargs):
>>  +        """Proxy to kojihub.getBuild"""
> 
> Please describe the (keyword) arguments expected.
> 
>>  +        return self.session.getBuild(*args, **kwargs)
>>  +
> 
>>  +    @log_call
>>  +    def get_rpm_build_id(self, name, version, release, arch=None):
>>  +        """Get build ID that contains a rpm with specific 
> nvra
>>  +
>>  +        If arch is omitted, a rpm can be identified easily by its N-V-R-A.
>>  +
>>  +        If arch is omitted, name is used to get associated package, and 
> then
>>  +        to get the build.
>>  +
>>  +        :param str name: name of a rpm
>>  +        :param str version: version of a rpm
>>  +        :param str release: release of a rpm
> 
> I think that here, example of what you expect as release (at least)
> would be interesting.
> 
> [...]
> 
>>  +
>>  +    @log_call
>>  +    def get_package_latest_build(self, package_name, distro):
>>  +        """Get latest build from a package
>>  +
>>  +        :param str package_name: from which package to get the latest 
> build
>>  +        :param str distro: which distro the latest build belongs to
> 
> The form of what is expected as distro would be important too.  That
> would help to maintain the test suite.
> 
> [...]
> 
> +@log_call
> +def get_session():
> 
> Comments.
> 
> +    return Brew(global_config.koji_server)
> 
> [...]
> 
> +def get_download_dir():
> +    """Return the directory holding all downloaded 
> rpms"""
> +    download_dir = os.path.join(HOME_DIR, 'downloads')
> 
> I would use the standard $XDG_CACHE_HOME place, to store the
> downloaded packages:
> https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html.
> That is, ~/.local/fedabipkgdiff.
> 
> I could also use a fedabipkgdiff --clean-cache option to clean the
> cache cache, but we can add that this later, after the patch is in.
> 
> +    if not os.path.exists(download_dir):
> +        os.makedirs(download_dir)
> +    return download_dir
> +
> 
>>  +@log_call
>>  +def abipkgdiff(pkg_info1, pkg_info2):
>>  +    """Run abipkgdiff against found two RPM 
> packages"""
> 
> Please document the expected types of the parameters here.
> 
> +    print 'ABI check on {0} and 
> {1}'.format(pkg_info1.package.filename,
> +                                            pkg_info2.package.filename)
> 
> Please, rather print:
> 
> 'Comparing the ABI of binaries between {0} and {1}:'
> 
> [...]
> 
>>  +def magic_construct(rpms):
>>  +    """Construct RPMs into a magic structure
>>  +
>>  +    Convert list of
>>  +
>>  +    foo-1.0-1.fc22.i686
>>  +    foo-debuginfo-1.0-1.fc22.i686
>>  +    foo-devel-1.0-1.fc22.i686
>>  +
>>  +    to list of
>>  +
>>  +    (foo-1.0-1.fc22.i686, foo-debuginfo-1.0-1.fc22.i686)
>>  +    (foo-devel-1.0-1.fc22.i686, foo-debuginfo-1.0-1.fc22.i686)
>>  +    """
>>  +    debuginfo = None
>>  +    packages = []
>>  +    for rpm in rpms:
>>  +        if rpm.is_debuginfo:
>>  +            debuginfo = rpm
>>  +        else:
>>  +            packages.append(rpm)
>>  +    return [PkgInfo(package, debuginfo) for package in packages]
>>  +
>>  +
>>  +@log_call
>>  +def run_abipkgdiff(pkg1_infos, pkg2_infos):
>>  +    """Run abipkgdiff
>>  +
>>  +    If one of the executions finds ABI differences, the return code is the
>>  +    return code from abipkgdiff.
>>  +
>>  +    :param dict pkg1_infos: a dict mapping from arch to list of rpms, that 
> is
>>  +        returned from method make_rpms_usable_for_abipkgdiff
> 
> Rather than refer to what make_rpms_usable_for_abipkgdiff returns,
> please redundantly describe the dict here, giving a real example, so
> that a user willing to use this function can see what she should feed
> it with.
> 
> [...]
> 
>>  +@log_call
>>  +def diff_local_rpm_with_latest_rpm_from_koji():
>>  +    """Diff against local rpm and remove latest rpm
>>  +
>>  +    This operation handles a local rpm and debuginfo rpm and remote ones
>>  +    located in remote Koji server, that has specific distro specificed by
>>  +    argument --from.
>>  +
>>  +    1/ Suppose the packager has just locally built a package named
>>  +    foo-3.0.fc24.rpm. To compare the ABI of this locally build package 
> with the
>>  +    latest stable package from Fedora 23, one would do:
>>  +
>>  +    fedabipkgdiff --from f23 ./foo-3.0.fc24.rpm
>>  +    """
> 
> Great comment, thanks.
> 
> [...]
> 
> 
>>  +@log_call
>>  +def make_rpms_usable_for_abipkgdiff(rpms):
>>  +    """
>>  +    Construct result that contains mappings from arch to download url and
>>  +    downloaded rpm filename of rpm and debuginfo rpm
>>  +
>>  +    :return: a mapping from an arch to a list of rpms
> 
> Please give an example of this dict.
> 
> [...]
> 
> 
>>  +@log_call
>>  +def diff_rpms_with_nvra(name, version, release, arch=None,
>>  +                        all_subpackages=None):
> 
> Comments.
> 
>>  +    build_id = session.get_rpm_build_id(name, version, release, arch)
>>  +    rpms = session.select_rpms_from_a_build(build_id, name, arches=arch,
>>  +                                            
> select_subpackages=all_subpackages)
>>  +    return make_rpms_usable_for_abipkgdiff(rpms)
>>  +
> 
> Also, I wouldn't call this function "diff_rpms_with_nvra", because 
> the
> function doesn't perform any diffing, does it?  Rather, my
> understanding is that it constructs the two sets of package
> descriptors that would be passed to abipkgdiff.
> 
> 
>>  +@log_call
>>  +def diff_two_nvras_from_koji():
>>  +    """Diff two nvras from koji
>>  +
>>  +    The arch probably omits, that means febabipkgdiff will diff all 
> arches. If
> 
> I would say "the arch is probably omitted ..."
> 
> [...]
> 
> +def main():
> 
> [...]
> 
> 
> +    else:
> +        print >>sys.stderr, 'Unknown arguments. Please refer to 
> -h.'
> +        returncode = 1
> 
> Please, refer to --help, rather -h just like what is done in the other
> libabigail tools.
> 
> +
> +    return returncode
> 
> All in all, I am wondering if we shouldn't add some progress
> indication to the user, just like what "dnf" does.  You know, saying
> things like "we are downloading this package etc, and now we are
> comparing these packages, etc."  Otherwise, the time taken can seem
> quite long.  But we can add this later, after the patch has been
> committed, I guess.

Maybe, a simple way is to use --verbose option to print such information next step.

> 
> Also, we should arrange for the downloads to happen in parallel,
> somehow.  Again, this can happen after the patch is in.

Haha, I know what you mean. Yeah, it's worth to do this.

> 
> Now, the other big thing missing from this patch is a manual.
> 
> That is really important.  Every single libabigail tool has a
> documentation in docs/manuals.  Documentation is in the restructured
> text format, and we use python-sphinx to generate html, man and info
> variants for it.  The manual should also say where the packages are
> downloaded.
> 
> If you don't feel like adding a manual for this tool, please tell me,
> I'll help.
> 
> I love this patch very much!  Thank you for your hard work on this.
> This is simply awesome.  I cannot wait for it to get in :-)
> 
> Cheers,
> 
> -- 
>         Dodji
> 

This time, the patch is updated

- more documentation inside fedabipkgdiff and runtestfedabipkgdiff.py.in
- using curl instead of wget to download packages from Koji
- a new manual (with Dodji's help, man page can be generated from this manual by sphnix)
- rebased, on latest commit fa5a5acbbcb1e

Hello, Mr. Dodji, please review. Thanks.

Regards,
Chenxiong Qi

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-new-tool-of-fedabipkgdiff.patch --]
[-- Type: text/x-patch, Size: 87961 bytes --]

From ae5c2c6704c764946e7dca748a20eda378e15f56 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Tue, 9 Feb 2016 18:05:33 +0800
Subject: [PATCH] new tool of fedabipkgdiff

fedabipkgdiff is a convenient way for Fedora packagers to inspect ABI
compatibility issues quickly.

Currently with the first version of fedabipkgdiff, you can invoke it in
following ways.

fedabipkgdiff --from fc23 foo-0.1-1.fc23.x86_64.rpm
fedabipkgdiff --from fc23 --to fc24 foo
fedabipkgdiff foo-0.1-1.fc23 foo-0.1-1.fc24
fedabipkgdiff foo-0.1-1.fc23.i686 foo-0.1-1.fc24.i686
fedabipkgdiff --all-subpackages foo-0.1-1.fc23 foo-0.1-1.fc24

	* autoconf-archive/ax_compare_version.m4: New file copied from the
	autoconf-archive project.
	* autoconf-archive/ax_prog_python_version.m4: Likewise.
	* autoconf-archive/ax_python_module.m4: Likewise.
	* Makefile.am: Add the new files above to the source distribution.
	* configure.ac: Include the new m4 macros from the autoconf
	archive. Add a new --enable-fedabipkgdiff option. Update the report
	at the end of the configure process to show the status of the
	fedabipkgdiff feature. Add check for prerequisite python modules
	itertools, shutil, unittest and mock.  These are necessary for the
	unit test of fedabipkgdiff. Generate tests/runtestfedabipkgdiff.py
	into the build directory, from the tests/runtestfedabipkgdiff.py.in
	input file.
	* tools/Makefile.am: Include the fedabipkgdiff to the source
	distribution and install it if the "fedabipkgdiff" feature is
	enabled.
	* tests/Makefile.am: Rename runtestfedabipkgdiff.sh into
	runtestfedabipkgdiff.py.  Add the new runtestfedabipkgdiff.py.in
	autoconf template file in here.
	* tests/runtestfedabipkgdiff.py.in: New unit test file.
	* tools/fedabipkgdiff: New tool fedabipkgdiff.
	* doc/manuals/fedabipkgdiff.rst: New manual.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>
---
 .gitignore                                 |    6 +
 Makefile.am                                |    3 +
 autoconf-archive/ax_compare_version.m4     |  177 +++++
 autoconf-archive/ax_prog_python_version.m4 |   66 ++
 autoconf-archive/ax_python_module.m4       |   56 ++
 configure.ac                               |   86 ++-
 doc/manuals/Makefile.am                    |    3 +-
 doc/manuals/conf.py                        |    1 +
 doc/manuals/fedabipkgdiff.rst              |  108 +++
 doc/manuals/libabigail-tools.rst           |    1 +
 tests/Makefile.am                          |    6 +-
 tests/runtestfedabipkgdiff.py.in           |  634 +++++++++++++++++
 tools/Makefile.am                          |    6 +
 tools/fedabipkgdiff                        | 1050 ++++++++++++++++++++++++++++
 14 files changed, 2200 insertions(+), 3 deletions(-)
 create mode 100644 autoconf-archive/ax_compare_version.m4
 create mode 100644 autoconf-archive/ax_prog_python_version.m4
 create mode 100644 autoconf-archive/ax_python_module.m4
 create mode 100644 doc/manuals/fedabipkgdiff.rst
 create mode 100755 tests/runtestfedabipkgdiff.py.in
 create mode 100755 tools/fedabipkgdiff

diff --git a/.gitignore b/.gitignore
index bb7c42a..a60cadb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,6 +6,7 @@ Makefile.in
 *.lo
 *.o
 *~
+*.swp
 
 /aclocal.m4
 /autom4te.cache/
@@ -17,3 +18,8 @@ Makefile.in
 
 /include/abg-version.h
 /*.pc
+
+.tags
+build/
+TAGS
+fedabipkgdiffc
\ No newline at end of file
diff --git a/Makefile.am b/Makefile.am
index cdee0db..29bed61 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -12,6 +12,9 @@ pkgconfig_DATA = libabigail.pc
 #dist_bashcompletion_DATA =
 
 EXTRA_DIST = 			\
+autoconf-archive/ax_python_module.m4 \
+autoconf-archive/ax_prog_python_version.m4 \
+autoconf-archive/ax_compare_version.m4 \
 NEWS README COPYING ChangeLog	\
 COPYING-LGPLV2 COPYING-LGPLV3	\
 COPYING-GPLV3 gen-changelog.py	\
diff --git a/autoconf-archive/ax_compare_version.m4 b/autoconf-archive/ax_compare_version.m4
new file mode 100644
index 0000000..74dc0fd
--- /dev/null
+++ b/autoconf-archive/ax_compare_version.m4
@@ -0,0 +1,177 @@
+# ===========================================================================
+#    http://www.gnu.org/software/autoconf-archive/ax_compare_version.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_COMPARE_VERSION(VERSION_A, OP, VERSION_B, [ACTION-IF-TRUE], [ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   This macro compares two version strings. Due to the various number of
+#   minor-version numbers that can exist, and the fact that string
+#   comparisons are not compatible with numeric comparisons, this is not
+#   necessarily trivial to do in a autoconf script. This macro makes doing
+#   these comparisons easy.
+#
+#   The six basic comparisons are available, as well as checking equality
+#   limited to a certain number of minor-version levels.
+#
+#   The operator OP determines what type of comparison to do, and can be one
+#   of:
+#
+#    eq  - equal (test A == B)
+#    ne  - not equal (test A != B)
+#    le  - less than or equal (test A <= B)
+#    ge  - greater than or equal (test A >= B)
+#    lt  - less than (test A < B)
+#    gt  - greater than (test A > B)
+#
+#   Additionally, the eq and ne operator can have a number after it to limit
+#   the test to that number of minor versions.
+#
+#    eq0 - equal up to the length of the shorter version
+#    ne0 - not equal up to the length of the shorter version
+#    eqN - equal up to N sub-version levels
+#    neN - not equal up to N sub-version levels
+#
+#   When the condition is true, shell commands ACTION-IF-TRUE are run,
+#   otherwise shell commands ACTION-IF-FALSE are run. The environment
+#   variable 'ax_compare_version' is always set to either 'true' or 'false'
+#   as well.
+#
+#   Examples:
+#
+#     AX_COMPARE_VERSION([3.15.7],[lt],[3.15.8])
+#     AX_COMPARE_VERSION([3.15],[lt],[3.15.8])
+#
+#   would both be true.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq],[3.15.8])
+#     AX_COMPARE_VERSION([3.15],[gt],[3.15.8])
+#
+#   would both be false.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq2],[3.15.8])
+#
+#   would be true because it is only comparing two minor versions.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq0],[3.15])
+#
+#   would be true because it is only comparing the lesser number of minor
+#   versions of the two values.
+#
+#   Note: The characters that separate the version numbers do not matter. An
+#   empty string is the same as version 0. OP is evaluated by autoconf, not
+#   configure, so must be a string, not a variable.
+#
+#   The author would like to acknowledge Guido Draheim whose advice about
+#   the m4_case and m4_ifvaln functions make this macro only include the
+#   portions necessary to perform the specific comparison specified by the
+#   OP argument in the final configure script.
+#
+# LICENSE
+#
+#   Copyright (c) 2008 Tim Toolan <toolan@ele.uri.edu>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 11
+
+dnl #########################################################################
+AC_DEFUN([AX_COMPARE_VERSION], [
+  AC_REQUIRE([AC_PROG_AWK])
+
+  # Used to indicate true or false condition
+  ax_compare_version=false
+
+  # Convert the two version strings to be compared into a format that
+  # allows a simple string comparison.  The end result is that a version
+  # string of the form 1.12.5-r617 will be converted to the form
+  # 0001001200050617.  In other words, each number is zero padded to four
+  # digits, and non digits are removed.
+  AS_VAR_PUSHDEF([A],[ax_compare_version_A])
+  A=`echo "$1" | sed -e 's/\([[0-9]]*\)/Z\1Z/g' \
+                     -e 's/Z\([[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/[[^0-9]]//g'`
+
+  AS_VAR_PUSHDEF([B],[ax_compare_version_B])
+  B=`echo "$3" | sed -e 's/\([[0-9]]*\)/Z\1Z/g' \
+                     -e 's/Z\([[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/[[^0-9]]//g'`
+
+  dnl # In the case of le, ge, lt, and gt, the strings are sorted as necessary
+  dnl # then the first line is used to determine if the condition is true.
+  dnl # The sed right after the echo is to remove any indented white space.
+  m4_case(m4_tolower($2),
+  [lt],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort -r | sed "s/x${A}/false/;s/x${B}/true/;1q"`
+  ],
+  [gt],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort | sed "s/x${A}/false/;s/x${B}/true/;1q"`
+  ],
+  [le],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort | sed "s/x${A}/true/;s/x${B}/false/;1q"`
+  ],
+  [ge],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort -r | sed "s/x${A}/true/;s/x${B}/false/;1q"`
+  ],[
+    dnl Split the operator from the subversion count if present.
+    m4_bmatch(m4_substr($2,2),
+    [0],[
+      # A count of zero means use the length of the shorter version.
+      # Determine the number of characters in A and B.
+      ax_compare_version_len_A=`echo "$A" | $AWK '{print(length)}'`
+      ax_compare_version_len_B=`echo "$B" | $AWK '{print(length)}'`
+
+      # Set A to no more than B's length and B to no more than A's length.
+      A=`echo "$A" | sed "s/\(.\{$ax_compare_version_len_B\}\).*/\1/"`
+      B=`echo "$B" | sed "s/\(.\{$ax_compare_version_len_A\}\).*/\1/"`
+    ],
+    [[0-9]+],[
+      # A count greater than zero means use only that many subversions
+      A=`echo "$A" | sed "s/\(\([[0-9]]\{4\}\)\{m4_substr($2,2)\}\).*/\1/"`
+      B=`echo "$B" | sed "s/\(\([[0-9]]\{4\}\)\{m4_substr($2,2)\}\).*/\1/"`
+    ],
+    [.+],[
+      AC_WARNING(
+        [illegal OP numeric parameter: $2])
+    ],[])
+
+    # Pad zeros at end of numbers to make same length.
+    ax_compare_version_tmp_A="$A`echo $B | sed 's/./0/g'`"
+    B="$B`echo $A | sed 's/./0/g'`"
+    A="$ax_compare_version_tmp_A"
+
+    # Check for equality or inequality as necessary.
+    m4_case(m4_tolower(m4_substr($2,0,2)),
+    [eq],[
+      test "x$A" = "x$B" && ax_compare_version=true
+    ],
+    [ne],[
+      test "x$A" != "x$B" && ax_compare_version=true
+    ],[
+      AC_WARNING([illegal OP parameter: $2])
+    ])
+  ])
+
+  AS_VAR_POPDEF([A])dnl
+  AS_VAR_POPDEF([B])dnl
+
+  dnl # Execute ACTION-IF-TRUE / ACTION-IF-FALSE.
+  if test "$ax_compare_version" = "true" ; then
+    m4_ifvaln([$4],[$4],[:])dnl
+    m4_ifvaln([$5],[else $5])dnl
+  fi
+]) dnl AX_COMPARE_VERSION
diff --git a/autoconf-archive/ax_prog_python_version.m4 b/autoconf-archive/ax_prog_python_version.m4
new file mode 100644
index 0000000..628a3e4
--- /dev/null
+++ b/autoconf-archive/ax_prog_python_version.m4
@@ -0,0 +1,66 @@
+# ===========================================================================
+#  http://www.gnu.org/software/autoconf-archive/ax_prog_python_version.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_PROG_PYTHON_VERSION([VERSION],[ACTION-IF-TRUE],[ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   Makes sure that python supports the version indicated. If true the shell
+#   commands in ACTION-IF-TRUE are executed. If not the shell commands in
+#   ACTION-IF-FALSE are run. Note if $PYTHON is not set (for example by
+#   running AC_CHECK_PROG or AC_PATH_PROG) the macro will fail.
+#
+#   Example:
+#
+#     AC_PATH_PROG([PYTHON],[python])
+#     AX_PROG_PYTHON_VERSION([2.4.4],[ ... ],[ ... ])
+#
+#   This will check to make sure that the python you have supports at least
+#   version 2.4.4.
+#
+#   NOTE: This macro uses the $PYTHON variable to perform the check.
+#   AX_WITH_PYTHON can be used to set that variable prior to running this
+#   macro. The $PYTHON_VERSION variable will be valorized with the detected
+#   version.
+#
+# LICENSE
+#
+#   Copyright (c) 2009 Francesco Salvestrini <salvestrini@users.sourceforge.net>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 11
+
+AC_DEFUN([AX_PROG_PYTHON_VERSION],[
+    AC_REQUIRE([AC_PROG_SED])
+    AC_REQUIRE([AC_PROG_GREP])
+
+    AS_IF([test -n "$PYTHON"],[
+        ax_python_version="$1"
+
+        AC_MSG_CHECKING([for python version])
+        changequote(<<,>>)
+        python_version=`$PYTHON -V 2>&1 | $GREP "^Python " | $SED -e 's/^.* \([0-9]*\.[0-9]*\.[0-9]*\)/\1/'`
+        changequote([,])
+        AC_MSG_RESULT($python_version)
+
+	AC_SUBST([PYTHON_VERSION],[$python_version])
+
+        AX_COMPARE_VERSION([$ax_python_version],[le],[$python_version],[
+	    :
+            $2
+        ],[
+	    :
+            $3
+        ])
+    ],[
+        AC_MSG_WARN([could not find the python interpreter])
+        $3
+    ])
+])
diff --git a/autoconf-archive/ax_python_module.m4 b/autoconf-archive/ax_python_module.m4
new file mode 100644
index 0000000..f182c48
--- /dev/null
+++ b/autoconf-archive/ax_python_module.m4
@@ -0,0 +1,56 @@
+# ===========================================================================
+#     http://www.gnu.org/software/autoconf-archive/ax_python_module.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_PYTHON_MODULE(modname[, fatal, python])
+#
+# DESCRIPTION
+#
+#   Checks for Python module.
+#
+#   If fatal is non-empty then absence of a module will trigger an error.
+#   The third parameter can either be "python" for Python 2 or "python3" for
+#   Python 3; defaults to Python 3.
+#
+# LICENSE
+#
+#   Copyright (c) 2008 Andrew Collier
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 8
+
+AU_ALIAS([AC_PYTHON_MODULE], [AX_PYTHON_MODULE])
+AC_DEFUN([AX_PYTHON_MODULE],[
+    if test -z $PYTHON;
+    then
+        if test -z "$3";
+        then
+            PYTHON="python3"
+        else
+            PYTHON="$3"
+        fi
+    fi
+    PYTHON_NAME=`basename $PYTHON`
+    AC_MSG_CHECKING($PYTHON_NAME module: $1)
+    $PYTHON -c "import $1" 2>/dev/null
+    if test $? -eq 0;
+    then
+        AC_MSG_RESULT(yes)
+        eval AS_TR_CPP(HAVE_PYMOD_$1)=yes
+    else
+        AC_MSG_RESULT(no)
+        eval AS_TR_CPP(HAVE_PYMOD_$1)=no
+        #
+        if test -n "$2"
+        then
+            AC_MSG_ERROR(failed to find required module $1)
+            exit 1
+        fi
+    fi
+])
diff --git a/configure.ac b/configure.ac
index fa87105..9d61051 100644
--- a/configure.ac
+++ b/configure.ac
@@ -14,6 +14,18 @@ AC_CONFIG_HEADER([config.h])
 AC_CONFIG_SRCDIR([README])
 AC_CONFIG_MACRO_DIR([m4])
 
+dnl Include some autoconf macros to check for python modules.
+dnl
+dnl These macros are coming from the autoconf archive at
+dnl http://www.gnu.org/software/autoconf-archive
+
+dnl This one is for the AX_PYTHON_MODULE() macro.
+m4_include([autoconf-archive/ax_python_module.m4])
+
+dnl These two below are for the AX_PROG_PYTHON_VERSION() module.
+m4_include([autoconf-archive/ax_compare_version.m4])
+m4_include([autoconf-archive/ax_prog_python_version.m4])
+
 AM_INIT_AUTOMAKE([1.11.1 foreign subdir-objects tar-ustar parallel-tests])
 AM_MAINTAINER_MODE([enable])
 
@@ -76,6 +88,12 @@ AC_ARG_ENABLE([bash-completion],
 	      ENABLE_BASH_COMPLETION=$enableval,
 	      ENABLE_BASH_COMPLETION=auto)
 
+AC_ARG_ENABLE([fedabipkgdiff],
+	      AS_HELP_STRING([--enable-fedabipkgdiff=yes|no|auto],
+			     [enable the fedabipkgdiff tool]),
+	      ENABLE_FEDABIPKGDIFF=$enableval,
+	      ENABLE_FEDABIPKGDIFF=auto)
+
 dnl *************************************************
 dnl check for dependencies
 dnl *************************************************
@@ -219,6 +237,68 @@ fi
 
 AM_CONDITIONAL(ENABLE_BASH_COMPLETION, test x$ENABLE_BASH_COMPLETION = xyes)
 
+dnl if --enable-fedabipkgdiff has the 'auto' value, then check for the required
+dnl python modules.  If they are present, then enable the fedabipkgdiff program.
+dnl If they are not then disable the program.
+dnl
+dnl If --enable-fedabipkgdiff has the 'yes' value, then check for the required
+dnl python modules and whatever dependency fedabipkgdiff needs.  If they are
+dnl not present then the configure script will error out.
+
+if test x$ENABLE_FEDABIPKGDIFF = xauto -o x$ENABLE_FEDABIPKGDIFF = xyes; then
+   CHECK_DEPS_FOR_FEDABIPKGDIFF=yes
+else
+   CHECK_DEPS_FOR_FEDABIPKGDIFF=no
+fi
+
+if test x$CHECK_DEPS_FOR_FEDABIPKGDIFF = xyes; then
+  if test x$ENABLE_FEDABIPKGDIFF = xyes; then
+     FATAL=yes
+  fi
+
+  AC_PATH_PROG(WGET, wget, no)
+
+  if test x$WGET = x$no; then
+    AC_MSG_ERROR(could not find the wget program)
+  fi
+
+  # 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"
+
+  AC_PATH_PROG(PYTHON, python, no)
+  AX_PROG_PYTHON_VERSION($MINIMAL_PYTHON_VERSION,
+			 [MINIMAL_PYTHON_VERSION_FOUND=yes],
+			 [MINIMAL_PYTHON_VERSION_FOUND=no])
+
+  if test x$MINIMAL_PYTHON_VERSION_FOUND = xno; then
+    AC_MSG_ERROR([could not find a python program of version at least $MINIMAL_PYTHON_VERSION])
+  fi
+
+  AX_PYTHON_MODULE(argparse, $FATAL, python2)
+  AX_PYTHON_MODULE(glob, $FATAL, python2)
+  AX_PYTHON_MODULE(logging, $FATAL, python2)
+  AX_PYTHON_MODULE(os, $FATAL, python2)
+  AX_PYTHON_MODULE(re, $FATAL, python2)
+  AX_PYTHON_MODULE(shlex, $FATAL, python2)
+  AX_PYTHON_MODULE(subprocess, $FATAL, python2)
+  AX_PYTHON_MODULE(sys, $FATAL, python2)
+  AX_PYTHON_MODULE(itertools, $FATAL, python2)
+  AX_PYTHON_MODULE(urlparse, $FATAL, python2)
+  AX_PYTHON_MODULE(itertools, $FATAL, python2)
+  AX_PYTHON_MODULE(shutil, $FATAL, python2)
+  AX_PYTHON_MODULE(unittest, $FATAL, python2)
+  AX_PYTHON_MODULE(koji, $FATAL, python2)
+  AX_PYTHON_MODULE(mock, $FATAL, python2)
+  ENABLE_FEDABIPKGDIFF=yes
+
+  if test x$ENABLE_FEDABIPKGDIFF != xyes; then
+    ENABLE_FEDABIPKGDIFF=no
+  fi
+fi
+
+AM_CONDITIONAL(ENABLE_FEDABIPKGDIFF, test x$ENABLE_FEDABIPKGDIFF = xyes)
+
 dnl Check for dependency: libzip
 LIBZIP_VERSION=0.10.1
 
@@ -361,7 +441,10 @@ libabigail.pc
     bash-completion/Makefile])
 
 dnl Some test scripts are generated by autofoo.
-AC_CONFIG_FILES([tests/runtestcanonicalizetypes.sh], [chmod +x tests/runtestcanonicalizetypes.sh])
+AC_CONFIG_FILES([tests/runtestcanonicalizetypes.sh],
+		[chmod +x tests/runtestcanonicalizetypes.sh])
+AC_CONFIG_FILES([tests/runtestfedabipkgdiff.py],
+		[chmod +x tests/runtestfedabipkgdiff.py])
 
 AC_OUTPUT
 
@@ -384,6 +467,7 @@ AC_MSG_NOTICE([
     Enable deb support in abipkgdiff               : ${ENABLE_DEB}
     Enable GNU tar archive support in abipkgdiff   : ${ENABLE_TAR}
     Enable bash completion	                   : ${ENABLE_BASH_COMPLETION}
+    Enable fedabipkgdiff			   : ${ENABLE_FEDABIPKGDIFF}
     Generate html apidoc	                   : ${ENABLE_APIDOC}
     Generate html manual	                   : ${ENABLE_MANUAL}
 ])
diff --git a/doc/manuals/Makefile.am b/doc/manuals/Makefile.am
index f23d1ab..5128bc0 100644
--- a/doc/manuals/Makefile.am
+++ b/doc/manuals/Makefile.am
@@ -10,7 +10,8 @@ conf.py \
 index.rst \
 libabigail-concepts.rst \
 libabigail-overview.rst \
-libabigail-tools.rst
+libabigail-tools.rst \
+fedabipkgdiff
 
 # You can set these variables from the command line.
 SPHINXOPTS    =
diff --git a/doc/manuals/conf.py b/doc/manuals/conf.py
index 2a7019f..a0967a2 100644
--- a/doc/manuals/conf.py
+++ b/doc/manuals/conf.py
@@ -219,6 +219,7 @@ man_pages = [
     ('abidw', 'abidw', u'serialize the ABI of an ELF file', [u'Dodji Seketeli'], 1),
     ('abilint', 'abilint', u'validate an abigail ABI representation', [u'Dodji Seketeli'], 1),
     ('abicompat', 'abicompat', u'check ABI compatibility', [u'Dodji Seketeli'], 1),
+    ('fedabipkgdiff', 'fedabipkgdiff', u'compare ABIs of Fedora packages', [u'Chenxiong Qi'], 1),
 ]
 
 # If true, show URL addresses after external links.
diff --git a/doc/manuals/fedabipkgdiff.rst b/doc/manuals/fedabipkgdiff.rst
new file mode 100644
index 0000000..69e2368
--- /dev/null
+++ b/doc/manuals/fedabipkgdiff.rst
@@ -0,0 +1,108 @@
+.. _fedabipkgdiff_label:
+
+==============
+fedabipkgdiff
+==============
+
+``fedabipkgdiff`` compares ABIs of Fedora packages. It's a convenient way for
+Fedora packagers to run ``abipkgdiff`` against two RPM packages with their debug
+information packages. With well specified options, ``fedabipkgdiff`` is able to
+help packagers to find corresponding RPM packages from Koji, and then, if found,
+download them, and run ``abipkgdiff`` eventually to report possible ABI changes.
+
+
+.. _fedabipkgdiff_invocation_label:
+
+Invocation
+==========
+
+::
+
+   fedabipkgdiff [option] <NVR> ...
+
+
+.. _fedabipkgdiff_options_label:
+
+Options
+=======
+
+  * ``--help | -h``
+
+    Display a short help about the command and exit.
+
+  * ``--dry-run``
+
+    Don't actually run abipkgdiff. The commands that should be run will be sent
+    to stdout.
+
+  * ``--debug``
+
+    Run in debug mode, to show very detail debug output of each method
+    invocation and input paramters and returned result.
+
+  * ``--traceback``
+
+    Show traceback when there is an exception thrown. This could be useful for
+    developers to know what and where the error is.
+
+  * ``--server`` <URL>
+
+    URL of koji XMLRPC service. Default is http://koji.fedoraproject.org/kojihub
+
+  * ``--topdir`` <URL>
+
+    URL for RPM files access. Default is https://kojipkgs.fedoraproject.org
+
+  * ``--from`` <distro>
+
+    Baseline Fedora distrobution from where to find proper build that is used
+    for comparison. distro could be any valid value of RPM macro ``%{?dist}``
+    for Fedora, for example, ``fc4``, ``fc23``, ``fc25``.
+
+  * ``--to`` <distro>
+
+    The Fedora distrobution from where to find a proper build that is compared
+    with the baseline specified by option ``--from``.
+
+
+.. _fedabipkgdiff_return_value_label:
+
+Return value
+============
+
+The exit code of the ``abipkgdiff`` depends on the :ref:`return code
+<abipkgdiff_return_value_label>` from underlying abipkgdiff. The exit code is
+either 0 if all ``abipkgdiff`` invocations succeed and no ABI changes between
+packages, or non-zero that is returned by the last ``abipkgdiff`` invocation.
+
+
+.. _fedabipkgdiff_usage_example_label:
+
+
+Use cases
+=========
+
+Several cases ``fedabipkgdiff`` is supporting currectly are shown here.
+
+  1. Compare ABIs of local package with latest stable package from Koji.
+     Suppose, you have built packages for httpd, and you would like to compare
+     the ABI of this locally built package with the latest stable package
+     built for Fedora 23. ::
+
+       $ fedabipkgdiff --from fc23 ./httpd-2.4.18-2.fc24.x86_64.rpm
+
+  2. Compare ABIs of package httpd between Fedora 23 and 24. ::
+
+       $ fedabipkgdiff --from fc23 --to fc24 httpd
+
+  3. Compare ABIs of package httpd designated by name, version and release,
+     even with a specific arch, for example x86_64. ::
+
+       $ fedabipkgdiff httpd-2.8.14.fc23 httpd-2.8.14.fc24
+       $ fedabipkgdiff httpd-2.8.14.fc23.x86_64 httpd-2.8.14.fc24.x86_64
+
+  4. Within case 3, in addition, packager is also able to compare all packages
+     rather than specified package only. Obviously, both noarch and src packages
+     are excluded. ::
+
+       $ fedabipkgdiff --all-subpackages httpd-2.8.14.fc23 httpd-2.8.14.fc24
diff --git a/doc/manuals/libabigail-tools.rst b/doc/manuals/libabigail-tools.rst
index 2f7f4c1..d3d2492 100644
--- a/doc/manuals/libabigail-tools.rst
+++ b/doc/manuals/libabigail-tools.rst
@@ -20,3 +20,4 @@ Tools manuals
    abicompat
    abidw
    abilint
+   fedabipkgdiff
diff --git a/tests/Makefile.am b/tests/Makefile.am
index caf49e6..953dfef 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -31,9 +31,10 @@ runtestlookupsyms		\
 runtestaltdwarf			\
 runtestcorediff			\
 runtestabidiffexit		\
+runtestfedabipkgdiff.py		\
 $(CXX11_TESTS)
 
-EXTRA_DIST = runtestcanonicalizetypes.sh.in
+EXTRA_DIST = runtestcanonicalizetypes.sh.in runtestfedabipkgdiff.py.in
 CLEANFILES = \
  runtestcanonicalizetypes.output.txt \
  runtestcanonicalizetypes.output.final.txt
@@ -114,6 +115,9 @@ printdifftree_LDADD = $(top_builddir)/src/libabigail.la
 runtestcanonicalizetypes_sh_SOURCES =
 runtestcanonicalizetypes.sh$(EXEEXT):
 
+runtestfedabipkgdiff_py_SOURCES =
+runtestfedabipkgdiff.py$(EXEEXT):
+
 AM_CPPFLAGS=-I${abs_top_srcdir}/include \
 -I${abs_top_builddir}/include -I${abs_top_srcdir}/tools -fPIC
 
diff --git a/tests/runtestfedabipkgdiff.py.in b/tests/runtestfedabipkgdiff.py.in
new file mode 100755
index 0000000..84a4320
--- /dev/null
+++ b/tests/runtestfedabipkgdiff.py.in
@@ -0,0 +1,634 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+# -*- Mode: Python
+#
+# This file is part of the GNU Application Binary Interface Generic
+# Analysis and Instrumentation Library (libabigail).  This library is
+# free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 3, or (at your option) any
+# later version.
+#
+# This library is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public
+# License along with this program; see the file COPYING-GPLV3.  If
+# not, see <http:#www.gnu.org/licenses/>.
+#
+# Author: Chenxiong Qi
+
+import os
+import itertools
+import unittest
+import tempfile
+import shutil
+
+import koji
+
+"""
+This test harness tests various global methods and classes within
+tools/fedabipkgdiff.
+"""
+
+try:
+    from mock import patch
+except ImportError:
+    import sys
+    print >>sys.stderr, \
+        'mock is required to run tests. Please install before running tests.'
+    sys.exit(1)
+
+import imp
+# Import the fedabipkgdiff program file from the source directory.
+fedabipkgdiff_mod = imp.load_source('fedabidiff',
+                                    '@top_srcdir@/tools/fedabipkgdiff')
+
+# Used to generate integer values (greater or equal to zero) in
+# RunAbipkgdiffTest.test_partial_failure, those values simulate return code
+# from run_abipkgdiff. To represent partial failure, counter must start from 0.
+counter = itertools.count(0)
+
+# prefix for creating a temporary file or directory. The name would be
+# fedabipkgdiff-test-slkw3ksox
+temp_file_or_dir_prefix = 'fedabipkgdiff-test-'
+
+
+class UtilsTest(unittest.TestCase):
+
+    def test_is_distro_valid(self):
+        """Test is_fedora_distro method
+
+        is_fedora_distro aims to test if a string is a valid Fedora distro. I
+        don't see there is a general rule or format definition for such a
+        Fedora distro. I refer to second part of %{dist} splited by dot as the
+        reference. Generally, fc4, fc19, fc23 are valid ones, and el6, el7 are
+        also valid one currently.
+        """
+        distro = 'fc5'
+        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'f5'
+        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'fc23'
+        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'fc'
+        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'fc234'
+        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'el7'
+        self.assertTrue(fedabipkgdiff_mod.is_distro_valid(distro))
+
+        distro = 'el7_2'
+        self.assertFalse(fedabipkgdiff_mod.is_distro_valid(distro))
+
+
+class RPMTest(unittest.TestCase):
+    """Test case for RPM class
+
+    RPM class is a class wrapping a underlying dict object represeting a RPM
+    information, that is returned from Koji XMLRPC APIs.
+
+    This test aims to test the class to see if RPM attributes is accessible in
+    Python class attribute way, and if a RPM is specific type of RPM, for
+    example, if it's a debuginfo.
+    """
+
+    def setUp(self):
+        """Setup test data for testing RPM class
+
+        According to the tests, it's unnecessary to contruct a complete dict
+        containing full RPM information. So, only part of of them is
+        enough. This test case only need name, version, release, and arch.
+
+        In case Koji changes the name of name, version, release or arch in the
+        future to express same meaning individually, (I don't think it could
+        happen), please update there also.
+        """
+
+        # Argument passed to RPM.__init__ to construct a RPM class object, that
+        # represents a debuginfo RPM.
+        self.debuginfo_rpm_info = {
+            'arch': 'i686',
+            'name': 'httpd-debuginfo',
+            'release': '1.fc22',
+            'version': '2.4.18'
+            }
+
+        # Argument passed to RPM.__init__ to construct a RPM class object, that
+        # represents a RPM.
+        self.rpm_info = {
+            'arch': 'x86_64',
+            'name': 'httpd',
+            'release': '1.fc22',
+            'version': '2.4.18'
+            }
+
+    def test_attribute_access(self):
+        """Ensure wrapped RPM information is accessible via attribute"""
+        rpm = fedabipkgdiff_mod.RPM(self.debuginfo_rpm_info)
+        self.assertEquals(self.debuginfo_rpm_info['arch'], rpm.arch)
+        self.assertEquals(self.debuginfo_rpm_info['name'], rpm.name)
+        self.assertEquals(self.debuginfo_rpm_info['release'], rpm.release)
+        self.assertEquals(self.debuginfo_rpm_info['version'], rpm.version)
+
+    def test_raise_error_if_name_not_exist(self):
+        """
+        Ensure AttributeError should be raised when accessing a non-existent
+        attribute
+        """
+        rpm = fedabipkgdiff_mod.RPM({})
+        try:
+            rpm.xxxxx
+        except AttributeError:
+            # Succeed, exit normally
+            return
+        self.fail('AttributeError should be raised, but not.')
+
+    def test_is_debuginfo(self):
+        """Ensure to return True if a RPM's name contains -debuginfo"""
+        rpm = fedabipkgdiff_mod.RPM(self.debuginfo_rpm_info)
+        self.assertTrue(rpm.is_debuginfo)
+
+        rpm = fedabipkgdiff_mod.RPM(self.rpm_info)
+        self.assertFalse(rpm.is_debuginfo)
+
+    def test_nvra(self):
+        """
+        Ensure value from RPM.nvra is parsable and contains correct value from
+        underlying RPM information
+        """
+        rpm = fedabipkgdiff_mod.RPM(self.rpm_info)
+        nvra = koji.parse_NVRA(rpm.nvra)
+        self.assertEquals(nvra['name'], rpm.name)
+        self.assertEquals(nvra['version'], rpm.version)
+        self.assertEquals(nvra['release'], rpm.release)
+        self.assertEquals(nvra['arch'], rpm.arch)
+
+    def test_str_representation(self):
+        """
+        Enforce a RPM object has same string represetation as underlying
+        wrapped rpm information that is a dict object.
+        """
+        rpm = fedabipkgdiff_mod.RPM(self.rpm_info)
+        self.assertEquals(str(self.rpm_info), str(rpm))
+
+
+class LocalRPMTest(unittest.TestCase):
+    """Test case for LocalRPM class
+
+    Because LocalRPM inherits from RPM, all tests against RPM class are also
+    applied to LocalRPM, so I don't repeat them again here. This test case
+    mainly focus on the abilities against files on the local disk.
+    """
+
+    def setUp(self):
+        # A RPM filename that simulates a RPM file that is stored somewhere on
+        # the disk.
+        # This is the only argument passed to LocalRPM.__init__ to initialize
+        # an object.
+        self.filename = 'httpd-2.4.18-1.fc22.x86_64.rpm'
+
+    def test_file_parser_without_path(self):
+        """Ensure LocalRPM can get RPM information from a filename
+
+        LocalRPM gets name, version, release, and arch of a RPM by parsing the
+        passed filename to __init__ method. Then, all these information is
+        accessible via LocalRPM name, version, release, and arch attribute.
+
+        A filename either with an absolute path, relative path, or without a
+        path, LocalRPM should be able to find these files and get correct
+        information by removing the potential present path. For example, by
+        giving following filenames,
+
+        - httpd-2.4.18-1.fc22.x86_64.rpm
+        - artifacts/httpd-2.4.18-1.fc22.x86_64.rpm
+        - /mnt/koji/packages/httpd/2.4.18/1.fc22/httpd-2.4.18-1.fc22.x86_64.rpm
+
+        LocalRPM has to determine the necessary RPM information from
+        httpd-2.4.18-1.fc22.x86_64.rpm
+
+        Without specifying path in the filename, it usually means LocalRPM
+        should find the RPM file relative to current working directory. So, no
+        need of additional test against a filename with a relative path.
+        """
+        rpm = fedabipkgdiff_mod.LocalRPM(self.filename)
+        nvra = koji.parse_NVRA(self.filename)
+        self.assertEquals(nvra['name'], rpm.name)
+        self.assertEquals(nvra['version'], rpm.version)
+        self.assertEquals(nvra['release'], rpm.release)
+        self.assertEquals(nvra['arch'], rpm.arch)
+
+        full_filename = os.path.join('/', 'tmp', self.filename)
+        rpm = fedabipkgdiff_mod.LocalRPM(full_filename)
+        nvra = koji.parse_NVRA(self.filename)
+        self.assertEquals(nvra['name'], rpm.name)
+        self.assertEquals(nvra['version'], rpm.version)
+        self.assertEquals(nvra['release'], rpm.release)
+        self.assertEquals(nvra['arch'], rpm.arch)
+        self.assertEquals(full_filename, rpm.downloaded_file)
+
+    @patch('os.path.exists')
+    def test_find_existent_debuginfo(self, mock_exists):
+        """Ensure LocalRPM can find an associated existent debuginfo RPM
+
+        Currently, find_debuginfo is only able to find associated debuginfo RPM
+        from the directory where local RPM resides. This test works for this
+        case at this moment. If there is a requirement to allow find debuginfo
+        RPM from somewhere else, any level of subdirectory for instance, add
+        new test case for that, and update these words you are reading :)
+        """
+        mock_exists.return_value = True
+
+        rpm = fedabipkgdiff_mod.LocalRPM(self.filename)
+        self.assertTrue(isinstance(rpm, fedabipkgdiff_mod.LocalRPM))
+
+        nvra = koji.parse_NVRA(self.filename)
+        expected_debuginfo = fedabipkgdiff_mod.LocalRPM(
+            '%(name)s-debuginfo-%(version)s-%(release)s.%(arch)s.rpm' % nvra)
+        debuginfo = rpm.find_debuginfo()
+        self.assertEquals(expected_debuginfo.name, debuginfo.name)
+        self.assertEquals(expected_debuginfo.version, debuginfo.version)
+        self.assertEquals(expected_debuginfo.release, debuginfo.release)
+
+    def test_find_non_existent_debuginfo(self):
+        """Ensure to return None if cannot find associated debuginfo RPM
+
+        os.path.exists is not mocked, that is because the associated debuginfo
+        RPM of httpd-2.4.18-1.fc22.x86_64.rpm given in setUp must be
+        non-existed during this test's run.
+        """
+        rpm = fedabipkgdiff_mod.LocalRPM(self.filename)
+        self.assertEquals(None, rpm.find_debuginfo())
+
+
+class RunAbipkgdiffTest(unittest.TestCase):
+    """Test case for method run_abipkgdiff
+
+    Method run_abipkgdiff accepts package informations and passes them to and
+    run abipkgdiff command line utility. Since run_abipkgdiff does not catch
+    output to either standard output or standard error, and only returns the
+    return code that is returned from underlying abipkgdiff, these various test
+    cases test whether run_abipkgdiff is able to return the return code
+    correctly.
+    """
+
+    def setUp(self):
+        """Define packages information for calling run_abipkgdiff method
+
+        Due to the tests just care about the return code from underlying
+        abipkgdiff, only partial attributes of a RPM is required. That means,
+        it's unnecessary to give a full dict representing a complete RPM, just
+        build_id, name, version, release, and arch.
+
+        Full RPM information is not required. For this test case, only partial
+        information arch, build_id, name, release, and version are enough.
+        """
+
+        # Used for testing the case of running abipkgdiff against one RPM
+        self.pkg1_single_info = {
+            'i686': [
+                fedabipkgdiff_mod.RPM({'arch': 'i686',
+                                       'build_id': 720222,
+                                       'name': 'httpd',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                fedabipkgdiff_mod.RPM({'arch': 'i686',
+                                       'build_id': 720222,
+                                       'name': 'httpd-debuginfo',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       })
+                ],
+            }
+
+        # Whatever the concrete content of pkg2_infos is, so just make a copy
+        # from self.pkg1_infos
+        self.pkg2_single_info = self.pkg1_single_info.copy()
+
+        # Used for testing the case of running abipkgdiff against multiple RPMs
+        self.pkg1_infos = {
+            'i686': [
+                fedabipkgdiff_mod.RPM({'arch': 'i686',
+                                       'build_id': 720222,
+                                       'name': 'httpd',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                fedabipkgdiff_mod.RPM({'arch': 'i686',
+                                       'build_id': 720222,
+                                       'name': 'httpd-debuginfo',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                ],
+            'x86_64': [
+                fedabipkgdiff_mod.RPM({'arch': 'x86_64',
+                                       'build_id': 720222,
+                                       'name': 'httpd',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                fedabipkgdiff_mod.RPM({'arch': 'x86_64',
+                                       'build_id': 720222,
+                                       'name': 'httpd-debuginfo',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                ],
+            'armv7hl': [
+                fedabipkgdiff_mod.RPM({'arch': 'armv7hl',
+                                       'build_id': 720222,
+                                       'name': 'httpd',
+                                       'release': '2.fc24',
+                                      'version': '2.4.18',
+                                       }),
+                fedabipkgdiff_mod.RPM({'arch': 'armv7hl',
+                                       'build_id': 720222,
+                                       'name': 'httpd-debuginfo',
+                                       'release': '2.fc24',
+                                       'version': '2.4.18',
+                                       }),
+                ],
+            }
+
+        # Whatever the concrete content of pkg2_infos is, so just make a copy
+        # from self.pkg1_infos
+        self.pkg2_infos = self.pkg1_infos.copy()
+
+    @patch('fedabidiff.abipkgdiff')
+    def test_all_success(self, mock_abipkgdiff):
+        """
+        Ensure run_abipkgdiff returns 0 when it succeeds to run against one or
+        more packages.
+        """
+        mock_abipkgdiff.return_value = 0
+
+        result = fedabipkgdiff_mod.run_abipkgdiff(self.pkg1_single_info,
+                                                  self.pkg2_single_info)
+        self.assertEquals(0, result)
+
+        result = fedabipkgdiff_mod.run_abipkgdiff(self.pkg1_infos,
+                                                  self.pkg2_infos)
+        self.assertEquals(0, result)
+
+    @patch('fedabidiff.abipkgdiff')
+    def test_all_failure(self, mock_abipkgdiff):
+        """
+        Ensure run_abipkgdiff returns the return code from underlying
+        abipkgdiff when all calls to abipkgdiff fails against one or more
+        packages.
+        """
+        mock_abipkgdiff.return_value = 4
+
+        result = fedabipkgdiff_mod.run_abipkgdiff(self.pkg1_single_info,
+                                                  self.pkg2_single_info)
+        self.assertEquals(4, result)
+
+        result = fedabipkgdiff_mod.run_abipkgdiff(self.pkg1_infos,
+                                                  self.pkg2_infos)
+        self.assertEquals(4, result)
+
+    @patch('fedabidiff.abipkgdiff', new=lambda param1, param2: counter.next())
+    def test_partial_failure(self):
+        """
+        Ensure run_abipkgdiff returns non-zero when partial calls to
+        run_abipkgdiff succeed
+
+        abipkgdiff is mocked in order to simulte the partial success
+        calls. Why? That is because, counter starts from 0. So, it will
+        generate 0, 1, 2, ...
+        """
+        result = fedabipkgdiff_mod.run_abipkgdiff(self.pkg1_infos,
+                                                  self.pkg2_infos)
+        self.assertTrue(result > 0)
+
+
+class DownloadRPMTest(unittest.TestCase):
+    """Test case for download_rpm
+
+    Download a remote file, which is a local file simulating a remote file with
+    scheme file://, for example file:///tmp/a.txt, to download directory.
+    """
+
+    def setUp(self):
+        # Create a remote file for testing download of this file
+        self.fd, self.remote_filename = tempfile.mkstemp(
+            prefix=temp_file_or_dir_prefix)
+        # Whatever the content is, this case does not care about. Close it
+        # immediately.
+        os.close(self.fd)
+
+        # Used as a fake download directory to mock get_download_dir method
+        self.download_dir = tempfile.mkdtemp(prefix=temp_file_or_dir_prefix)
+
+    def tearDown(self):
+        os.remove(self.remote_filename)
+        shutil.rmtree(self.download_dir)
+
+    def make_remote_file_url(self):
+        """Make URL of remote file that is used for downloading this file"""
+        return 'file://{}'.format(self.remote_filename)
+
+    def make_nonexistent_remote_file_url(self):
+        """Return URL to a non-existent remote file"""
+        return os.path.join(self.make_remote_file_url(), 'nonexistent-file')
+
+    @patch('__main__.fedabipkgdiff_mod.get_download_dir')
+    def test_succeed_to_download_a_rpm(self, mock_get_download_dir):
+        """Enusre True is returned if curl succeeds to download remote file
+
+        Download remote file to a fake download directory. Ensure everything is
+        okay, and return value from download_rpm should be truth.
+        """
+        mock_get_download_dir.return_value = self.download_dir
+
+        url = self.make_remote_file_url()
+        ret = fedabipkgdiff_mod.download_rpm(url)
+        self.assertTrue(ret)
+
+    @patch('__main__.fedabipkgdiff_mod.get_download_dir')
+    def test_failed_to_download_a_rpm(self, mock_get_download_dir):
+        """Ensure False is returned if curl fails to download remote file
+
+        Download remote file to a fake download directory. But, making
+        something wrong to cause download_rpm returns false.
+        """
+        mock_get_download_dir.return_value = self.download_dir
+
+        url = self.make_nonexistent_remote_file_url()
+        ret = fedabipkgdiff_mod.download_rpm(url)
+        self.assertFalse(ret)
+
+
+class MockGlobalConfig(object):
+    """Used to mock global_config
+
+    Since tests do not parse options from command line, so this class is
+    helpful for tests to contain all potential parsed (simulated)
+    options.
+
+    Currently, only koji_server is required for running tests. If any new test
+    cases need others, please add them add as class attribute directly.
+    """
+    koji_server = fedabipkgdiff_mod.DEFAULT_KOJI_SERVER
+
+
+def mock_get_session():
+    """Used to mock get_session method to get mocked KojiSession instance"""
+    return MockKojiClientSession(baseurl=fedabipkgdiff_mod.DEFAULT_KOJI_SERVER)
+
+
+class MockKojiClientSession(object):
+    """Mock koji.ClientSession
+
+    This mock ClientSession aims to avoid touching a real Koji instance to
+    interact with XMLRPC APIs required by fedabipkgdiff.
+
+    For the tests within this module, methods do not necessarily to return
+    complete RPM and build information. So, if you need more additional
+    information, here is the right place to add them.
+    """
+
+    def __init__(self, *args, **kwargs):
+        """Accept arbitrary parameters but do nothing for this mock
+
+        Add this method, although it's not used by this mock ClientSession.
+        That is because, when initiate an instance of koji.ClientSession,
+        paramters, at least URL to kojihub, is required and passed.
+        """
+        self.args = args
+        self.kwargs = kwargs
+
+    def getPackage(self, *args, **kwargs):
+        return {
+            'id': 1,
+            'name': 'whatever a name of a package',
+        }
+
+    def listRPMs(self, *args, **kwargs):
+        return [{'arch': 'i686',
+                 'name': 'httpd-debuginfo',
+                 'nvr': 'httpd-debuginfo-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'},
+                {'arch': 'i686',
+                 'name': 'mod_session',
+                 'nvr': 'mod_session-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'},
+                {'arch': 'i686',
+                 'name': 'httpd',
+                 'nvr': 'httpd-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'},
+                {'arch': 'i686',
+                 'name': 'mod_proxy_html',
+                 'nvr': 'mod_proxy_html-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'},
+                {'arch': 'i686',
+                 'name': 'mod_ldap',
+                 'nvr': 'mod_ldap-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'},
+                {'arch': 'i686',
+                 'name': 'mod_ssl',
+                 'nvr': 'mod_ssl-2.4.18-1.fc22',
+                 'release': '1.fc22',
+                 'version': '2.4.18'}]
+
+    def listBuilds(self, *args, **kwargs):
+        return [
+            {'build_id': 720222,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.18-2.fc24',
+             'release': '2.fc24',
+             'version': '2.4.18'},
+            {'build_id': 708769,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.18-1.fc22',
+             'release': '1.fc22',
+             'version': '2.4.18'},
+            {'build_id': 708711,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.18-1.fc23',
+             'release': '1.fc23',
+             'version': '2.4.18'},
+            {'build_id': 705335,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.18-1.fc24',
+             'release': '1.fc24',
+             'version': '2.4.18'},
+            {'build_id': 704434,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.17-4.fc24',
+             'release': '4.fc24',
+             'version': '2.4.17'},
+            {'build_id': 704433,
+             'name': 'httpd',
+             'nvr': 'httpd-2.4.17-4.fc23',
+             'release': '4.fc23',
+             'version': '2.4.17'},
+        ]
+
+
+class GetPackageLatestBuildTest(unittest.TestCase):
+    """Test case for get_package_latest_build"""
+
+    @patch('fedabidiff.global_config', new=MockGlobalConfig)
+    @patch('koji.ClientSession', new=MockKojiClientSession)
+    def test_get_latest_one(self):
+        """Ensure to get latest build of a package
+
+        Brew.listBuilds is mocked to return a series of builds that contains
+        a latest build, which is httpd-2.4.18-1.fc23, of package httpd for
+        Fedora 23, and that must be found and returned.
+        """
+        session = fedabipkgdiff_mod.get_session()
+        build = session.get_package_latest_build('httpd', 'fc23')
+        self.assertEquals('httpd-2.4.18-1.fc23', build['nvr'])
+
+    @patch('fedabidiff.global_config', new=MockGlobalConfig)
+    @patch('koji.ClientSession', new=MockKojiClientSession)
+    def test_cannot_find_a_latest_build_with_invalid_distro(self):
+        """
+        Ensure NoCompleteBuilds is raised when trying to find a latest build of
+        a package for unknown Fedora distribution.
+        """
+        session = fedabipkgdiff_mod.get_session()
+        self.assertRaises(fedabipkgdiff_mod.NoCompleteBuilds,
+                          session.get_package_latest_build, 'httpd', 'xxxx')
+
+
+class BrewListRPMsTest(unittest.TestCase):
+    """Test case for Brew.listRPMs"""
+
+    @patch('fedabidiff.global_config', new=MockGlobalConfig)
+    @patch('fedabidiff.koji.ClientSession', new=MockKojiClientSession)
+    def test_select_specific_rpms(self):
+        """Ensure Brew.listRPMs can select RPMs by a specific selector
+
+        This test will select RPMs whose name starts with httpd, that is only
+        httpd and httpd-debuginfo RPMs are selected and returned.
+        """
+        session = fedabipkgdiff_mod.get_session()
+        selector = lambda rpm: rpm['name'].startswith('httpd')
+        rpms = session.listRPMs(buildID=1000, selector=selector)
+        self.assertTrue(
+            len(rpms) > 0,
+            'More than one rpms should be selected. But, it\'s empty.')
+        for rpm in rpms:
+            self.assertTrue(rpm['name'] in ('httpd', 'httpd-debuginfo'),
+                            '{0} should not be selected'.format(rpm['name']))
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tools/Makefile.am b/tools/Makefile.am
index b855f41..0d96215 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -6,6 +6,12 @@ else
   bin_PROGRAMS = abidiff abilint abidw abicompat abipkgdiff
 endif
 
+if ENABLE_FEDABIPKGDIFF
+  bin_SCRIPTS = fedabipkgdiff
+else
+  noinst_SCRIPTS = fedabipkgdiff
+endif
+
 noinst_PROGRAMS = abisym abinilint
 
 if ENABLE_ZIP_ARCHIVE
diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff
new file mode 100755
index 0000000..e7c4785
--- /dev/null
+++ b/tools/fedabipkgdiff
@@ -0,0 +1,1050 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# -*- Mode: Python
+#
+# Copyright (C) 2013-2016 Red Hat, Inc.
+#
+# This file is part of the GNU Application Binary Interface Generic
+# Analysis and Instrumentation Library (libabigail).  This library is
+# free software; you can redistribute it and/or modify it under the
+# terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 3, or (at your option) any
+# later version.
+#
+# This library is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public
+# License along with this program; see the file COPYING-GPLV3.  If
+# not, see <http:#www.gnu.org/licenses/>.
+#
+# Author: Chenxiong Qi
+
+import argparse
+import logging
+import os
+import re
+import shlex
+import subprocess
+import sys
+
+from collections import namedtuple
+from itertools import groupby
+
+import xdg.BaseDirectory
+
+import koji
+
+# @file
+#
+# You might have known that abipkgdiff is a command line tool to compare two
+# RPM packages to find potential differences of ABI. This is really useful for
+# Fedora packagers and developers. Usually, excpet the RPM packages built
+# locally, if a packager wants to compare RPM packages he just built with
+# specific RPM packages that were already built and availabe in Koji,
+# fedabipkgdiff is the right tool for him.
+#
+# With fedabipkgdiff, packager is able to specify certain criteria to tell
+# fedabipkgdiff which RPM packages he wants to compare, then fedabipkgdiff will
+# find them, download them, and boom, run the abipkgdiff for you.
+#
+# Currently, fedabipkgdiff returns 0 if everything works well, otherwise, 1 if
+# something wrong.
+
+
+DEFAULT_KOJI_SERVER = 'http://koji.fedoraproject.org/kojihub'
+DEFAULT_KOJI_TOPDIR = 'https://kojipkgs.fedoraproject.org'
+
+# The working directory where to hold all data including downloaded RPM
+# packages Currently, it's not configurable and hardcode here. In the future
+# version of fedabipkgdiff, I'll make it configurable by users.
+HOME_DIR = os.path.join(xdg.BaseDirectory.xdg_data_home,
+                        os.path.splitext(os.path.basename(__file__))[0])
+
+# Used to construct abipkgdiff command line argument, package and associated
+# debuginfo package
+# fedabipkgdiff runs abipkgdiff in this form
+#
+#   abipkgdiff \
+#       --d1 /path/to/package1-debuginfo.rpm \
+#       --d2 /path/to/package2-debuginfo.rpm \
+#       /path/to/package1.rpm \
+#       /path/to/package2.rpm
+#
+# PkgInfo is a two-elements tuple in format
+#
+#   (/path/to/package1.rpm, /path/to/package1-debuginfo.rpm)
+#
+# So, before calling abipkgdiff, fedabipkgdiff must prepare and pass following
+# two package information
+#
+#   (/path/to/package1.rpm, /path/to/package1-debuginfo.rpm)
+#   (/path/to/package2.rpm, /path/to/package2-debuginfo.rpm)
+#
+PkgInfo = namedtuple('PkgInfo', 'package debuginfo_package')
+
+
+global_config = None
+pathinfo = None
+session = None
+
+# There is no way to configure the log format so far. I hope I would have time
+# to make it available so that if fedabipkgdiff is scheduled and run by some
+# service, the logs logged into log file is muc usable.
+logging.basicConfig(format='[%(levelname)s] %(message)s',
+                    level=logging.CRITICAL)
+logger = logging.getLogger(os.path.basename(__file__))
+
+
+class KojiPackageNotFound(Exception):
+    """Package is not found in Koji"""
+
+
+class PackageNotFound(Exception):
+    """Package is not found locally"""
+
+
+class RpmNotFound(Exception):
+    """RPM is not found"""
+
+
+class NoBuildsError(Exception):
+    """No builds returned from a method to select specific builds"""
+
+
+class NoCompleteBuilds(Exception):
+    """No complete builds for a package
+
+    This is a serious problem, nothing can be done if there is no complete
+    builds for a package.
+    """
+
+
+class InvalidDistroError(Exception):
+    """Invalid distro error"""
+
+
+class CannotFindLatestBuildError(Exception):
+    """Cannot find latest build from a package"""
+
+
+def is_distro_valid(distro):
+    """Adjust if a distro is valid
+
+    Currently, check for Fedora and RHEL.
+
+    :param str distro: a string representing a distro value.
+    :return: True if distro is the one specific to Fedora, like fc24, el7.
+    "rtype: bool
+    """
+    return re.match(r'^(fc|el)\d{1,2}$', distro) is not None
+
+
+def log_call(func):
+    """A decorator that logs a method invocation
+
+    Method's name and all arguments, either positional or keyword arguments,
+    will be logged by logger.debug. Also, return value from the decorated
+    method will be logged just after the invocation is done.
+
+    This decorator does not catch any exception thrown from the decorated
+    method. If there is any exception thrown from decorated method, you can
+    catch them in the caller and obviously, no return value is logged.
+
+    :param callable func: a callable object to decorate
+    """
+    def proxy(*args, **kwargs):
+        logger.debug('Call %s, args: %s, kwargs: %s',
+                     func.__name__,
+                     args if args else '',
+                     kwargs if kwargs else '')
+        result = func(*args, **kwargs)
+        logger.debug('Result from %s: %s', func.__name__, result)
+        return result
+    return proxy
+
+
+class RPM(object):
+    """Wrapper of RPM representing a RPM got from Koji
+
+    A RPM is returned from Koji XMLRPC API is in dict type. This wrapper makes
+    it eaiser to access all these properties in the way of object.property.
+    """
+
+    def __init__(self, rpm_info):
+        """Initialize a RPM object
+
+        :param dict rpm_info: a dict representing a RPM information got from
+        koji API, either listRPMs or getRPM
+        """
+        self.rpm_info = rpm_info
+
+    def __str__(self):
+        """Return the string representation of this RPM
+
+        Return the string representation of RPM information returned from Koji
+        directly so that RPM can be treated in same way.
+        """
+        return str(self.rpm_info)
+
+    def __getattr__(self, name):
+        """Access RPM information in the way of object.property
+
+        :param str name: the property name to access.
+        :raises AttributeError: if name is not one of keys of RPM information.
+        """
+        if name in self.rpm_info:
+            return self.rpm_info[name]
+        else:
+            raise AttributeError('No attribute name {0}'.format(name))
+
+    @property
+    def nvra(self):
+        """Return a RPM's N-V-R-A representation
+
+        An example: libabigail-1.0-0.8.rc4.1.fc23.x86_64
+        """
+        return '%(name)s-%(version)s-%(release)s.%(arch)s' % self.rpm_info
+
+    @property
+    def filename(self):
+        """Return a RPM file name
+
+        An example: libabigail-1.0-0.8.rc4.1.fc23.x86_64.rpm
+        """
+        return '{0}.rpm'.format(self.nvra)
+
+    @property
+    def is_debuginfo(self):
+        """Check if the name of the current RPM denotes a debug info package"""
+        return koji.is_debuginfo(self.rpm_info['name'])
+
+    @property
+    def download_url(self):
+        """Get the URL from where to download this RPM"""
+        build = session.getBuild(self.build_id)
+        return os.path.join(pathinfo.build(build), pathinfo.rpm(self.rpm_info))
+
+    @property
+    def downloaded_file(self):
+        """Get a pridictable downloaded file name with absolute path"""
+        # arch should be removed from the result returned from PathInfo.rpm
+        filename = os.path.basename(pathinfo.rpm(self.rpm_info))
+        return os.path.join(get_download_dir(), filename)
+
+    @property
+    def is_downloaded(self):
+        """Check if this RPM was already downloaded to local disk"""
+        return os.path.exists(self.downloaded_file)
+
+
+class LocalRPM(RPM):
+    """Representing a local RPM
+
+    Local RPM means the one that could be already downloaded or built from
+    where I can find it
+    """
+
+    def __init__(self, filename):
+        """Initialize local RPM with a filename
+
+        :param str filename: a filename pointing to a RPM file in local
+        disk. Note that, this file must not exist necessarily.
+        """
+        self.local_filename = filename
+        self.rpm_info = koji.parse_NVRA(os.path.basename(filename))
+
+    @property
+    def downloaded_file(self):
+        """Return filename of this RPM
+
+        Returned filename is just the one passed when initializing this RPM.
+
+        :return: filename of this RPM
+        :rtype: str
+        """
+        return self.local_filename
+
+    @property
+    def download_url(self):
+        raise NotImplementedError('LocalRPM has no URL to download')
+
+    @log_call
+    def find_debuginfo(self):
+        """Find debuginfo RPM package from a directory
+
+        :param str rpm_file: the rpm file name
+        :return: the absolute file name of the found debuginfo rpm
+        :rtype: str or None
+        """
+        search_dir = os.path.dirname(os.path.abspath(self.local_filename))
+        filename = \
+            '%(name)s-debuginfo-%(version)s-%(release)s.%(arch)s.rpm' % \
+            self.rpm_info
+        filename = os.path.join(search_dir, filename)
+        return LocalRPM(filename) if os.path.exists(filename) else None
+
+
+class Brew(object):
+    """Interface to Koji XMLRPC API with enhancements specific to fedabipkgdiff
+
+    kojihub XMLRPC APIs are well-documented in koji's source code. For more
+    details information, please refer to class RootExports within kojihub.py.
+
+    For details of APIs used within fedabipkgdiff, refer to from line
+
+    https://pagure.io/koji/blob/master/f/hub/kojihub.py#_7835
+    """
+
+    def __init__(self, baseurl):
+        """Initialize Brew
+
+        :param str baseurl: the kojihub URL to initialize a session, that is
+        used to access koji XMLRPC APIs.
+        """
+        self.session = koji.ClientSession(baseurl)
+
+    @log_call
+    def listRPMs(self, buildID=None, arches=None, selector=None):
+        """Get list of RPMs of a build from Koji
+
+        Call kojihub.listRPMs to get list of RPMs. Return selected RPMs without
+        changing each RPM information.
+
+        A RPM returned from listRPMs contains following keys:
+
+        - id
+        - name
+        - version
+        - release
+        - nvr (synthesized for sorting purposes)
+        - arch
+        - epoch
+        - payloadhash
+        - size
+        - buildtime
+        - build_id
+        - buildroot_id
+        - external_repo_id
+        - external_repo_name
+        - metadata_only
+        - extra
+
+        :param int buildID: id of a build from which to list RPMs.
+        :param arches: to restrict to list RPMs with specified arches.
+        :type arches: list or tuple
+        :param selector: called to determine if a RPM should be selected and
+        included in the final returned result. Selector must be a callable
+        object and accepts one parameter of a RPM.
+        :type selector: a callable object
+        :return: a list of RPMs, each of them is a dict object
+        :rtype: list
+        """
+        if selector:
+            assert hasattr(selector, '__call__'), 'selector must be callable.'
+        rpms = self.session.listRPMs(buildID=buildID, arches=arches)
+        if selector:
+            rpms = [rpm for rpm in rpms if selector(rpm)]
+        return rpms
+
+    @log_call
+    def getRPM(self, rpminfo):
+        """Get a RPM from koji
+
+        Call kojihub.getRPM, and returns the result directly without any
+        change.
+
+        When not found a RPM, koji.getRPM will return None, then
+        this method will raise RpmNotFound error immediately to claim what is
+        happening. I want to raise fedabipkgdiff specific error rather than
+        koji's GenericError and then raise RpmNotFound again, so I just simply
+        don't use strict parameter to call koji.getRPM.
+
+        :param rpminfo: rpminfo may be a N-V-R.A or a map containing name,
+        version, release, and arch. For example, file-5.25-5.fc24.x86_64, and
+        `{'name': 'file', 'version': '5.25', 'release': '5.fc24', 'arch':
+        'x86_64'}`.
+        :type rpminfo: str or dict
+        :return: a map containing RPM information, that contains same keys as
+        method `Brew.listRPMs`.
+        :rtype: dict
+        :raises RpmNotFound: if a RPM cannot be found with rpminfo.
+        """
+        rpm = self.session.getRPM(rpminfo)
+        if rpm is None:
+            raise RpmNotFound('Cannot find RPM {0}'.format(args[0]))
+        return rpm
+
+    @log_call
+    def listBuilds(self, packageID, state=None, topone=None,
+                   selector=None, order_by=None, reverse=None):
+        """Get list of builds from Koji
+
+        Call kojihub.listBuilds, and return selected builds without changing
+        each build information.
+
+        By default, only builds with COMPLETE state are queried and returns
+        afterwards.
+
+        :param int packageID: id of package to list builds from.
+        :param int state: build state. There are five states of a build in
+        Koji. fedabipkgdiff only cares about builds with COMPLETE state. If
+        state is omitted, builds with COMPLETE state are queried from Koji by
+        default.
+        :param bool topone: just return the top first build.
+        :param selector: a callable object used to select specific subset of
+        builds. Selector will be called immediately after Koji returns queried
+        builds. When each call to selector, a build is passed to
+        selector. Return True if select current build, False if not.
+        :type selector: a callable object
+        :param str order_by: the attribute name by which to order the builds,
+        for example, name, version, or nvr.
+        :param bool reverse: whether to order builds reversely.
+        :return: a list of builds, even if there is only one build.
+        :rtype: list
+        :raises TypeError: if selector is not callable, or if order_by is not a
+        string value.
+        """
+        if state is None:
+            state = koji.BUILD_STATES['COMPLETE']
+
+        if selector is not None and not hasattr(selector, '__call__'):
+            raise TypeError(
+                '{0} is not a callable object.'.format(str(selector)))
+
+        if order_by is not None and not isinstance(order_by, basestring):
+            raise TypeError('order_by {0} is invalid.'.format(order_by))
+
+        builds = self.session.listBuilds(packageId=packageID, state=state)
+        if selector is not None:
+            builds = [build for build in builds if selector(build)]
+        if order_by is not None:
+            # FIXME: is it possible to sort builds by using opts parameter of
+            # listBuilds
+            builds = sorted(builds,
+                            key=lambda item: item[order_by],
+                            reverse=reverse)
+        if topone:
+            builds = builds[0:1]
+
+        return builds
+
+    @log_call
+    def getPackage(self, name):
+        """Get a package from Koji
+
+        :param str name: a package name.
+        :return: a mapping containing package information. For example,
+        `{'id': 1, 'name': 'package'}`.
+        :rtype: dict
+        """
+        package = self.session.getPackage(name)
+        if package is None:
+            package = self.session.getPackage(name.rsplit('-', 1)[0])
+            if package is None:
+                raise KojiPackageNotFound(
+                    'Cannot find package {0}.'.format(name))
+        return package
+
+    @log_call
+    def getBuild(self, buildID):
+        """Get a build from Koji
+
+        Call kojihub.getBuild. Return got build directly without change.
+
+        :param int buildID: id of build to get from Koji.
+        :return: the found build. Return None, if not found a build with
+        buildID.
+        :rtype: dict
+        """
+        return self.session.getBuild(buildID)
+
+    @log_call
+    def get_rpm_build_id(self, name, version, release, arch=None):
+        """Get build ID that contains a RPM with specific nvra
+
+        If arch is not omitted, a RPM can be identified by its N-V-R-A.
+
+        If arch is omitted, name is used to get associated package, and then
+        to get the build.
+
+        Example:
+
+        >>> brew = Brew('url to kojihub')
+        >>> brew.get_rpm_build_id('httpd', '2.4.18', '2.fc24')
+        >>> brew.get_rpm_build_id('httpd', '2.4.18', '2.fc25', 'x86_64')
+
+        :param str name: name of a rpm
+        :param str version: version of a rpm
+        :param str release: release of a rpm
+        :param arch: arch of a rpm
+        :type arch: str or None
+        :return: id of the build from where the RPM is built
+        :rtype: dict
+        :raises KojiPackageNotFound: if name is not found from Koji if arch
+        is None.
+        """
+        if arch is None:
+            package = self.getPackage(name)
+            selector = lambda item: item['version'] == version and \
+                item['release'] == release
+            builds = self.listBuilds(packageID=package['id'],
+                                     selector=selector)
+            if not builds:
+                raise NoBuildsError(
+                    'No builds are selected from package {0}.'.format(
+                        package['name']))
+            return builds[0]['build_id']
+        else:
+            rpm = self.getRPM({'name': name,
+                               'version': version,
+                               'release': release,
+                               'arch': arch,
+                               })
+            return rpm['build_id']
+
+    @log_call
+    def get_package_latest_build(self, package_name, distro):
+        """Get latest build from a package
+
+        Example:
+
+        >>> brew = Brew('url to kojihub')
+        >>> brew.get_package_latest_build('httpd', 'fc24')
+
+        :param str package_name: from which package to get the latest build
+        :param str distro: which distro the latest build belongs to
+        :return: the found build
+        :rtype: dict or None
+        :raises NoCompleteBuilds: if there is no latest build of a package.
+        """
+        package = self.getPackage(package_name)
+        selector = lambda item: item['release'].find(distro) > -1
+
+        builds = self.listBuilds(packageID=package['id'],
+                                 selector=selector,
+                                 order_by='nvr',
+                                 reverse=True)
+        if not builds:
+            raise NoCompleteBuilds(
+                'No complete builds of package {0}'.format(package_name))
+
+        return builds[0]
+
+    @log_call
+    def select_rpms_from_a_build(self, build_id, package_name, arches=None,
+                                 select_subpackages=None):
+        """Select specific RPMs within a build
+
+        RPMs could be filtered be specific criterias by the parameters.
+
+        By default, fedabipkgdiff requires RPM package and associated debuginfo
+        package, both of these two packages are selected, and noarch and src
+        are excluded.
+
+        :param int build_id: from which build to select rpms.
+        :param str package_name: which rpm to select that matches this name.
+        :param arches: which arches to select. If arches omits, rpms with all
+        arches except noarch and src will be selected.
+        :type arches: list, tuple or None
+        :param bool select_subpackages: indicate whether to select all RPMs
+        with specific arch from build.
+        :return: a list of RPMs returned from listRPMs
+        :rtype: list
+        """
+        excluded_arches = ('noarch', 'src')
+
+        def rpms_selector(package_name, excluded_arches):
+            return lambda rpm: \
+                rpm['arch'] not in excluded_arches and \
+                (rpm['name'] == package_name or
+                 rpm['name'].endswith('-debuginfo'))
+
+        if select_subpackages:
+            selector = lambda rpm: rpm['arch'] not in excluded_arches
+        else:
+            selector = rpms_selector(package_name, excluded_arches)
+        rpm_infos = self.listRPMs(buildID=build_id,
+                                  arches=arches,
+                                  selector=selector)
+        return [RPM(rpm_info) for rpm_info in rpm_infos]
+
+    @log_call
+    def get_latest_built_rpms(self, package_name, distro, arches=None):
+        """Get RPMs from latest build of a package
+
+        :param str package_name: from which package to get the rpms
+        :param str distro: which distro the rpms belong to
+        :param arches: which arches the rpms belong to
+        :type arches: str or None
+        :return: the selected RPMs
+        :rtype: list
+        """
+        latest_build = self.get_package_latest_build(package_name, distro)
+        # Get rpm and debuginfo rpm from each arch
+        return self.select_rpms_from_a_build(latest_build['build_id'],
+                                             package_name,
+                                             arches=arches)
+
+
+@log_call
+def get_session():
+    """Get instance of Brew to talk with Koji"""
+    return Brew(global_config.koji_server)
+
+
+@log_call
+def get_download_dir():
+    """Return the directory holding all downloaded RPMs
+
+    If directory does not exist, it is created automatically.
+
+    :return: path to directory holding downloaded RPMs.
+    :rtype: str
+    """
+    download_dir = os.path.join(HOME_DIR, 'downloads')
+    if not os.path.exists(download_dir):
+        os.makedirs(download_dir)
+    return download_dir
+
+
+@log_call
+def download_rpm(url):
+    """Using curl to download a RPM from Koji
+
+    Currently, curl is called and runs in a spawned process. pycurl would be a
+    good way instead. This would be changed in the future.
+
+    :param str url: URL of a RPM to download.
+    :return: True if a RPM is downloaded successfully, False otherwise.
+    :rtype: bool
+    """
+    cmd = 'curl --silent -C - -O {} > {}'.format(
+        url, os.path.join(get_download_dir(),
+                          os.path.basename(url)))
+    return_code = subprocess.call(cmd, shell=True)
+    if return_code > 0:
+        logger.error('curl fails with returned code: %d.', return_code)
+        return False
+    return True
+
+
+@log_call
+def download_rpms(rpms):
+    """Download RPMs
+
+    :param list rpms: list of RPMs to download.
+    """
+    def _download(rpm):
+        if rpm.is_downloaded:
+            logger.debug('Reuse %s', rpm.downloaded_file)
+        else:
+            logger.debug('Download %s', rpm.download_url)
+            download_rpm(rpm.download_url)
+
+    map(_download, rpms)
+
+
+@log_call
+def abipkgdiff(pkg_info1, pkg_info2):
+    """Run abipkgdiff against found two RPM packages
+
+    Construct and execute abipkgdiff to get ABI diff
+
+    abipkgdiff \
+        --d1 package1-debuginfo --d2 package2-debuginfo \
+        package1-rpm package2-rpm
+
+    Output to stdout or stderr from abipkgdiff is not captured. abipkgdiff is
+    called synchronously. fedabipkgdiff does not return until underlying
+    abipkgdiff finishes.
+
+    :param PkgInfo pkg_info1: the first package information provided for
+    abipkgdiff package1 paramter.
+    :param PkgInfo pkg_info2: the second package information provided for
+    abipkgdiff package2 paramter.
+    :return: return code of underlying abipkgdiff execution.
+    :rtype: int
+    """
+    cmd = 'abipkgdiff --d1 {0} --d2 {1} {2} {3}'.format(
+        pkg_info1.debuginfo_package.downloaded_file,
+        pkg_info2.debuginfo_package.downloaded_file,
+        pkg_info1.package.downloaded_file,
+        pkg_info2.package.downloaded_file)
+
+    if global_config.dry_run:
+        print 'DRY-RUN:', cmd
+        return
+
+    logger.debug('Run: %s', cmd)
+
+    print 'Comparing the ABI of binaries between {} and {}:'.format(
+        pkg_info1.package.filename, pkg_info2.package.filename)
+    print
+
+    proc = subprocess.Popen(shlex.split(cmd))
+    return proc.wait()
+
+
+def magic_construct(rpms):
+    """Construct RPMs into a magic structure
+
+    Convert list of
+
+    foo-1.0-1.fc22.i686
+    foo-debuginfo-1.0-1.fc22.i686
+    foo-devel-1.0-1.fc22.i686
+
+    to list of
+
+    (foo-1.0-1.fc22.i686, foo-debuginfo-1.0-1.fc22.i686)
+    (foo-devel-1.0-1.fc22.i686, foo-debuginfo-1.0-1.fc22.i686)
+
+    :param rpms: a sequence of RPM packages.
+    :type rpms: list or tuple
+    :return: list of two-element tuple where the first element is a RPM package
+    and the second one is the debuginfo package.
+    :rtype: list
+    """
+    debuginfo = None
+    packages = []
+    for rpm in rpms:
+        if rpm.is_debuginfo:
+            debuginfo = rpm
+        else:
+            packages.append(rpm)
+    return [PkgInfo(package, debuginfo) for package in packages]
+
+
+@log_call
+def run_abipkgdiff(pkg1_infos, pkg2_infos):
+    """Run abipkgdiff
+
+    If one of the executions finds ABI differences, the return code is the
+    return code from abipkgdiff.
+
+    :param dict pkg1_infos: a mapping from arch to list of RPMs
+    :return: exit code of the last non-zero returned from underlying abipkgdiff
+    :rtype: number
+    """
+    arches = pkg1_infos.keys()
+    arches.sort()
+
+    return_code = 0
+
+    for arch in arches:
+        pkg_infos = magic_construct(pkg1_infos[arch])
+
+        for pkg_info in pkg_infos:
+            rpms = pkg2_infos[arch]
+
+            package = [rpm for rpm in rpms
+                       if rpm.name == pkg_info.package.name][0]
+            debuginfo = [rpm for rpm in rpms
+                         if rpm.name == pkg_info.debuginfo_package.name][0]
+
+            ret = abipkgdiff(pkg_info,
+                             PkgInfo(package=package,
+                                     debuginfo_package=debuginfo))
+            if ret > 0:
+                return_code = ret
+
+    return return_code
+
+
+@log_call
+def diff_local_rpm_with_latest_rpm_from_koji():
+    """Diff against local rpm and remove latest rpm
+
+    This operation handles a local rpm and debuginfo rpm and remote ones
+    located in remote Koji server, that has specific distro specificed by
+    argument --from.
+
+    1/ Suppose the packager has just locally built a package named
+    foo-3.0.fc24.rpm. To compare the ABI of this locally build package with the
+    latest stable package from Fedora 23, one would do:
+
+    fedabipkgdiff --from fc23 ./foo-3.0.fc24.rpm
+    """
+
+    from_distro = global_config.from_distro
+    if not is_distro_valid(from_distro):
+        raise InvalidDistroError('Invalid distro {0}'.format(from_distro))
+
+    local_rpm_file = global_config.NVR[0]
+    if not os.path.exists(local_rpm_file):
+        raise ValueError('{0} does not exist.'.format(local_rpm_file))
+
+    local_rpm = LocalRPM(local_rpm_file)
+    local_debuginfo = local_rpm.find_debuginfo()
+    if local_debuginfo is None:
+        raise ValueError(
+            'debuginfo rpm {0} does not exist.'.format(local_debuginfo))
+
+    rpms = session.get_latest_built_rpms(local_rpm.name,
+                                         from_distro,
+                                         arches=local_rpm.arch)
+    download_rpms(rpms)
+    pkg_infos = make_rpms_usable_for_abipkgdiff(rpms)
+
+    rpms = pkg_infos.values()[0]
+    package, debuginfo = sorted(rpms, key=lambda rpm: rpm.name)
+    return abipkgdiff(PkgInfo(package, debuginfo),
+                      PkgInfo(local_rpm, local_debuginfo))
+
+
+@log_call
+def make_rpms_usable_for_abipkgdiff(rpms):
+    """Prepare package information structure for running abipkgdiff
+
+    So far, RPMs input to this method are queried from Koji and abipkgdiff will
+    run against these RPMs. For convenience, these RPMs should be restructured
+    into a mapping so that subsequent operations could easily find RPMs from
+    arch.
+
+    For example, input RPMs are
+
+    [RPM(arch='x86_64', name='httpd'),
+     RPM(arch='i686', name='httpd'),
+     RPM(arch='x86_64', name='httpd-devel'),
+     RPM(arch='i686', name='http-debuginfo'),
+     RPM(arch='x86_64', name='httpd-debuginfo'),
+     ]
+
+    it is converted into mapping
+
+    {
+        'x86_64': [RPM(arch='x86_64', name='httpd'),
+                   RPM(arch='x86_64', name='httpd-devel'),
+                   RPM(arch='x86_64', name='httpd-debuginfo')],
+        'i686': [RPM(arch='i686', name='httpd'),
+                 RPM(arch='i686', name='http-debuginfo')],
+    }
+
+    The order RPMs in the mapping is unpredictable. So, if they must be in a
+    particular order, caller is responsible for this.
+
+    :param list rpms: a list of RPMs
+    :return: a mapping from an arch to corresponding list of RPMs
+    :rtype: dict
+    """
+    result = {}
+    rpms_iter = groupby(sorted(rpms, key=lambda rpm: rpm.arch),
+                        key=lambda item: item.arch)
+    for arch, rpms in rpms_iter:
+        result[arch] = list(rpms)
+    return result
+
+
+@log_call
+def diff_latest_rpms_based_on_distros():
+    """abipkgdiff rpms based on two distros
+
+    2/ Suppose the packager wants to see how the ABIs of the package foo
+    evolved between fedora 19 and fedora 22. She would thus type the command:
+
+    fedabipkgdiff --from fc19 --to fc22 foo
+    """
+
+    from_distro = global_config.from_distro
+    to_distro = global_config.to_distro
+
+    if not is_distro_valid(from_distro):
+        raise InvalidDistroError('Invalid distro {0}'.format(from_distro))
+
+    if not is_distro_valid(to_distro):
+        raise InvalidDistroError('Invalid distro {0}'.format(distro))
+
+    package_name = global_config.NVR[0]
+
+    rpms = session.get_latest_built_rpms(package_name,
+                                         distro=global_config.from_distro)
+    download_rpms(rpms)
+    pkg1_infos = make_rpms_usable_for_abipkgdiff(rpms)
+
+    rpms = session.get_latest_built_rpms(package_name,
+                                         distro=global_config.to_distro)
+    download_rpms(rpms)
+    pkg2_infos = make_rpms_usable_for_abipkgdiff(rpms)
+
+    return run_abipkgdiff(pkg1_infos, pkg2_infos)
+
+
+@log_call
+def diff_two_nvras_from_koji():
+    """Diff two nvras from koji
+
+    The arch probably omits, that means febabipkgdiff will diff all arches. If
+    specificed, the specific arch will be handled.
+
+    3/ Suppose the packager wants to compare the ABI of two packages designated
+    by their name and version. She would issue a command like this:
+
+    fedabipkgdiff foo-1.0.fc19 foo-3.0.fc24
+    fedabipkgdiff foo-1.0.fc19.i686 foo-1.0.fc24.i686
+    """
+    left_rpm = koji.parse_NVRA(global_config.NVR[0])
+    right_rpm = koji.parse_NVRA(global_config.NVR[1])
+
+    if is_distro_valid(left_rpm['arch']) and \
+            is_distro_valid(right_rpm['arch']):
+        nvr = koji.parse_NVR(global_config.NVR[0])
+        params1 = (nvr['name'], nvr['version'], nvr['release'], None)
+
+        nvr = koji.parse_NVR(global_config.NVR[1])
+        params2 = (nvr['name'], nvr['version'], nvr['release'], None)
+    else:
+        params1 = (left_rpm['name'],
+                   left_rpm['version'],
+                   left_rpm['release'],
+                   left_rpm['arch'])
+        params2 = (right_rpm['name'],
+                   right_rpm['version'],
+                   right_rpm['release'],
+                   right_rpm['arch'])
+
+    build_id = session.get_rpm_build_id(*params1)
+    rpms = session.select_rpms_from_a_build(
+        build_id, params1[0], arches=params1[3],
+        select_subpackages=global_config.check_all_subpackages)
+    download_rpms(rpms)
+    pkg1_infos = make_rpms_usable_for_abipkgdiff(rpms)
+
+    build_id = session.get_rpm_build_id(*params2)
+    rpms = session.select_rpms_from_a_build(
+        build_id, params2[0], arches=params2[3],
+        select_subpackages=global_config.check_all_subpackages)
+    download_rpms(rpms)
+    pkg2_infos = make_rpms_usable_for_abipkgdiff(rpms)
+
+    return run_abipkgdiff(pkg1_infos, pkg2_infos)
+
+
+def build_commandline_args_parser():
+    parser = argparse.ArgumentParser(
+        description='Run abipkgdiff against RPM packages from koji')
+
+    parser.add_argument(
+        'NVR',
+        nargs='*',
+        help='RPM package N-V-R, N-V-R-A, N, or a local RPM '
+             'file name with relative or absolute path.')
+    parser.add_argument(
+        '--dry-run',
+        required=False,
+        dest='dry_run',
+        action='store_true',
+        help='Don\'t actually run abipkgdiff. The commands that should be '
+             'run will be sent to stdout.')
+    parser.add_argument(
+        '--from',
+        required=False,
+        metavar='DISTRO',
+        dest='from_distro',
+        help='baseline Fedora distro, for example, fc23')
+    parser.add_argument(
+        '--to',
+        required=False,
+        metavar='DISTRO',
+        dest='to_distro',
+        help='which Fedora distro to compare, for example, fc24')
+    parser.add_argument(
+        '-a',
+        '--all-subpackages',
+        required=False,
+        action='store_true',
+        dest='check_all_subpackages',
+        help='Check all subpackages instead of only the package specificed in '
+             'command line.')
+    parser.add_argument(
+        '--debug',
+        required=False,
+        action='store_true',
+        dest='debug',
+        help='show debug output')
+    parser.add_argument(
+        '--traceback',
+        required=False,
+        action='store_true',
+        dest='show_traceback',
+        help='show traceback when there is an exception thrown.')
+    parser.add_argument(
+        '--server',
+        required=False,
+        metavar='URL',
+        dest='koji_server',
+        default=DEFAULT_KOJI_SERVER,
+        help='URL of koji XMLRPC service. Default is {0}'.format(
+            DEFAULT_KOJI_SERVER))
+    parser.add_argument(
+        '--topdir',
+        required=False,
+        metavar='URL',
+        dest='koji_topdir',
+        default=DEFAULT_KOJI_TOPDIR,
+        help='URL for RPM files access')
+
+    return parser
+
+
+def main():
+    parser = build_commandline_args_parser()
+
+    args = parser.parse_args()
+
+    global global_config
+    global_config = args
+
+    global pathinfo
+    pathinfo = koji.PathInfo(topdir=global_config.koji_topdir)
+
+    global session
+    session = get_session()
+
+    if global_config.debug:
+        logger.setLevel(logging.DEBUG)
+
+    logger.debug(args)
+
+    if global_config.from_distro and global_config.to_distro is None and \
+            global_config.NVR:
+        returncode = diff_local_rpm_with_latest_rpm_from_koji()
+
+    elif global_config.from_distro and global_config.to_distro and \
+            global_config.NVR:
+        returncode = diff_latest_rpms_based_on_distros()
+
+    elif global_config.from_distro is None and \
+            global_config.to_distro is None and len(global_config.NVR) > 1:
+        returncode = diff_two_nvras_from_koji()
+
+    else:
+        print >>sys.stderr, 'Unknown arguments. Please refer to --help.'
+        returncode = 1
+
+    return returncode
+
+
+if __name__ == '__main__':
+    try:
+        main()
+    except KeyboardInterrupt:
+        if global_config.debug:
+            logger.debug('Terminate by user')
+        else:
+            print >>sys.stderr, 'Terminate by user'
+        if global_config.show_traceback:
+            raise
+        else:
+            sys.exit(2)
+    except Exception as e:
+        if global_config.debug:
+            logger.debug(str(e))
+        else:
+            print >>sys.stderr, str(e)
+        if global_config.show_traceback:
+            raise
+        else:
+            sys.exit(1)
-- 
2.5.5


  reply	other threads:[~2016-04-29 14:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-01  0:00 Chenxiong Qi
2016-01-01  0:00 ` Chenxiong Qi
2016-01-01  0:00   ` Dodji Seketeli
2016-01-01  0:00     ` Fix adding fedabipkgdiff to source distribution tarball Dodji Seketeli
2016-01-01  0:00     ` a new tool fedabipkgdiff Dodji Seketeli
2016-01-01  0:00 ` Chenxiong Qi
2016-01-01  0:00   ` Dodji Seketeli
2016-01-01  0:00     ` Chenxiong Qi [this message]
2016-01-01  0:00       ` Dodji Seketeli
2016-01-01  0:00         ` Chenxiong Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1620325890.3371217.1461940400376.JavaMail.yahoo@mail.yahoo.com \
    --to=qcxhome@yahoo.com \
    --cc=cqi@redhat.com \
    --cc=dodji@redhat.com \
    --cc=libabigail@sourceware.org \
    --cc=qcxhome@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).