From 424d9cb496651baada9cb68002e5c7997afc1487 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:26 +0000 Subject: [PATCH 1/11] don't spawn process for rpmdiff Speed improvement by using bundled rpmdiff library instead of spawning special process. Related: https://pagure.io/koji/issue/715 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 303ab9a..3ddb1b5 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -56,6 +56,8 @@ import koji.policy import koji.xmlrpcplus from koji.context import context from koji.util import dslice +import imp +_rpmdiff = imp.load_source('_rpmdiff', '/usr/libexec/koji-hub/rpmdiff') from koji.util import md5_constructor from koji.util import multi_fnmatch from koji.util import safer_move @@ -8359,21 +8361,13 @@ def rpmdiff(basepath, rpmlist): # ignore differences in file size, md5sum, and mtime # (files may have been generated at build time and contain # embedded dates or other insignificant differences) - args = ['/usr/libexec/koji-hub/rpmdiff', - '--ignore', 'S', '--ignore', '5', - '--ignore', 'T', '--ignore', 'N', - os.path.join(basepath, first_rpm), - os.path.join(basepath, other_rpm)] - proc = subprocess.Popen(args, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - close_fds=True) - output = proc.communicate()[0] - status = proc.wait() - if os.WIFSIGNALED(status) or \ - (os.WEXITSTATUS(status) != 0): + d = _rpmdiff.Rpmdiff(os.path.join(basepath, first_rpm), + os.path.join(basepath, other_rpm), ignore='S5TN') + if d.differs(): raise koji.BuildError( 'The following noarch package built differently on different architectures: %s\n' - 'rpmdiff output was:\n%s' % (os.path.basename(first_rpm), output)) + 'rpmdiff output was:\n%s' % (os.path.basename(first_rpm), d.textdiff())) + def importImageInternal(task_id, build_id, imgdata): """ diff --git a/tests/test_hub/test_rpmdiff.py b/tests/test_hub/test_rpmdiff.py index 277902f..4cbabb3 100644 --- a/tests/test_hub/test_rpmdiff.py +++ b/tests/test_hub/test_rpmdiff.py @@ -8,30 +8,30 @@ import kojihub class TestRPMDiff(unittest.TestCase): - @mock.patch('kojihub.subprocess') - def test_rpmdiff_empty_invocation(self, subprocess): - process = mock.MagicMock() - subprocess.Popen.return_value = process + @mock.patch('kojihub._rpmdiff.Rpmdiff') + def test_rpmdiff_empty_invocation(self, Rpmdiff): kojihub.rpmdiff('basepath', []) - self.assertEquals(len(subprocess.Popen.mock_calls), 0) + Rpmdiff.assert_not_called() kojihub.rpmdiff('basepath', ['foo']) - self.assertEquals(len(subprocess.Popen.mock_calls), 0) + Rpmdiff.assert_not_called() - @mock.patch('kojihub.subprocess') - def test_rpmdiff_simple_success(self, subprocess): - process = mock.MagicMock() - subprocess.Popen.return_value = process - process.wait.return_value = 0 - kojihub.rpmdiff('basepath', ['foo', 'bar']) - self.assertEquals(len(subprocess.Popen.call_args_list), 1) + @mock.patch('kojihub._rpmdiff.Rpmdiff') + def test_rpmdiff_simple_success(self, Rpmdiff): + d = mock.MagicMock() + d.differs.return_value = False + Rpmdiff.return_value = d + self.assertFalse(kojihub.rpmdiff('basepath', ['foo', 'bar'])) + Rpmdiff.assert_called_once_with('basepath/foo', 'basepath/bar', ignore='S5TN') - @mock.patch('kojihub.subprocess') - def test_rpmdiff_simple_failure(self, subprocess): - process = mock.MagicMock() - subprocess.Popen.return_value = process - process.wait.return_value = 1 + @mock.patch('kojihub._rpmdiff.Rpmdiff') + def test_rpmdiff_simple_failure(self, Rpmdiff): + d = mock.MagicMock() + d.differs.return_value = True + Rpmdiff.return_value = d with self.assertRaises(koji.BuildError): kojihub.rpmdiff('basepath', ['foo', 'bar']) + Rpmdiff.assert_called_once_with('basepath/foo', 'basepath/bar', ignore='S5TN') + d.textdiff.assert_called_once_with() class TestCheckNoarchRpms(unittest.TestCase): @mock.patch('kojihub.rpmdiff') From bc3629393256e3ed350e7556b945aecf66ef7a7d Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:26 +0000 Subject: [PATCH 2/11] cache rpmdiff results --- diff --git a/builder/kojid b/builder/kojid index a058689..f83d2be 100755 --- a/builder/kojid +++ b/builder/kojid @@ -64,6 +64,9 @@ from yum import repoMDObject import yum.packages import yum.Errors +import imp +_rpmdiff = imp.load_source('_rpmdiff', '/usr/libexec/koji-hub/rpmdiff') + #imports for LiveCD, LiveMedia, and Appliance handler image_enabled = False try: @@ -434,7 +437,7 @@ class BuildRoot(object): if workdir: outfile = os.path.join(workdir, mocklog) flags = os.O_CREAT | os.O_WRONLY | os.O_APPEND - fd = os.open(outfile, flags, 0666) + fd = os.open(outfile, flags, 0o666) os.dup2(fd, 1) os.dup2(fd, 2) if os.getuid() == 0 and hasattr(self.options,"mockuser"): @@ -1247,6 +1250,21 @@ class BuildArchTask(BaseBuildTask): f = os.path.join(broot.workdir, mocklog) if os.path.exists(f): log_files.append(os.path.basename(f)) + + # for noarch rpms compute rpmdiff hash + rpmdiff_hash = {self.id: {}} + for rpmf in rpm_files: + if rpmf.endswith('.noarch.rpm'): + fpath = os.path.join(resultdir, rpmf) + d = _rpmdiff.Rpmdiff(fpath, fpath, ignore='S5TN') + rpmdiff_hash[self.id][rpmf] = d.kojihash() + if rpmdiff_hash[self.id]: + log_name = 'noarch_rpmdiff.json' + noarch_hash_path = os.path.join(broot.workdir, log_name) + with open(noarch_hash_path, 'wt') as f: + json.dump(rpmdiff_hash, f, indent=2, sort_keys=True) + log_files.append(log_name) + self.logger.debug("rpms: %r" % rpm_files) self.logger.debug("srpms: %r" % srpm_files) self.logger.debug("logs: %r" % log_files) @@ -1277,6 +1295,8 @@ class BuildArchTask(BaseBuildTask): else: ret['srpms'] = [] ret['logs'] = [ "%s/%s" % (uploadpath,f) for f in log_files ] + if rpmdiff_hash[self.id]: + self.uploadFile(noarch_hash_path) ret['brootid'] = broot.id diff --git a/hub/kojihub.py b/hub/kojihub.py index 3ddb1b5..d22524c 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -35,7 +35,6 @@ import os import re import shutil import stat -import subprocess import sys import tarfile import tempfile @@ -5005,7 +5004,7 @@ def recycle_build(old, data): old=old['state'], new=data['state'], info=buildinfo) -def check_noarch_rpms(basepath, rpms): +def check_noarch_rpms(basepath, rpms, logs=None): """ If rpms contains any noarch rpms with identical names, run rpmdiff against the duplicate rpms. @@ -5014,6 +5013,8 @@ def check_noarch_rpms(basepath, rpms): """ result = [] noarch_rpms = {} + if logs is None: + logs = {} for relpath in rpms: if relpath.endswith('.noarch.rpm'): filename = os.path.basename(relpath) @@ -5027,8 +5028,18 @@ def check_noarch_rpms(basepath, rpms): else: result.append(relpath) + hashes = {} + for arch in logs: + for log in logs[arch]: + if os.path.basename(log) == 'noarch_rpmdiff.json': + task_hash = json.load(open(os.path.join(basepath, log), 'rt')) + for task_id in task_hash: + hashes[task_id] = task_hash[task_id] + for noarch_list in noarch_rpms.values(): - rpmdiff(basepath, noarch_list) + if len(noarch_list) < 2: + continue + rpmdiff(basepath, noarch_list, hashes=hashes) return result @@ -5054,7 +5065,7 @@ def import_build(srpm, rpms, brmap=None, task_id=None, build_id=None, logs=None) if not os.path.exists(fn): raise koji.GenericError("no such file: %s" % fn) - rpms = check_noarch_rpms(uploadpath, rpms) + rpms = check_noarch_rpms(uploadpath, rpms, logs=logs) #verify buildroot ids from brmap found = {} @@ -8352,12 +8363,20 @@ def assert_policy(name, data, default='deny'): """ check_policy(name, data, default=default, strict=True) -def rpmdiff(basepath, rpmlist): +def rpmdiff(basepath, rpmlist, hashes): "Diff the first rpm in the list against the rest of the rpms." if len(rpmlist) < 2: return first_rpm = rpmlist[0] + task_id = first_rpm.split('/')[1] + first_hash = hashes.get(task_id, {}).get(os.path.basename(first_rpm), False) for other_rpm in rpmlist[1:]: + if first_hash: + task_id = other_rpm.split('/')[1] + other_hash = hashes[task_id][os.path.basename(other_rpm)] + if first_hash == other_hash: + logger.debug("Skipping noarch rpmdiff for %s vs %s" % (first_rpm, other_rpm)) + continue # ignore differences in file size, md5sum, and mtime # (files may have been generated at build time and contain # embedded dates or other insignificant differences) @@ -11663,7 +11682,7 @@ class HostExports(object): if not os.path.exists(fn): raise koji.GenericError("no such file: %s" % fn) - rpms = check_noarch_rpms(uploadpath, rpms) + rpms = check_noarch_rpms(uploadpath, rpms, logs=logs) #figure out storage location # //task_ diff --git a/hub/rpmdiff b/hub/rpmdiff index 7ef9562..136be67 100755 --- a/hub/rpmdiff +++ b/hub/rpmdiff @@ -20,6 +20,8 @@ # This library and program is heavily based on rpmdiff from the rpmlint package # It was modified to be used as standalone library for the Koji project. +import hashlib +import json import rpm import os import itertools @@ -77,6 +79,8 @@ class Rpmdiff: def __init__(self, old, new, ignore=None): self.result = [] self.ignore = ignore + self.old_data = { 'tags': {} } + self.new_data = { 'tags': {} } if self.ignore is None: self.ignore = [] @@ -94,6 +98,8 @@ class Rpmdiff: for tag in self.TAGS: old_tag = old[tag] new_tag = new[tag] + self.old_data['tags']['tag'] = old[tag] + self.new_data['tags']['tag'] = new[tag] if old_tag != new_tag: tagname = rpm.tagnames[tag] if old_tag == None: @@ -114,6 +120,8 @@ class Rpmdiff: files = list(set(itertools.chain(old_files_dict.iterkeys(), new_files_dict.iterkeys()))) files.sort() + self.old_data['files'] = old_files_dict + self.new_data['files'] = new_files_dict for f in files: diff = 0 @@ -177,6 +185,8 @@ class Rpmdiff: o = zip(old[name], oldflags, old[name[:-1]+'VERSION']) n = zip(new[name], newflags, new[name[:-1]+'VERSION']) + self.old_data[name] = sorted(o) + self.new_data[name] = sorted(n) if name == 'PROVIDES': # filter our self provide oldNV = (old['name'], rpm.RPMSENSE_EQUAL, @@ -211,6 +221,14 @@ class Rpmdiff: result[filedata[0]] = filedata[1:] return result + def kojihash(self, new=False): + """return hashed data for use in koji""" + if new: + s = json.dumps(self.new_data, sort_keys=True) + else: + s = json.dumps(self.old_data, sort_keys=True) + return hashlib.sha256(s).hexdigest() + def _usage(exit=1): print("Usage: %s [] " % sys.argv[0]) print("Options:") @@ -225,7 +243,7 @@ def main(): ignore_tags = [] try: opts, args = getopt.getopt(sys.argv[1:], "hi:", ["help", "ignore="]) - except getopt.GetoptError, e: + except getopt.GetoptError as e: print("Error: %s" % e) _usage() From ce253acb9f2fa292ac9952891a1de545df9cc071 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 3/11] move rpmdiff to koji lib --- diff --git a/builder/kojid b/builder/kojid index f83d2be..f9f46d2 100755 --- a/builder/kojid +++ b/builder/kojid @@ -27,6 +27,7 @@ except ImportError: # pragma: no cover krbV = None import koji import koji.plugin +import koji.rpmdiff import koji.util import koji.tasks import glob @@ -64,9 +65,6 @@ from yum import repoMDObject import yum.packages import yum.Errors -import imp -_rpmdiff = imp.load_source('_rpmdiff', '/usr/libexec/koji-hub/rpmdiff') - #imports for LiveCD, LiveMedia, and Appliance handler image_enabled = False try: @@ -1256,7 +1254,7 @@ class BuildArchTask(BaseBuildTask): for rpmf in rpm_files: if rpmf.endswith('.noarch.rpm'): fpath = os.path.join(resultdir, rpmf) - d = _rpmdiff.Rpmdiff(fpath, fpath, ignore='S5TN') + d = koji.rpmdiff.Rpmdiff(fpath, fpath, ignore='S5TN') rpmdiff_hash[self.id][rpmf] = d.kojihash() if rpmdiff_hash[self.id]: log_name = 'noarch_rpmdiff.json' diff --git a/hub/Makefile b/hub/Makefile index 72b475f..f95a173 100644 --- a/hub/Makefile +++ b/hub/Makefile @@ -1,6 +1,5 @@ PYTHON=python PACKAGE = $(shell basename `pwd`) -LIBEXECFILES = rpmdiff PYFILES = $(wildcard *.py) PYVER := $(shell $(PYTHON) -c 'import sys; print("%.3s" %(sys.version))') PYSYSDIR := $(shell $(PYTHON) -c 'import sys; print(sys.prefix)') @@ -24,7 +23,6 @@ install: fi mkdir -p $(DESTDIR)/usr/libexec/koji-hub - install -p -m 755 $(LIBEXECFILES) $(DESTDIR)/usr/libexec/koji-hub mkdir -p $(DESTDIR)/etc/httpd/conf.d install -p -m 644 httpd.conf $(DESTDIR)/etc/httpd/conf.d/kojihub.conf diff --git a/hub/kojihub.py b/hub/kojihub.py index d22524c..0a41788 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -24,6 +24,7 @@ import base64 import calendar +import koji.rpmdiff import datetime import errno import fcntl @@ -55,13 +56,10 @@ import koji.policy import koji.xmlrpcplus from koji.context import context from koji.util import dslice -import imp -_rpmdiff = imp.load_source('_rpmdiff', '/usr/libexec/koji-hub/rpmdiff') from koji.util import md5_constructor from koji.util import multi_fnmatch from koji.util import safer_move from koji.util import sha1_constructor - logger = logging.getLogger('koji.hub') def log_error(msg): @@ -8380,7 +8378,7 @@ def rpmdiff(basepath, rpmlist, hashes): # ignore differences in file size, md5sum, and mtime # (files may have been generated at build time and contain # embedded dates or other insignificant differences) - d = _rpmdiff.Rpmdiff(os.path.join(basepath, first_rpm), + d = koji.rpmdiff.Rpmdiff(os.path.join(basepath, first_rpm), os.path.join(basepath, other_rpm), ignore='S5TN') if d.differs(): raise koji.BuildError( diff --git a/hub/rpmdiff b/hub/rpmdiff deleted file mode 100755 index 136be67..0000000 --- a/hub/rpmdiff +++ /dev/null @@ -1,266 +0,0 @@ -#!/usr/bin/python -# -# Copyright (C) 2006 Mandriva; 2009-2014 Red Hat, Inc. -# Authors: Frederic Lepied, Florian Festi -# -# 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 2 of the License, 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 Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -# -# This library and program is heavily based on rpmdiff from the rpmlint package -# It was modified to be used as standalone library for the Koji project. - -import hashlib -import json -import rpm -import os -import itertools - -import sys, getopt - - -class Rpmdiff: - - # constants - - TAGS = ( rpm.RPMTAG_NAME, rpm.RPMTAG_SUMMARY, - rpm.RPMTAG_DESCRIPTION, rpm.RPMTAG_GROUP, - rpm.RPMTAG_LICENSE, rpm.RPMTAG_URL, - rpm.RPMTAG_PREIN, rpm.RPMTAG_POSTIN, - rpm.RPMTAG_PREUN, rpm.RPMTAG_POSTUN) - - PRCO = ( 'REQUIRES', 'PROVIDES', 'CONFLICTS', 'OBSOLETES') - - #{fname : (size, mode, mtime, flags, dev, inode, - # nlink, state, vflags, user, group, digest)} - __FILEIDX = [ ['S', 0], - ['M', 1], - ['5', 11], - ['D', 4], - ['N', 6], - ['L', 7], - ['V', 8], - ['U', 9], - ['G', 10], - ['F', 3], - ['T', 2] ] - - try: - if rpm.RPMSENSE_SCRIPT_PRE: - PREREQ_FLAG=rpm.RPMSENSE_PREREQ|rpm.RPMSENSE_SCRIPT_PRE|\ - rpm.RPMSENSE_SCRIPT_POST|rpm.RPMSENSE_SCRIPT_PREUN|\ - rpm.RPMSENSE_SCRIPT_POSTUN - except AttributeError: - try: - PREREQ_FLAG=rpm.RPMSENSE_PREREQ - except: - #(proyvind): This seems ugly, but then again so does - # this whole check as well. - PREREQ_FLAG=False - - DEPFORMAT = '%-12s%s %s %s %s' - FORMAT = '%-12s%s' - - ADDED = 'added' - REMOVED = 'removed' - - # code starts here - - def __init__(self, old, new, ignore=None): - self.result = [] - self.ignore = ignore - self.old_data = { 'tags': {} } - self.new_data = { 'tags': {} } - if self.ignore is None: - self.ignore = [] - - FILEIDX = self.__FILEIDX - for tag in self.ignore: - for entry in FILEIDX: - if tag == entry[0]: - entry[1] = None - break - - old = self.__load_pkg(old) - new = self.__load_pkg(new) - - # Compare single tags - for tag in self.TAGS: - old_tag = old[tag] - new_tag = new[tag] - self.old_data['tags']['tag'] = old[tag] - self.new_data['tags']['tag'] = new[tag] - if old_tag != new_tag: - tagname = rpm.tagnames[tag] - if old_tag == None: - self.__add(self.FORMAT, (self.ADDED, tagname)) - elif new_tag == None: - self.__add(self.FORMAT, (self.REMOVED, tagname)) - else: - self.__add(self.FORMAT, ('S.5........', tagname)) - - # compare Provides, Requires, ... - for tag in self.PRCO: - self.__comparePRCOs(old, new, tag) - - # compare the files - - old_files_dict = self.__fileIteratorToDict(old.fiFromHeader()) - new_files_dict = self.__fileIteratorToDict(new.fiFromHeader()) - files = list(set(itertools.chain(old_files_dict.iterkeys(), - new_files_dict.iterkeys()))) - files.sort() - self.old_data['files'] = old_files_dict - self.new_data['files'] = new_files_dict - - for f in files: - diff = 0 - - old_file = old_files_dict.get(f) - new_file = new_files_dict.get(f) - - if not old_file: - self.__add(self.FORMAT, (self.ADDED, f)) - elif not new_file: - self.__add(self.FORMAT, (self.REMOVED, f)) - else: - format = '' - for entry in FILEIDX: - if entry[1] != None and \ - old_file[entry[1]] != new_file[entry[1]]: - format = format + entry[0] - diff = 1 - else: - format = format + '.' - if diff: - self.__add(self.FORMAT, (format, f)) - - # return a report of the differences - def textdiff(self): - return '\n'.join((format % data for format, data in self.result)) - - # do the two rpms differ - def differs(self): - return bool(self.result) - - # add one differing item - def __add(self, format, data): - self.result.append((format, data)) - - # load a package from a file or from the installed ones - def __load_pkg(self, filename): - ts = rpm.ts() - f = os.open(filename, os.O_RDONLY) - hdr = ts.hdrFromFdno(f) - os.close(f) - return hdr - - # output the right string according to RPMSENSE_* const - def sense2str(self, sense): - s = "" - for tag, char in ((rpm.RPMSENSE_LESS, "<"), - (rpm.RPMSENSE_GREATER, ">"), - (rpm.RPMSENSE_EQUAL, "=")): - if sense & tag: - s += char - return s - - # compare Provides, Requires, Conflicts, Obsoletes - def __comparePRCOs(self, old, new, name): - oldflags = old[name[:-1]+'FLAGS'] - newflags = new[name[:-1]+'FLAGS'] - # fix buggy rpm binding not returning list for single entries - if not isinstance(oldflags, list): oldflags = [ oldflags ] - if not isinstance(newflags, list): newflags = [ newflags ] - - o = zip(old[name], oldflags, old[name[:-1]+'VERSION']) - n = zip(new[name], newflags, new[name[:-1]+'VERSION']) - self.old_data[name] = sorted(o) - self.new_data[name] = sorted(n) - - if name == 'PROVIDES': # filter our self provide - oldNV = (old['name'], rpm.RPMSENSE_EQUAL, - "%s-%s" % (old['version'], old['release'])) - newNV = (new['name'], rpm.RPMSENSE_EQUAL, - "%s-%s" % (new['version'], new['release'])) - o = [entry for entry in o if entry != oldNV] - n = [entry for entry in n if entry != newNV] - - for oldentry in o: - if not oldentry in n: - if name == 'REQUIRES' and oldentry[1] & self.PREREQ_FLAG: - tagname = 'PREREQ' - else: - tagname = name - self.__add(self.DEPFORMAT, - (self.REMOVED, tagname, oldentry[0], - self.sense2str(oldentry[1]), oldentry[2])) - for newentry in n: - if not newentry in o: - if name == 'REQUIRES' and newentry[1] & self.PREREQ_FLAG: - tagname = 'PREREQ' - else: - tagname = name - self.__add(self.DEPFORMAT, - (self.ADDED, tagname, newentry[0], - self.sense2str(newentry[1]), newentry[2])) - - def __fileIteratorToDict(self, fi): - result = {} - for filedata in fi: - result[filedata[0]] = filedata[1:] - return result - - def kojihash(self, new=False): - """return hashed data for use in koji""" - if new: - s = json.dumps(self.new_data, sort_keys=True) - else: - s = json.dumps(self.old_data, sort_keys=True) - return hashlib.sha256(s).hexdigest() - -def _usage(exit=1): - print("Usage: %s [] " % sys.argv[0]) - print("Options:") - print(" -h, --help Output this message and exit") - print(" -i, --ignore Tag to ignore when calculating differences") - print(" (may be used multiple times)") - print(" Valid values are: SM5DNLVUGFT") - sys.exit(exit) - -def main(): - - ignore_tags = [] - try: - opts, args = getopt.getopt(sys.argv[1:], "hi:", ["help", "ignore="]) - except getopt.GetoptError as e: - print("Error: %s" % e) - _usage() - - for option, argument in opts: - if option in ("-h", "--help"): - _usage(0) - if option in ("-i", "--ignore"): - ignore_tags.append(argument) - - if len(args) != 2: - _usage() - - d = Rpmdiff(args[0], args[1], ignore=ignore_tags) - print(d.textdiff()) - sys.exit(int(d.differs())) - -if __name__ == '__main__': - main() - -# rpmdiff ends here diff --git a/koji.spec b/koji.spec index 3665a1f..ceb5e7f 100644 --- a/koji.spec +++ b/koji.spec @@ -321,7 +321,6 @@ rm -rf $RPM_BUILD_ROOT %defattr(-,root,root) %{_datadir}/koji-hub %dir %{_libexecdir}/koji-hub -%{_libexecdir}/koji-hub/rpmdiff %config(noreplace) /etc/httpd/conf.d/kojihub.conf %dir /etc/koji-hub %config(noreplace) /etc/koji-hub/hub.conf diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py new file mode 100755 index 0000000..136be67 --- /dev/null +++ b/koji/rpmdiff.py @@ -0,0 +1,266 @@ +#!/usr/bin/python +# +# Copyright (C) 2006 Mandriva; 2009-2014 Red Hat, Inc. +# Authors: Frederic Lepied, Florian Festi +# +# 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 2 of the License, 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 Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +# +# This library and program is heavily based on rpmdiff from the rpmlint package +# It was modified to be used as standalone library for the Koji project. + +import hashlib +import json +import rpm +import os +import itertools + +import sys, getopt + + +class Rpmdiff: + + # constants + + TAGS = ( rpm.RPMTAG_NAME, rpm.RPMTAG_SUMMARY, + rpm.RPMTAG_DESCRIPTION, rpm.RPMTAG_GROUP, + rpm.RPMTAG_LICENSE, rpm.RPMTAG_URL, + rpm.RPMTAG_PREIN, rpm.RPMTAG_POSTIN, + rpm.RPMTAG_PREUN, rpm.RPMTAG_POSTUN) + + PRCO = ( 'REQUIRES', 'PROVIDES', 'CONFLICTS', 'OBSOLETES') + + #{fname : (size, mode, mtime, flags, dev, inode, + # nlink, state, vflags, user, group, digest)} + __FILEIDX = [ ['S', 0], + ['M', 1], + ['5', 11], + ['D', 4], + ['N', 6], + ['L', 7], + ['V', 8], + ['U', 9], + ['G', 10], + ['F', 3], + ['T', 2] ] + + try: + if rpm.RPMSENSE_SCRIPT_PRE: + PREREQ_FLAG=rpm.RPMSENSE_PREREQ|rpm.RPMSENSE_SCRIPT_PRE|\ + rpm.RPMSENSE_SCRIPT_POST|rpm.RPMSENSE_SCRIPT_PREUN|\ + rpm.RPMSENSE_SCRIPT_POSTUN + except AttributeError: + try: + PREREQ_FLAG=rpm.RPMSENSE_PREREQ + except: + #(proyvind): This seems ugly, but then again so does + # this whole check as well. + PREREQ_FLAG=False + + DEPFORMAT = '%-12s%s %s %s %s' + FORMAT = '%-12s%s' + + ADDED = 'added' + REMOVED = 'removed' + + # code starts here + + def __init__(self, old, new, ignore=None): + self.result = [] + self.ignore = ignore + self.old_data = { 'tags': {} } + self.new_data = { 'tags': {} } + if self.ignore is None: + self.ignore = [] + + FILEIDX = self.__FILEIDX + for tag in self.ignore: + for entry in FILEIDX: + if tag == entry[0]: + entry[1] = None + break + + old = self.__load_pkg(old) + new = self.__load_pkg(new) + + # Compare single tags + for tag in self.TAGS: + old_tag = old[tag] + new_tag = new[tag] + self.old_data['tags']['tag'] = old[tag] + self.new_data['tags']['tag'] = new[tag] + if old_tag != new_tag: + tagname = rpm.tagnames[tag] + if old_tag == None: + self.__add(self.FORMAT, (self.ADDED, tagname)) + elif new_tag == None: + self.__add(self.FORMAT, (self.REMOVED, tagname)) + else: + self.__add(self.FORMAT, ('S.5........', tagname)) + + # compare Provides, Requires, ... + for tag in self.PRCO: + self.__comparePRCOs(old, new, tag) + + # compare the files + + old_files_dict = self.__fileIteratorToDict(old.fiFromHeader()) + new_files_dict = self.__fileIteratorToDict(new.fiFromHeader()) + files = list(set(itertools.chain(old_files_dict.iterkeys(), + new_files_dict.iterkeys()))) + files.sort() + self.old_data['files'] = old_files_dict + self.new_data['files'] = new_files_dict + + for f in files: + diff = 0 + + old_file = old_files_dict.get(f) + new_file = new_files_dict.get(f) + + if not old_file: + self.__add(self.FORMAT, (self.ADDED, f)) + elif not new_file: + self.__add(self.FORMAT, (self.REMOVED, f)) + else: + format = '' + for entry in FILEIDX: + if entry[1] != None and \ + old_file[entry[1]] != new_file[entry[1]]: + format = format + entry[0] + diff = 1 + else: + format = format + '.' + if diff: + self.__add(self.FORMAT, (format, f)) + + # return a report of the differences + def textdiff(self): + return '\n'.join((format % data for format, data in self.result)) + + # do the two rpms differ + def differs(self): + return bool(self.result) + + # add one differing item + def __add(self, format, data): + self.result.append((format, data)) + + # load a package from a file or from the installed ones + def __load_pkg(self, filename): + ts = rpm.ts() + f = os.open(filename, os.O_RDONLY) + hdr = ts.hdrFromFdno(f) + os.close(f) + return hdr + + # output the right string according to RPMSENSE_* const + def sense2str(self, sense): + s = "" + for tag, char in ((rpm.RPMSENSE_LESS, "<"), + (rpm.RPMSENSE_GREATER, ">"), + (rpm.RPMSENSE_EQUAL, "=")): + if sense & tag: + s += char + return s + + # compare Provides, Requires, Conflicts, Obsoletes + def __comparePRCOs(self, old, new, name): + oldflags = old[name[:-1]+'FLAGS'] + newflags = new[name[:-1]+'FLAGS'] + # fix buggy rpm binding not returning list for single entries + if not isinstance(oldflags, list): oldflags = [ oldflags ] + if not isinstance(newflags, list): newflags = [ newflags ] + + o = zip(old[name], oldflags, old[name[:-1]+'VERSION']) + n = zip(new[name], newflags, new[name[:-1]+'VERSION']) + self.old_data[name] = sorted(o) + self.new_data[name] = sorted(n) + + if name == 'PROVIDES': # filter our self provide + oldNV = (old['name'], rpm.RPMSENSE_EQUAL, + "%s-%s" % (old['version'], old['release'])) + newNV = (new['name'], rpm.RPMSENSE_EQUAL, + "%s-%s" % (new['version'], new['release'])) + o = [entry for entry in o if entry != oldNV] + n = [entry for entry in n if entry != newNV] + + for oldentry in o: + if not oldentry in n: + if name == 'REQUIRES' and oldentry[1] & self.PREREQ_FLAG: + tagname = 'PREREQ' + else: + tagname = name + self.__add(self.DEPFORMAT, + (self.REMOVED, tagname, oldentry[0], + self.sense2str(oldentry[1]), oldentry[2])) + for newentry in n: + if not newentry in o: + if name == 'REQUIRES' and newentry[1] & self.PREREQ_FLAG: + tagname = 'PREREQ' + else: + tagname = name + self.__add(self.DEPFORMAT, + (self.ADDED, tagname, newentry[0], + self.sense2str(newentry[1]), newentry[2])) + + def __fileIteratorToDict(self, fi): + result = {} + for filedata in fi: + result[filedata[0]] = filedata[1:] + return result + + def kojihash(self, new=False): + """return hashed data for use in koji""" + if new: + s = json.dumps(self.new_data, sort_keys=True) + else: + s = json.dumps(self.old_data, sort_keys=True) + return hashlib.sha256(s).hexdigest() + +def _usage(exit=1): + print("Usage: %s [] " % sys.argv[0]) + print("Options:") + print(" -h, --help Output this message and exit") + print(" -i, --ignore Tag to ignore when calculating differences") + print(" (may be used multiple times)") + print(" Valid values are: SM5DNLVUGFT") + sys.exit(exit) + +def main(): + + ignore_tags = [] + try: + opts, args = getopt.getopt(sys.argv[1:], "hi:", ["help", "ignore="]) + except getopt.GetoptError as e: + print("Error: %s" % e) + _usage() + + for option, argument in opts: + if option in ("-h", "--help"): + _usage(0) + if option in ("-i", "--ignore"): + ignore_tags.append(argument) + + if len(args) != 2: + _usage() + + d = Rpmdiff(args[0], args[1], ignore=ignore_tags) + print(d.textdiff()) + sys.exit(int(d.differs())) + +if __name__ == '__main__': + main() + +# rpmdiff ends here From 4f826fd104d5626e05c6c0a603970da020b4cd7c Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 4/11] typo --- diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py index 136be67..f530da8 100755 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -98,8 +98,8 @@ class Rpmdiff: for tag in self.TAGS: old_tag = old[tag] new_tag = new[tag] - self.old_data['tags']['tag'] = old[tag] - self.new_data['tags']['tag'] = new[tag] + self.old_data['tags'][tag] = old[tag] + self.new_data['tags'][tag] = new[tag] if old_tag != new_tag: tagname = rpm.tagnames[tag] if old_tag == None: From d8b3aef86a093e50e54eba1dbd3ea174c5d40c99 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 5/11] check for empty header --- diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py index f530da8..6b92c12 100755 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -224,9 +224,12 @@ class Rpmdiff: def kojihash(self, new=False): """return hashed data for use in koji""" if new: - s = json.dumps(self.new_data, sort_keys=True) + data = self.new_data else: - s = json.dumps(self.old_data, sort_keys=True) + data = self.old_data + if not data: + raise ValueError("rpm header data are empty") + s = json.dumps(self.old_data, sort_keys=True) return hashlib.sha256(s).hexdigest() def _usage(exit=1): From 200f29d89feebc1de11ae1f41a3087644029f93f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 6/11] TEST: add sanity check to rpmdiff main --- diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py index 6b92c12..220f554 100755 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -229,7 +229,7 @@ class Rpmdiff: data = self.old_data if not data: raise ValueError("rpm header data are empty") - s = json.dumps(self.old_data, sort_keys=True) + s = json.dumps(data, sort_keys=True) return hashlib.sha256(s).hexdigest() def _usage(exit=1): @@ -261,7 +261,11 @@ def main(): d = Rpmdiff(args[0], args[1], ignore=ignore_tags) print(d.textdiff()) - sys.exit(int(d.differs())) + rv = d.differs() + chk = (d.kojihash() != d.kojihash(new=True)) + if rv != chk: + raise Exception('hash compare disagrees with rpmdiff') + sys.exit(int(rv)) if __name__ == '__main__': main() From a0f34aa5619c3ba5fa92dfe12c2879762a815267 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 7/11] fix tests for rpmdiff --- diff --git a/tests/test_hub/test_rpmdiff.py b/tests/test_hub/test_rpmdiff.py index 4cbabb3..f8bae3e 100644 --- a/tests/test_hub/test_rpmdiff.py +++ b/tests/test_hub/test_rpmdiff.py @@ -8,29 +8,29 @@ import kojihub class TestRPMDiff(unittest.TestCase): - @mock.patch('kojihub._rpmdiff.Rpmdiff') + @mock.patch('koji.rpmdiff.Rpmdiff') def test_rpmdiff_empty_invocation(self, Rpmdiff): - kojihub.rpmdiff('basepath', []) + kojihub.rpmdiff('basepath', [], hashes={}) Rpmdiff.assert_not_called() - kojihub.rpmdiff('basepath', ['foo']) + kojihub.rpmdiff('basepath', ['foo'], hashes={}) Rpmdiff.assert_not_called() - @mock.patch('kojihub._rpmdiff.Rpmdiff') + @mock.patch('koji.rpmdiff.Rpmdiff') def test_rpmdiff_simple_success(self, Rpmdiff): d = mock.MagicMock() d.differs.return_value = False Rpmdiff.return_value = d - self.assertFalse(kojihub.rpmdiff('basepath', ['foo', 'bar'])) - Rpmdiff.assert_called_once_with('basepath/foo', 'basepath/bar', ignore='S5TN') + self.assertFalse(kojihub.rpmdiff('basepath', ['12/1234/foo', '23/2345/bar'], hashes={})) + Rpmdiff.assert_called_once_with('basepath/12/1234/foo', 'basepath/23/2345/bar', ignore='S5TN') - @mock.patch('kojihub._rpmdiff.Rpmdiff') + @mock.patch('koji.rpmdiff.Rpmdiff') def test_rpmdiff_simple_failure(self, Rpmdiff): d = mock.MagicMock() d.differs.return_value = True Rpmdiff.return_value = d with self.assertRaises(koji.BuildError): - kojihub.rpmdiff('basepath', ['foo', 'bar']) - Rpmdiff.assert_called_once_with('basepath/foo', 'basepath/bar', ignore='S5TN') + kojihub.rpmdiff('basepath', ['12/1234/foo', '13/1345/bar'], hashes={}) + Rpmdiff.assert_called_once_with('basepath/12/1234/foo', 'basepath/13/1345/bar', ignore='S5TN') d.textdiff.assert_called_once_with() class TestCheckNoarchRpms(unittest.TestCase): @@ -42,10 +42,10 @@ class TestCheckNoarchRpms(unittest.TestCase): @mock.patch('kojihub.rpmdiff') def test_check_noarch_rpms_simple_invocation(self, rpmdiff): - originals = ['foo.noarch.rpm', 'bar.noarch.rpm'] + originals = ['12/1234/foo.noarch.rpm', '23/2345/foo.noarch.rpm'] result = kojihub.check_noarch_rpms('basepath', copy.copy(originals)) - self.assertEquals(result, originals) - self.assertEquals(len(rpmdiff.mock_calls), 2) + self.assertEquals(result, originals[0:1]) + self.assertEquals(len(rpmdiff.mock_calls), 1) @mock.patch('kojihub.rpmdiff') def test_check_noarch_rpms_with_duplicates(self, rpmdiff): @@ -56,7 +56,7 @@ class TestCheckNoarchRpms(unittest.TestCase): ] result = kojihub.check_noarch_rpms('basepath', copy.copy(originals)) self.assertEquals(result, ['bar.noarch.rpm']) - rpmdiff.assert_called_once_with('basepath', originals) + rpmdiff.assert_called_once_with('basepath', originals, hashes={}) @mock.patch('kojihub.rpmdiff') def test_check_noarch_rpms_with_mixed(self, rpmdiff): @@ -70,6 +70,8 @@ class TestCheckNoarchRpms(unittest.TestCase): self.assertEquals(result, [ 'foo.x86_64.rpm', 'bar.x86_64.rpm', 'bar.noarch.rpm' ]) - rpmdiff.assert_called_once_with('basepath', [ - 'bar.noarch.rpm', 'bar.noarch.rpm' - ]) + rpmdiff.assert_called_once_with( + 'basepath', + ['bar.noarch.rpm', 'bar.noarch.rpm'], + hashes={} + ) From d79aa6c6cd3620f764f0bcf07da63a2f695582e6 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 8/11] make rpmdiff.py lib only Original sript-like behaviour is moved to koji-tools https://pagure.io/koji-tools/pull-request/6 --- diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py old mode 100755 new mode 100644 index 220f554..14de022 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -1,5 +1,3 @@ -#!/usr/bin/python -# # Copyright (C) 2006 Mandriva; 2009-2014 Red Hat, Inc. # Authors: Frederic Lepied, Florian Festi # @@ -26,9 +24,6 @@ import rpm import os import itertools -import sys, getopt - - class Rpmdiff: # constants @@ -231,43 +226,3 @@ class Rpmdiff: raise ValueError("rpm header data are empty") s = json.dumps(data, sort_keys=True) return hashlib.sha256(s).hexdigest() - -def _usage(exit=1): - print("Usage: %s [] " % sys.argv[0]) - print("Options:") - print(" -h, --help Output this message and exit") - print(" -i, --ignore Tag to ignore when calculating differences") - print(" (may be used multiple times)") - print(" Valid values are: SM5DNLVUGFT") - sys.exit(exit) - -def main(): - - ignore_tags = [] - try: - opts, args = getopt.getopt(sys.argv[1:], "hi:", ["help", "ignore="]) - except getopt.GetoptError as e: - print("Error: %s" % e) - _usage() - - for option, argument in opts: - if option in ("-h", "--help"): - _usage(0) - if option in ("-i", "--ignore"): - ignore_tags.append(argument) - - if len(args) != 2: - _usage() - - d = Rpmdiff(args[0], args[1], ignore=ignore_tags) - print(d.textdiff()) - rv = d.differs() - chk = (d.kojihash() != d.kojihash(new=True)) - if rv != chk: - raise Exception('hash compare disagrees with rpmdiff') - sys.exit(int(rv)) - -if __name__ == '__main__': - main() - -# rpmdiff ends here From 05decae3b6a71a0e8d50fc2ae162a6c20bcef8a4 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 9/11] use 'ignore' in hash computation --- diff --git a/builder/kojid b/builder/kojid index f9f46d2..5bd366a 100755 --- a/builder/kojid +++ b/builder/kojid @@ -1255,7 +1255,7 @@ class BuildArchTask(BaseBuildTask): if rpmf.endswith('.noarch.rpm'): fpath = os.path.join(resultdir, rpmf) d = koji.rpmdiff.Rpmdiff(fpath, fpath, ignore='S5TN') - rpmdiff_hash[self.id][rpmf] = d.kojihash() + rpmdiff_hash[self.id][rpmf] = d.kojihash(ignore='S5TN') if rpmdiff_hash[self.id]: log_name = 'noarch_rpmdiff.json' noarch_hash_path = os.path.join(broot.workdir, log_name) diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py index 14de022..fa77dcc 100644 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -74,8 +74,8 @@ class Rpmdiff: def __init__(self, old, new, ignore=None): self.result = [] self.ignore = ignore - self.old_data = { 'tags': {} } - self.new_data = { 'tags': {} } + self.old_data = { 'tags': {}, 'ignore': ignore } + self.new_data = { 'tags': {}, 'ignore': ignore } if self.ignore is None: self.ignore = [] From 3fc94ef1fd79ab9065b364d67d272a92ac15ed1d Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 08:40:31 +0000 Subject: [PATCH 10/11] remove duplicate ignore --- diff --git a/builder/kojid b/builder/kojid index 5bd366a..f9f46d2 100755 --- a/builder/kojid +++ b/builder/kojid @@ -1255,7 +1255,7 @@ class BuildArchTask(BaseBuildTask): if rpmf.endswith('.noarch.rpm'): fpath = os.path.join(resultdir, rpmf) d = koji.rpmdiff.Rpmdiff(fpath, fpath, ignore='S5TN') - rpmdiff_hash[self.id][rpmf] = d.kojihash(ignore='S5TN') + rpmdiff_hash[self.id][rpmf] = d.kojihash() if rpmdiff_hash[self.id]: log_name = 'noarch_rpmdiff.json' noarch_hash_path = os.path.join(broot.workdir, log_name) From 4b4d340ca40baabde563ab79451f9347e693a31a Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 19 2018 09:05:48 +0000 Subject: [PATCH 11/11] erase ignored fields --- diff --git a/koji/rpmdiff.py b/koji/rpmdiff.py index fa77dcc..db66b15 100644 --- a/koji/rpmdiff.py +++ b/koji/rpmdiff.py @@ -83,7 +83,8 @@ class Rpmdiff: for tag in self.ignore: for entry in FILEIDX: if tag == entry[0]: - entry[1] = None + # store marked position for erasing data + entry[1] = -entry[1] break old = self.__load_pkg(old) @@ -131,10 +132,15 @@ class Rpmdiff: else: format = '' for entry in FILEIDX: - if entry[1] != None and \ + if entry[1] >= 0 and \ old_file[entry[1]] != new_file[entry[1]]: format = format + entry[0] diff = 1 + elif entry[1] < 0: + # erase fields which are ignored + old_file[-entry[1]] = None + new_file[-entry[1]] = None + format = format + '.' else: format = format + '.' if diff: @@ -213,7 +219,7 @@ class Rpmdiff: def __fileIteratorToDict(self, fi): result = {} for filedata in fi: - result[filedata[0]] = filedata[1:] + result[filedata[0]] = list(filedata[1:]) return result def kojihash(self, new=False):