[master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[Claus Stovgaard]

Return early when globs is either None or an empty string. If globs is
an empty string from the self.d.getVar, we should skip the reset of
install_complementary, as the result from processing with empty glob in
oe-pkgdata-util will always be 0 packages to install.

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

Re: [OE-core] [master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[Ross Burton]

On 26 Sep 2024, at 21:40, Claus Stovgaard via lists.openembedded.org <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> 
> Return early when globs is either None or an empty string. If globs is
> an empty string from the self.d.getVar, we should skip the reset of
> install_complementary, as the result from processing with empty glob in
> oe-pkgdata-util will always be 0 packages to install.

This isn’t right: if the globs are empty then we can skip the processing of the globs, but this function also contains the locale archive generation which is then skipped.

Ideally this function is tidied up a little as locale archive generation isn’t really related to complementary installation.

Ross

Re: [OE-core] [master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[claus.stovgaard]

On Mon, 2024-09-30 at 12:33 +0000, Ross Burton wrote:
> On 26 Sep 2024, at 21:40, Claus Stovgaard via lists.openembedded.org
> <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> > 
> > Return early when globs is either None or an empty string. If globs
> > is
> > an empty string from the self.d.getVar, we should skip the reset of
> > install_complementary, as the result from processing with empty
> > glob in
> > oe-pkgdata-util will always be 0 packages to install.
> 
> This isn’t right: if the globs are empty then we can skip the
> processing of the globs, but this function also contains the locale
> archive generation which is then skipped.
> 

Oh yes - you are correct. I focused to much on the top part, and the
with loop, so somehow it slipped. Nicely catch.

I see 3 options.

1) drop this patch, and keep doing the extra work.

2) invert the condition and move the with loop in under this condition

3) split the archiving out in seperate method, and then call it after
install_complementary the places where install_complementary is called
from. This option is what I belive you refering to below.

What do you think is the best options?

/Claus


> Ideally this function is tidied up a little as locale archive
> generation isn’t really related to complementary installation.
> 
> Ross

Re: [OE-core] [master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[Ross Burton]

On 1 Oct 2024, at 08:52, claus.stovgaard@gmail.com wrote:
> 
> On Mon, 2024-09-30 at 12:33 +0000, Ross Burton wrote:
>> On 26 Sep 2024, at 21:40, Claus Stovgaard via lists.openembedded.org
>> <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
>>> 
>>> Return early when globs is either None or an empty string. If globs
>>> is
>>> an empty string from the self.d.getVar, we should skip the reset of
>>> install_complementary, as the result from processing with empty
>>> glob in
>>> oe-pkgdata-util will always be 0 packages to install.
>> 
>> This isn’t right: if the globs are empty then we can skip the
>> processing of the globs, but this function also contains the locale
>> archive generation which is then skipped.
>> 
> 
> Oh yes - you are correct. I focused to much on the top part, and the
> with loop, so somehow it slipped. Nicely catch.
> 
> I see 3 options.
> 
> 1) drop this patch, and keep doing the extra work.
> 
> 2) invert the condition and move the with loop in under this condition
> 
> 3) split the archiving out in seperate method, and then call it after
> install_complementary the places where install_complementary is called
> from. This option is what I belive you refering to below.
> 
> What do you think is the best options?

(3) but moving the logic somewhere so callers don’t need to be updated would be ideal, if possible.  (2) if not.

Ross

Re: [OE-core] [master][scarthgap][PATCH v2] lib/oe/package-manager: return early in install_complementary with empty globs

[Claus Stovgaard]

On Tue, 2024-10-01 at 09:21 +0000, Ross Burton wrote:
> On 1 Oct 2024, at 08:52, claus.stovgaard@gmail.com wrote:
> > 
> > On Mon, 2024-09-30 at 12:33 +0000, Ross Burton wrote:
> > > On 26 Sep 2024, at 21:40, Claus Stovgaard via
> > > lists.openembedded.org
> > > <claus.stovgaard=gmail.com@lists.openembedded.org> wrote:
> > > > 
> > > > Return early when globs is either None or an empty string. If
> > > > globs
> > > > is
> > > > an empty string from the self.d.getVar, we should skip the
> > > > reset of
> > > > install_complementary, as the result from processing with empty
> > > > glob in
> > > > oe-pkgdata-util will always be 0 packages to install.
> > > 
> > > This isn’t right: if the globs are empty then we can skip the
> > > processing of the globs, but this function also contains the
> > > locale
> > > archive generation which is then skipped.
> > > 
> > 
> > Oh yes - you are correct. I focused to much on the top part, and
> > the
> > with loop, so somehow it slipped. Nicely catch.
> > 
> > I see 3 options.
> > 
> > 1) drop this patch, and keep doing the extra work.
> > 
> > 2) invert the condition and move the with loop in under this
> > condition
> > 
> > 3) split the archiving out in seperate method, and then call it
> > after
> > install_complementary the places where install_complementary is
> > called
> > from. This option is what I belive you refering to below.
> > 
> > What do you think is the best options?
> 
> (3) but moving the logic somewhere so callers don’t need to be
> updated would be ideal, if possible.  (2) if not.

I looked to see if I could move the generate_locale_archive logic to
somewhere, where the callers don't need to be updated. I could not find
a place where it would fit, so I will send option 2 as version 3 of the
patch

Regards Claus

> 
> Ross