[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Bug 21567 - Fedabipkgdiff matches build distro names too tightly
On Wed, Jun 14, 2017 at 7:54 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> 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.
+1 from me
> 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
--
http://sinny.io/