Openembedded Core Discussions
 help / color / mirror / Atom feed
* [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary
@ 2024-09-26  9:17 Claus Stovgaard
  2024-09-26  9:51 ` [OE-core] " Alexander Kanavin
  0 siblings, 1 reply; 5+ messages in thread
From: Claus Stovgaard @ 2024-09-26  9:17 UTC (permalink / raw)
  To: openembedded-core; +Cc: Claus Stovgaard

We are rewriting the globs variable, so it can't be None, but rather a
string. E.g. the lines above.

if globs is None:
    globs = self.d.getVar('IMAGE_INSTALL_COMPLEMENTARY')

As globs is now a string, we need to test for empty string instead of
compare with None.

Signed-off-by: Claus Stovgaard <claus.stovgaard@gmail.com>
---
 meta/lib/oe/package_manager/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/package_manager/__init__.py b/meta/lib/oe/package_manager/__init__.py
index d3b2317894..1d923c436e 100644
--- a/meta/lib/oe/package_manager/__init__.py
+++ b/meta/lib/oe/package_manager/__init__.py
@@ -365,7 +365,7 @@ class PackageManager(object, metaclass=ABCMeta):
                 for complementary_linguas in (self.d.getVar('IMAGE_LINGUAS_COMPLEMENTARY') or "").split():
                     globs += (" " + complementary_linguas) % lang
 
-        if globs is None:
+        if not globs:
             return
 
         # we need to write the list of installed packages to a file because the
-- 
2.45.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [OE-core] [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary
  2024-09-26  9:17 [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary Claus Stovgaard
@ 2024-09-26  9:51 ` Alexander Kanavin
  2024-09-26 15:01   ` claus.stovgaard
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Kanavin @ 2024-09-26  9:51 UTC (permalink / raw)
  To: claus.stovgaard; +Cc: openembedded-core

'optimize install_complementary' really doesn't match what the commit
contains, please be more specific.

Alex

On Thu, 26 Sept 2024 at 11:17, Claus Stovgaard via
lists.openembedded.org
<claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
>
> We are rewriting the globs variable, so it can't be None, but rather a
> string. E.g. the lines above.
>
> if globs is None:
>     globs = self.d.getVar('IMAGE_INSTALL_COMPLEMENTARY')
>
> As globs is now a string, we need to test for empty string instead of
> compare with None.
>
> Signed-off-by: Claus Stovgaard <claus.stovgaard@gmail.com>
> ---
>  meta/lib/oe/package_manager/__init__.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/lib/oe/package_manager/__init__.py b/meta/lib/oe/package_manager/__init__.py
> index d3b2317894..1d923c436e 100644
> --- a/meta/lib/oe/package_manager/__init__.py
> +++ b/meta/lib/oe/package_manager/__init__.py
> @@ -365,7 +365,7 @@ class PackageManager(object, metaclass=ABCMeta):
>                  for complementary_linguas in (self.d.getVar('IMAGE_LINGUAS_COMPLEMENTARY') or "").split():
>                      globs += (" " + complementary_linguas) % lang
>
> -        if globs is None:
> +        if not globs:
>              return
>
>          # we need to write the list of installed packages to a file because the
> --
> 2.45.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#204970): https://lists.openembedded.org/g/openembedded-core/message/204970
> Mute This Topic: https://lists.openembedded.org/mt/108664320/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [OE-core] [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary
  2024-09-26  9:51 ` [OE-core] " Alexander Kanavin
@ 2024-09-26 15:01   ` claus.stovgaard
  2024-09-26 15:12     ` Alexander Kanavin
  0 siblings, 1 reply; 5+ messages in thread
From: claus.stovgaard @ 2024-09-26 15:01 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: openembedded-core

Hi Alex

Would a better headline be "don't handle empty glob"


Also a small question.

Looking at the getVar code - It seems that it can return None in some
specific situations.

So it might be more clear to do

if globs is None or globs == "":
    return

The optimizing part is that the install_complementary normally does.


if globs is None:
    globs = ""

if globs is None:
    return


do work, where if glob is empty string, it does not matter.
(Involve creating installed_pkgs file, and process it with oe-pkgdata-
util but with empty glob, so it will never return anything, just spend
time on each line in the file, and create a empty list of packages,
where it will call install of those)


So the change optimize by removing the work on an empty string.

What is your take.

Do you like "don't handle empty glob" as headline, and what do you
prefer

if globs is None or globs == "":
    return

vs

if not globs:
    return


/Claus


On Thu, 2024-09-26 at 11:51 +0200, Alexander Kanavin wrote:
> 'optimize install_complementary' really doesn't match what the commit
> contains, please be more specific.
> 
> Alex
> 
> On Thu, 26 Sept 2024 at 11:17, Claus Stovgaard via
> lists.openembedded.org
> <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> > 
> > We are rewriting the globs variable, so it can't be None, but
> > rather a
> > string. E.g. the lines above.
> > 
> > if globs is None:
> >     globs = self.d.getVar('IMAGE_INSTALL_COMPLEMENTARY')
> > 
> > As globs is now a string, we need to test for empty string instead
> > of
> > compare with None.
> > 
> > Signed-off-by: Claus Stovgaard <claus.stovgaard@gmail.com>
> > ---
> >  meta/lib/oe/package_manager/__init__.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/lib/oe/package_manager/__init__.py
> > b/meta/lib/oe/package_manager/__init__.py
> > index d3b2317894..1d923c436e 100644
> > --- a/meta/lib/oe/package_manager/__init__.py
> > +++ b/meta/lib/oe/package_manager/__init__.py
> > @@ -365,7 +365,7 @@ class PackageManager(object,
> > metaclass=ABCMeta):
> >                  for complementary_linguas in
> > (self.d.getVar('IMAGE_LINGUAS_COMPLEMENTARY') or "").split():
> >                      globs += (" " + complementary_linguas) % lang
> > 
> > -        if globs is None:
> > +        if not globs:
> >              return
> > 
> >          # we need to write the list of installed packages to a
> > file because the
> > --
> > 2.45.2
> > 
> > 
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#204970):
> > https://lists.openembedded.org/g/openembedded-core/message/204970
> > Mute This Topic:
> > https://lists.openembedded.org/mt/108664320/1686489
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe:
> > https://lists.openembedded.org/g/openembedded-core/unsub [
> > alex.kanavin@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> > 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [OE-core] [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary
  2024-09-26 15:01   ` claus.stovgaard
@ 2024-09-26 15:12     ` Alexander Kanavin
  2024-09-26 20:41       ` Claus Stovgaard
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Kanavin @ 2024-09-26 15:12 UTC (permalink / raw)
  To: claus.stovgaard; +Cc: openembedded-core

Hello,

the headline should be 'return early from install_complementary if
glob is either None or an empty string'.

I prefer a shorter condition.

Alex

On Thu, 26 Sept 2024 at 17:01, <claus.stovgaard@gmail.com> wrote:
>
> Hi Alex
>
> Would a better headline be "don't handle empty glob"
>
>
> Also a small question.
>
> Looking at the getVar code - It seems that it can return None in some
> specific situations.
>
> So it might be more clear to do
>
> if globs is None or globs == "":
>     return
>
> The optimizing part is that the install_complementary normally does.
>
>
> if globs is None:
>     globs = ""
>
> if globs is None:
>     return
>
>
> do work, where if glob is empty string, it does not matter.
> (Involve creating installed_pkgs file, and process it with oe-pkgdata-
> util but with empty glob, so it will never return anything, just spend
> time on each line in the file, and create a empty list of packages,
> where it will call install of those)
>
>
> So the change optimize by removing the work on an empty string.
>
> What is your take.
>
> Do you like "don't handle empty glob" as headline, and what do you
> prefer
>
> if globs is None or globs == "":
>     return
>
> vs
>
> if not globs:
>     return
>
>
> /Claus
>
>
> On Thu, 2024-09-26 at 11:51 +0200, Alexander Kanavin wrote:
> > 'optimize install_complementary' really doesn't match what the commit
> > contains, please be more specific.
> >
> > Alex
> >
> > On Thu, 26 Sept 2024 at 11:17, Claus Stovgaard via
> > lists.openembedded.org
> > <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> > >
> > > We are rewriting the globs variable, so it can't be None, but
> > > rather a
> > > string. E.g. the lines above.
> > >
> > > if globs is None:
> > >     globs = self.d.getVar('IMAGE_INSTALL_COMPLEMENTARY')
> > >
> > > As globs is now a string, we need to test for empty string instead
> > > of
> > > compare with None.
> > >
> > > Signed-off-by: Claus Stovgaard <claus.stovgaard@gmail.com>
> > > ---
> > >  meta/lib/oe/package_manager/__init__.py | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/meta/lib/oe/package_manager/__init__.py
> > > b/meta/lib/oe/package_manager/__init__.py
> > > index d3b2317894..1d923c436e 100644
> > > --- a/meta/lib/oe/package_manager/__init__.py
> > > +++ b/meta/lib/oe/package_manager/__init__.py
> > > @@ -365,7 +365,7 @@ class PackageManager(object,
> > > metaclass=ABCMeta):
> > >                  for complementary_linguas in
> > > (self.d.getVar('IMAGE_LINGUAS_COMPLEMENTARY') or "").split():
> > >                      globs += (" " + complementary_linguas) % lang
> > >
> > > -        if globs is None:
> > > +        if not globs:
> > >              return
> > >
> > >          # we need to write the list of installed packages to a
> > > file because the
> > > --
> > > 2.45.2
> > >
> > >
> > > -=-=-=-=-=-=-=-=-=-=-=-
> > > Links: You receive all messages sent to this group.
> > > View/Reply Online (#204970):
> > > https://lists.openembedded.org/g/openembedded-core/message/204970
> > > Mute This Topic:
> > > https://lists.openembedded.org/mt/108664320/1686489
> > > Group Owner: openembedded-core+owner@lists.openembedded.org
> > > Unsubscribe:
> > > https://lists.openembedded.org/g/openembedded-core/unsub [
> > > alex.kanavin@gmail.com]
> > > -=-=-=-=-=-=-=-=-=-=-=-
> > >
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [OE-core] [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary
  2024-09-26 15:12     ` Alexander Kanavin
@ 2024-09-26 20:41       ` Claus Stovgaard
  0 siblings, 0 replies; 5+ messages in thread
From: Claus Stovgaard @ 2024-09-26 20:41 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: openembedded-core

Hi

Se version 2 of the patch.

/Claus

On Thu, 2024-09-26 at 17:12 +0200, Alexander Kanavin wrote:
> Hello,
> 
> the headline should be 'return early from install_complementary if
> glob is either None or an empty string'.
> 
> I prefer a shorter condition.
> 
> Alex
> 
> On Thu, 26 Sept 2024 at 17:01, <claus.stovgaard@gmail.com> wrote:
> > 
> > Hi Alex
> > 
> > Would a better headline be "don't handle empty glob"
> > 
> > 
> > Also a small question.
> > 
> > Looking at the getVar code - It seems that it can return None in
> > some
> > specific situations.
> > 
> > So it might be more clear to do
> > 
> > if globs is None or globs == "":
> >     return
> > 
> > The optimizing part is that the install_complementary normally
> > does.
> > 
> > 
> > if globs is None:
> >     globs = ""
> > 
> > if globs is None:
> >     return
> > 
> > 
> > do work, where if glob is empty string, it does not matter.
> > (Involve creating installed_pkgs file, and process it with oe-
> > pkgdata-
> > util but with empty glob, so it will never return anything, just
> > spend
> > time on each line in the file, and create a empty list of packages,
> > where it will call install of those)
> > 
> > 
> > So the change optimize by removing the work on an empty string.
> > 
> > What is your take.
> > 
> > Do you like "don't handle empty glob" as headline, and what do you
> > prefer
> > 
> > if globs is None or globs == "":
> >     return
> > 
> > vs
> > 
> > if not globs:
> >     return
> > 
> > 
> > /Claus
> > 
> > 
> > On Thu, 2024-09-26 at 11:51 +0200, Alexander Kanavin wrote:
> > > 'optimize install_complementary' really doesn't match what the
> > > commit
> > > contains, please be more specific.
> > > 
> > > Alex
> > > 
> > > On Thu, 26 Sept 2024 at 11:17, Claus Stovgaard via
> > > lists.openembedded.org
> > > <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> > > > 
> > > > We are rewriting the globs variable, so it can't be None, but
> > > > rather a
> > > > string. E.g. the lines above.
> > > > 
> > > > if globs is None:
> > > >     globs = self.d.getVar('IMAGE_INSTALL_COMPLEMENTARY')
> > > > 
> > > > As globs is now a string, we need to test for empty string
> > > > instead
> > > > of
> > > > compare with None.
> > > > 
> > > > Signed-off-by: Claus Stovgaard <claus.stovgaard@gmail.com>
> > > > ---
> > > >  meta/lib/oe/package_manager/__init__.py | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/meta/lib/oe/package_manager/__init__.py
> > > > b/meta/lib/oe/package_manager/__init__.py
> > > > index d3b2317894..1d923c436e 100644
> > > > --- a/meta/lib/oe/package_manager/__init__.py
> > > > +++ b/meta/lib/oe/package_manager/__init__.py
> > > > @@ -365,7 +365,7 @@ class PackageManager(object,
> > > > metaclass=ABCMeta):
> > > >                  for complementary_linguas in
> > > > (self.d.getVar('IMAGE_LINGUAS_COMPLEMENTARY') or "").split():
> > > >                      globs += (" " + complementary_linguas) %
> > > > lang
> > > > 
> > > > -        if globs is None:
> > > > +        if not globs:
> > > >              return
> > > > 
> > > >          # we need to write the list of installed packages to a
> > > > file because the
> > > > --
> > > > 2.45.2
> > > > 
> > > > 
> > > > -=-=-=-=-=-=-=-=-=-=-=-
> > > > Links: You receive all messages sent to this group.
> > > > View/Reply Online (#204970):
> > > > https://lists.openembedded.org/g/openembedded-core/message/204970
> > > > Mute This Topic:
> > > > https://lists.openembedded.org/mt/108664320/1686489
> > > > Group Owner: openembedded-core+owner@lists.openembedded.org
> > > > Unsubscribe:
> > > > https://lists.openembedded.org/g/openembedded-core/unsub [
> > > > alex.kanavin@gmail.com]
> > > > -=-=-=-=-=-=-=-=-=-=-=-
> > > > 
> > 



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-09-26 20:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-26  9:17 [master][scarthgap][PATCH] lib/oe/package-manager: optimize install_complementary Claus Stovgaard
2024-09-26  9:51 ` [OE-core] " Alexander Kanavin
2024-09-26 15:01   ` claus.stovgaard
2024-09-26 15:12     ` Alexander Kanavin
2024-09-26 20:41       ` Claus Stovgaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox