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

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(-)

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

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

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

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