Patchwork [v2] sanity.bbclass: support wildcards in SANITY_TESTED_DISTROS

login
register
mail settings
Submitter Olof Johansson
Date March 3, 2014, 2:37 p.m.
Message ID <1393857456-18343-1-git-send-email-olof.johansson@axis.com>
Download mbox | patch
Permalink /patch/67859/
State Accepted
Commit b1a75ba1d828b6f5e5b863937a0bb4683f352817
Headers show

Comments

Olof Johansson - March 3, 2014, 2:37 p.m.
With this change, you can use shell like globbing expressions (as
supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
are supported" with the entry "Debian-7.*".

[YOCTO #5265]

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 meta/classes/sanity.bbclass | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
Khem Raj - March 3, 2014, 6:17 p.m.
On Mon, Mar 3, 2014 at 6:37 AM, Olof Johansson <olof.johansson@axis.com> wrote:
> With this change, you can use shell like globbing expressions (as
> supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
> This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
> are supported" with the entry "Debian-7.*".

I dont think its a good thing. We should be strict about it as we are.
otherwise it can end up
with bigger problems to save few  typing words

>
> [YOCTO #5265]
>
> Signed-off-by: Olof Johansson <olof.johansson@axis.com>
> ---
>  meta/classes/sanity.bbclass | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index bae010d..d79db8f 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -246,6 +246,8 @@ def check_connectivity(d):
>      return retval
>
>  def check_supported_distro(sanity_data):
> +    from fnmatch import fnmatch
> +
>      tested_distros = sanity_data.getVar('SANITY_TESTED_DISTROS', True)
>      if not tested_distros:
>          return
> @@ -255,12 +257,15 @@ def check_supported_distro(sanity_data):
>      except Exception:
>          distro = None
>
> -    if distro:
> -        if distro not in [x.strip() for x in tested_distros.split('\\n')]:
> -            bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
> -    else:
> +    if not distro:
>          bb.warn('Host distribution could not be determined; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.')
>
> +    for supported in [x.strip() for x in tested_distros.split('\\n')]:
> +        if fnmatch(distro, supported):
> +            return
> +
> +    bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
> +
>  # Checks we should only make if MACHINE is set correctly
>  def check_sanity_validmachine(sanity_data):
>      messages = ""
> --
> 1.8.5.3
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Mark Hatle - March 3, 2014, 8:13 p.m.
On 3/3/14, 12:17 PM, Khem Raj wrote:
> On Mon, Mar 3, 2014 at 6:37 AM, Olof Johansson <olof.johansson@axis.com> wrote:
>> With this change, you can use shell like globbing expressions (as
>> supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
>> This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
>> are supported" with the entry "Debian-7.*".
>
> I dont think its a good thing. We should be strict about it as we are.
> otherwise it can end up
> with bigger problems to save few  typing words

We (and I know other commercial vendors) are already using patches similar to 
this to do exactly the same thing.

I'd hope that a generic mechanism (like this) could be put in place.. and that 
oe-core/Yocto Project would choose -not- to use it for their distributions.  (To 
avoid the exact problems you mention...)

The commercial folks like me can then use it, as we have to deal with the 
problems anyway.

--Mark

>>
>> [YOCTO #5265]
>>
>> Signed-off-by: Olof Johansson <olof.johansson@axis.com>
>> ---
>>   meta/classes/sanity.bbclass | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
>> index bae010d..d79db8f 100644
>> --- a/meta/classes/sanity.bbclass
>> +++ b/meta/classes/sanity.bbclass
>> @@ -246,6 +246,8 @@ def check_connectivity(d):
>>       return retval
>>
>>   def check_supported_distro(sanity_data):
>> +    from fnmatch import fnmatch
>> +
>>       tested_distros = sanity_data.getVar('SANITY_TESTED_DISTROS', True)
>>       if not tested_distros:
>>           return
>> @@ -255,12 +257,15 @@ def check_supported_distro(sanity_data):
>>       except Exception:
>>           distro = None
>>
>> -    if distro:
>> -        if distro not in [x.strip() for x in tested_distros.split('\\n')]:
>> -            bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
>> -    else:
>> +    if not distro:
>>           bb.warn('Host distribution could not be determined; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.')
>>
>> +    for supported in [x.strip() for x in tested_distros.split('\\n')]:
>> +        if fnmatch(distro, supported):
>> +            return
>> +
>> +    bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
>> +
>>   # Checks we should only make if MACHINE is set correctly
>>   def check_sanity_validmachine(sanity_data):
>>       messages = ""
>> --
>> 1.8.5.3
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
Khem Raj - March 3, 2014, 8:41 p.m.
On Mon, Mar 3, 2014 at 12:13 PM, Mark Hatle <mark.hatle@windriver.com> wrote:
>
> We (and I know other commercial vendors) are already using patches similar
> to this to do exactly the same thing.
>
> I'd hope that a generic mechanism (like this) could be put in place.. and
> that oe-core/Yocto Project would choose -not- to use it for their
> distributions.  (To avoid the exact problems you mention...)
>
> The commercial folks like me can then use it, as we have to deal with the
> problems anyway.

IMO with such globs you are covering untested versions too that I see
as a problem.
Olof Johansson - March 4, 2014, 8:37 a.m.
On 14-03-03 19:17 +0100, Khem Raj wrote:
> On Mon, Mar 3, 2014 at 6:37 AM, Olof Johansson <olof.johansson@axis.com> wrote:
> > With this change, you can use shell like globbing expressions (as
> > supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
> > This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
> > are supported" with the entry "Debian-7.*".
> 
> I dont think its a good thing. We should be strict about it as we are.
> otherwise it can end up
> with bigger problems to save few  typing words

I don't really think we are strict about it today. Currently,
Ubuntu LTS (12.04) does not include point release number in its
lsb release name. Debian does (and I noticed a similar bump patch
was sent for CentOS recently). I suspect that in many cases, a
new Ubuntu point release contains about the same changes that the
Debian point release does.

The issue is that new point releases, at least in the case of
Debian, happens about every two months. These bumps are
integrated to master, backported to older branches, but it still
won't reach people working off releases immediately.

Even if Poky chooses not to adopt this, it would still be a nice
thing to have support for doing this internally for our
layer/distro.
Anders Darander - March 4, 2014, 8:56 a.m.
* Olof Johansson <olof.johansson@axis.com> [140304 09:37]:

> On 14-03-03 19:17 +0100, Khem Raj wrote:
> > On Mon, Mar 3, 2014 at 6:37 AM, Olof Johansson <olof.johansson@axis.com> wrote:
> > > With this change, you can use shell like globbing expressions (as
> > > supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
> > > This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
> > > are supported" with the entry "Debian-7.*".

> > I dont think its a good thing. We should be strict about it as we are.
> > otherwise it can end up
> > with bigger problems to save few  typing words

> I don't really think we are strict about it today. Currently,
> Ubuntu LTS (12.04) does not include point release number in its
> lsb release name. Debian does (and I noticed a similar bump patch
> was sent for CentOS recently). I suspect that in many cases, a
> new Ubuntu point release contains about the same changes that the
> Debian point release does.

> The issue is that new point releases, at least in the case of
> Debian, happens about every two months. These bumps are
> integrated to master, backported to older branches, but it still
> won't reach people working off releases immediately.

> Even if Poky chooses not to adopt this, it would still be a nice
> thing to have support for doing this internally for our
> layer/distro.

I'm with Olof and Mark on this one. 

We should at least add the code for allowing wildcards in the
SANITY_TESTED_DISTROS. Then everyone creating our own setups/distros can
choose to add wildcards or not.

Whether we should add wildcards in Poky is a separate question. Though,
we should treat e.g. Debian and Ubuntu in the same way.

Cheers,
Anders
Paul Eggleton - March 4, 2014, 10:56 a.m.
On Tuesday 04 March 2014 09:56:53 Anders Darander wrote:
> * Olof Johansson <olof.johansson@axis.com> [140304 09:37]:
> > On 14-03-03 19:17 +0100, Khem Raj wrote:
> > > On Mon, Mar 3, 2014 at 6:37 AM, Olof Johansson <olof.johansson@axis.com> 
wrote:
> > > > With this change, you can use shell like globbing expressions (as
> > > > supported by Python's fnmatch) for entries in SANITY_TESTED_DISTROS.
> > > > This makes it possible to say that, e.g. "all Debian 7 Wheezy releases
> > > > are supported" with the entry "Debian-7.*".
> > > 
> > > I dont think its a good thing. We should be strict about it as we are.
> > > otherwise it can end up
> > > with bigger problems to save few  typing words
> > 
> > I don't really think we are strict about it today. Currently,
> > Ubuntu LTS (12.04) does not include point release number in its
> > lsb release name. Debian does (and I noticed a similar bump patch
> > was sent for CentOS recently). I suspect that in many cases, a
> > new Ubuntu point release contains about the same changes that the
> > Debian point release does.
> > 
> > The issue is that new point releases, at least in the case of
> > Debian, happens about every two months. These bumps are
> > integrated to master, backported to older branches, but it still
> > won't reach people working off releases immediately.
> > 
> > Even if Poky chooses not to adopt this, it would still be a nice
> > thing to have support for doing this internally for our
> > layer/distro.
> 
> I'm with Olof and Mark on this one.
> 
> We should at least add the code for allowing wildcards in the
> SANITY_TESTED_DISTROS. Then everyone creating our own setups/distros can
> choose to add wildcards or not.
> 
> Whether we should add wildcards in Poky is a separate question. Though,
> we should treat e.g. Debian and Ubuntu in the same way.

FWIW the above reflects my thoughts as well. We'll need to decide whether or 
not Poky adopts it but we should have the mechanism regardless.

Cheers,
Paul

Patch

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index bae010d..d79db8f 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -246,6 +246,8 @@  def check_connectivity(d):
     return retval
 
 def check_supported_distro(sanity_data):
+    from fnmatch import fnmatch
+
     tested_distros = sanity_data.getVar('SANITY_TESTED_DISTROS', True)
     if not tested_distros:
         return
@@ -255,12 +257,15 @@  def check_supported_distro(sanity_data):
     except Exception:
         distro = None
 
-    if distro:
-        if distro not in [x.strip() for x in tested_distros.split('\\n')]:
-            bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
-    else:
+    if not distro:
         bb.warn('Host distribution could not be determined; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.')
 
+    for supported in [x.strip() for x in tested_distros.split('\\n')]:
+        if fnmatch(distro, supported):
+            return
+
+    bb.warn('Host distribution "%s" has not been validated with this version of the build system; you may possibly experience unexpected failures. It is recommended that you use a tested distribution.' % distro)
+
 # Checks we should only make if MACHINE is set correctly
 def check_sanity_validmachine(sanity_data):
     messages = ""