From fb0ba44a3c394ee718014a9931c379ef20b6e8a9 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Apr 27 2018 15:53:37 +0000
Subject: 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 7f690bf..284ae1a 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
@@ -8358,21 +8360,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')