[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Bug 21567 - Fedabipkgdiff matches build distro names too tightly



Hello Chenxiong,

This patch fixes the Fedabipkgdiff problem that Sinny Kumari filed at
https://sourceware.org/bugzilla/show_bug.cgi?id=21567.

To understand the problem, consider the build libxcb-1.12-3.fc26, which
has been tagged for Fedora 27 (i.e, fc27).

When we ask fedabipkgdiff to get the builds of libxcb for Fedora 27,
it looks for builds which release string ends with 'fc27'.  It thus
fails to pick libxcb-1.12-3.fc26.

This patch changes this behaviour by making
Brew.get_package_latest_build to first try to get builds which release
string match exactly the distro string we are looking at.

But then if no build was found, the member function then tries to get
builds for which the distro part of the release string is "less than"
(in a lexicographic way) the distro string we are looking at.

I haven't added any regression test specifically for this because we
are planning to use the libabigail-selfcheck external tool to perform
regression testing on tons of Fedora packages:
https://pagure.io/libabigail-selfcheck.  Fixing this issue is a
pre-requisite for putting that regression test infrastructure in
place.

Is it OK to commit this into the master branch?

Thanks.

	* tools/fedabipkgdiff (get_distro_from_string): Define new function.
	(Brew.get_package_latest_build): Also consider builds which distro
	property is less than the expected distro string that we were
	given.


Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tools/fedabipkgdiff | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff
index a8329d4..42287fe 100755
--- a/tools/fedabipkgdiff
+++ b/tools/fedabipkgdiff
@@ -172,6 +172,26 @@ def is_distro_valid(distro):
     return re.match(r'^(fc|el)\d{1,2}$', distro) is not None
 
 
+def get_distro_from_string(str):
+    """Get the part of a string that designates the Fedora distro version number
+
+    For instance, when passed the string '2.3.fc12', this function
+    returns the string 'fc12'.
+
+    :param str the string to consider
+    :return: The sub-string of the parameter that represents the
+    Fedora distro version number, or None if the parameter does not
+    contain such a sub-string.
+    """
+
+    m = re.match(r'(.*)((fc|el)\d{1,2})(.*)', str)
+    if not m:
+        return None
+
+    distro = m.group(2)
+    return distro
+
+
 def match_nvr(s):
     """Determine if a string is a N-V-R"""
     return re.match(r'^([^/]+)-(.+)-(.+)$', s) is not None
@@ -776,7 +796,7 @@ class Brew(object):
 
     @log_call
     def get_package_latest_build(self, package_name, distro):
-        """Get latest build from a package
+        """Get latest build from a package, for a particular distro.
 
         Example:
 
@@ -797,6 +817,27 @@ class Brew(object):
                                  order_by='nvr',
                                  reverse=True)
         if not builds:
+            # So we found no build which distro string exactly matches
+            # the 'distro' parameter.
+            #
+            # Now lets try to get builds which distro string are less
+            # than the value of the 'distro' parameter.  This is for
+            # cases when, for instnace, the build of package foo that
+            # is present in current Fedora 27 is foo-1.fc26.  That
+            # build originates from Fedora 26 but is being re-used in
+            # Fedora 27.  So we want this function to pick up that
+            # foo-1.fc26, even though we want the builds of foo that
+            # match the distro string fc27.
+
+            selector = lambda build: get_distro(build['release']) and \
+                       get_distro(build['release']) <= distro
+
+            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))
 
-- 
1.8.3.1
		Dodji