Re: [OE-core] [PATCH 2/2] lib/oe: Split package manager code to multiple files

["Fredrik Gustafsson" <fredrik.gustafsson@axis.com>]

________________________________________
From: Richard Purdie <richard.purdie@linuxfoundation.org>
Sent: Tuesday, June 23, 2020 2:02 PM
To: Fredrik Gustafsson; openembedded-core@lists.openembedded.org
Cc: tools-cfpbuild-internal; Hugo Cedervall; Fredrik Gustafsson
Subject: Re: [OE-core] [PATCH 2/2] lib/oe: Split package manager code to multiple files

On Tue, 2020-06-23 at 13:13 +0200, Fredrik Gustafsson wrote:
> Today OE-Core has support for three package managers, even if there
> are many more package managers that could be interesting to use.
> Adding a new package manager to OE-Core would increase the test matrix
> significantly. In order to let users use their favorite package manager
> without need for it to have support from upstream, this patch refactors
> the package manager code so that it's easy to add a new package
> manager in another layer.
>
> This patch is mostly moving code and duplicating some code (because of
> the hard coupling between deb and ipk).
>
> Signed-off-by: Fredrik Gustafsson <fredrigu@axis.com>
> ---
>  meta/classes/populate_sdk_base.bbclass        |   11 +-
>  meta/lib/oe/manifest.py                       |  149 +-
>  meta/lib/oe/package_manager.py                | 1402 +----------------
>  meta/lib/oe/package_managers/deb/manifest.py  |   31 +
>  .../package_managers/deb/package_manager.py   |  719 +++++++++
>  meta/lib/oe/package_managers/deb/rootfs.py    |  213 +++
>  meta/lib/oe/package_managers/deb/sdk.py       |   97 ++
>  .../ipk/.package_manager.py.swo               |  Bin 0 -> 45056 bytes
>  meta/lib/oe/package_managers/ipk/manifest.py  |   79 +
>  .../package_managers/ipk/package_manager.py   |  588 +++++++
>  meta/lib/oe/package_managers/ipk/rootfs.py    |  395 +++++
>  meta/lib/oe/package_managers/ipk/sdk.py       |  104 ++
>  meta/lib/oe/package_managers/rpm/manifest.py  |   62 +
>  .../package_managers/rpm/package_manager.py   |  423 +++++
>  meta/lib/oe/package_managers/rpm/rootfs.py    |  154 ++
>  meta/lib/oe/package_managers/rpm/sdk.py       |  104 ++
>  meta/lib/oe/rootfs.py                         |  631 +-------
>  meta/lib/oe/sdk.py                            |  311 +---
>  meta/lib/oeqa/utils/package_manager.py        |   29 +-
>  meta/recipes-core/meta/package-index.bb       |    2 +-
>  20 files changed, 3014 insertions(+), 2490 deletions(-)

I'm afraid this patch is basically impossible to review. There are
warning signs like the inclusion of the binary swo file. The hint that
code gets duplicated also worries me, even if you abstract this, there
shouldn't be reason to do that.

If we do this its going to have to be a more granular series where the
patches *just* move code to new locations so we can see/understand what
other changes are being made as I simply don't trust this patch, sorry
:(.

Cheers,

Richard


Hi,
sorry about the swo file that's a mistake on my side. The duplicated code is simply so that deb and ipk shares a class. So my choice was to either duplicate that class (and let them diverge which is fine) or in the core package_manager.py file have a class that is not used by all package managers (rpm doesn't use it).

This patch is only code moves and class renames. I know it's very hard to review, that's a real problem. Let me see if I can split it up some more.

BR
Fredrik