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

[Dodji Seketeli <dodji@redhat.com>]

Sinny Kumari <ksinny@gmail.com> a écrit:

[...]

>>          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
>
> Small typo, s/instnace/instance/

Good catch.  I have fixed this in the patch below.

>
>> +            # 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
>
> You probably wanted to mention get_distro_from_string instead of
> get_distro because newly defined function name is get_distro_from_string.

And you are right again.  Oops.  I am fixing this too in the patch
below.

So here is the update patch.

Thanks!

From 08d297dd35e6bde7638787a246c181af3c534d72 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 14 Jun 2017 11:08:19 +0200
Subject: [PATCH] Bug 21567 - Fedabipkgdiff matches build distro names too
 tightly

The build libxcb-1.12-3.fc26 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.

	* 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..20cfd19 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 instance, 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_from_string(build['release']) and \
+                       get_distro_from_string(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