| Message ID | 6d40f0d943ede3088c8a3f77ef2ee1b7442ddda3.1676363203.git.liezhi.yang@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] rootfs.py: Set PACKAGE_FEED_ARCHS when it is not defined | expand |
On Tue, 14 Feb 2023 at 09:28, Robert Yang <liezhi.yang@windriver.com> wrote: > self.pm.insert_feeds_uris(self.d.getVar('PACKAGE_FEED_URIS') or "", > self.d.getVar('PACKAGE_FEED_BASE_PATHS') or "", > - self.d.getVar('PACKAGE_FEED_ARCHS')) > + self._get_feed_archs()) I have to say no to this. All three package manager-specific implementations of this function already handle the case of PACKAGE_FEED_ARCHS not being set. Are they doing the right thing? If they do, then this should not be necessary. If they do not, then the code in those implementations should be fixed,but not overriden like the patch does. Also, rootfs.py should not be mentioning any specific packaging format, that's what those classes are for. Alex
On Tue, 2023-02-14 at 00:28 -0800, Robert Yang wrote: > The PACKAGE_FEED_ARCHS is highly related to ALL_MULTILIB_PACKAGE_ARCHS, set it > automatically is better than manually, for example, we may forget to upgrade > PACKAGE_FEED_ARCHS when ALL_MULTILIB_PACKAGE_ARCHS is changed if set it > manually. > > The workflow is: > Use PACKAGE_FEED_ARCHS if it is defined, if not, check DEPLOY_DIR_XXX/<arch>, add > <arch> to PACKAGE_FEED_ARCHS if it exists. This says what the code does, which I can see from the patch. What this doesn't say is *why*. What issue is this change needed for? When would we currently need to set these things manually? The multilib variables are not really meant to be used in generic code and the multilib classes are meant to to be getting in the way like this. Does this mean multilib is not setting up something correctly? Cheers, Richard
Hi RP and Alexander, On 2/14/23 16:47, Richard Purdie wrote: > On Tue, 2023-02-14 at 00:28 -0800, Robert Yang wrote: >> The PACKAGE_FEED_ARCHS is highly related to ALL_MULTILIB_PACKAGE_ARCHS, set it >> automatically is better than manually, for example, we may forget to upgrade >> PACKAGE_FEED_ARCHS when ALL_MULTILIB_PACKAGE_ARCHS is changed if set it >> manually. >> >> The workflow is: >> Use PACKAGE_FEED_ARCHS if it is defined, if not, check DEPLOY_DIR_XXX/<arch>, add >> <arch> to PACKAGE_FEED_ARCHS if it exists. > > This says what the code does, which I can see from the patch. What this > doesn't say is *why*. > > What issue is this change needed for? When would we currently need to > set these things manually? This is used for online package feeds (When PACKAGE_FEED_URIS is set), the default PACKAGE_FEED_ARCHS is None, so "dnf install/upgrade" or "apt-get update" doesn't work by default, we must set PACKAGE_FEED_ARCHS to make it work, but it's hard to maintain PACKAGE_FEED_ARCHS manually since we can't add more/less archs: 1) When add less archs in PACKAGE_FEED_ARCHS, "dnf update" can't find the missed packages. 2) When add more archs in PACKAGE_FEED_ARCHS, "dnf upgrade" will report "can't find package feed" errors. We must set the accurate package feed archs to make it work correctly, but different builds may have different values since recipes can set PACKAGE_ARCH. Set it automatically (when not set manually) can make it work smoothly. > > The multilib variables are not really meant to be used in generic code > and the multilib classes are meant to to be getting in the way like > this. Does this mean multilib is not setting up something correctly? No, multilib works correctly, I just use it find archs in DEPLOY_DIR. // Robert > > Cheers, > > Richard
On Tue, 14 Feb 2023 at 10:12, Robert Yang <liezhi.yang@windriver.com> wrote: > This is used for online package feeds (When PACKAGE_FEED_URIS is set), the > default PACKAGE_FEED_ARCHS is None, so "dnf install/upgrade" or "apt-get update" > doesn't work by default, we must set PACKAGE_FEED_ARCHS to make it work, but > it's hard to maintain PACKAGE_FEED_ARCHS manually since we can't add more/less > archs: > > 1) When add less archs in PACKAGE_FEED_ARCHS, "dnf update" can't find the missed > packages. > > 2) When add more archs in PACKAGE_FEED_ARCHS, "dnf upgrade" will report > "can't find package feed" errors. > > We must set the accurate package feed archs to make it work correctly, but > different builds may have different values since recipes can set PACKAGE_ARCH. > Set it automatically (when not set manually) can make it work smoothly. Tests for online package feeds exist in meta/lib/oeqa/selftest/cases/runtime_test.py and they work without setting PACKAGE_FEED_ARCHS, so something isn't quite right on your side, or the tests aren't covering your local setup (and maybe need to be extended for it). As I said, you need to tweak existing code that handles empty PACKAGE_FEED_ARCHS, not add a whole another overlay function that clobbers that. Alex
On 2/14/23 18:38, Alexander Kanavin wrote: > On Tue, 14 Feb 2023 at 10:12, Robert Yang <liezhi.yang@windriver.com> wrote: >> This is used for online package feeds (When PACKAGE_FEED_URIS is set), the >> default PACKAGE_FEED_ARCHS is None, so "dnf install/upgrade" or "apt-get update" >> doesn't work by default, we must set PACKAGE_FEED_ARCHS to make it work, but >> it's hard to maintain PACKAGE_FEED_ARCHS manually since we can't add more/less >> archs: >> >> 1) When add less archs in PACKAGE_FEED_ARCHS, "dnf update" can't find the missed >> packages. >> >> 2) When add more archs in PACKAGE_FEED_ARCHS, "dnf upgrade" will report >> "can't find package feed" errors. >> >> We must set the accurate package feed archs to make it work correctly, but >> different builds may have different values since recipes can set PACKAGE_ARCH. >> Set it automatically (when not set manually) can make it work smoothly. > > Tests for online package feeds exist in > meta/lib/oeqa/selftest/cases/runtime_test.py and they work without > setting PACKAGE_FEED_ARCHS, so something isn't quite right on your > side, or the tests aren't covering your local setup (and maybe need to > be extended for it). Thanks, I will look into it on why it works. // Robert > > As I said, you need to tweak existing code that handles empty > PACKAGE_FEED_ARCHS, not add a whole another overlay function that > clobbers that. > > Alex
diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py index 890ba5f039..f26b66ba7a 100644 --- a/meta/lib/oe/rootfs.py +++ b/meta/lib/oe/rootfs.py @@ -89,12 +89,36 @@ class Rootfs(object, metaclass=ABCMeta): def _log_check_error(self): self._log_check_common('error', self.log_check_regex) + def _get_feed_archs(self): + feed_archs = self.d.getVar('PACKAGE_FEED_ARCHS') or '' + if not feed_archs: + bb.note(" Figuring PACKAGE_FEED_ARCHS from ALL_MULTILIB_PACKAGE_ARCHS") + manager = self.d.getVar('ROOTFS_PKGMANAGE') + deploy_dir = '' + if 'rpm' in manager: + deploy_dir = self.d.getVar('DEPLOY_DIR_RPM') + elif 'opkg' in manager: + deploy_dir = self.d.getVar('DEPLOY_DIR_IPK') + elif 'dpkg' in manager: + deploy_dir = self.d.getVar('DEPLOY_DIR_DEB') + else: + bb.warn('Failed to figure out deploy_dir') + return '' + + for arch in self.d.getVar('ALL_MULTILIB_PACKAGE_ARCHS').split(): + if 'rpm' in manager: + arch = arch.replace("-", "_") + arch_path = os.path.join(deploy_dir, arch) + if os.path.exists(arch_path): + feed_archs += " %s" % arch + return feed_archs.strip() + def _insert_feed_uris(self): if bb.utils.contains("IMAGE_FEATURES", "package-management", True, False, self.d): self.pm.insert_feeds_uris(self.d.getVar('PACKAGE_FEED_URIS') or "", self.d.getVar('PACKAGE_FEED_BASE_PATHS') or "", - self.d.getVar('PACKAGE_FEED_ARCHS')) + self._get_feed_archs()) """
The PACKAGE_FEED_ARCHS is highly related to ALL_MULTILIB_PACKAGE_ARCHS, set it automatically is better than manually, for example, we may forget to upgrade PACKAGE_FEED_ARCHS when ALL_MULTILIB_PACKAGE_ARCHS is changed if set it manually. The workflow is: Use PACKAGE_FEED_ARCHS if it is defined, if not, check DEPLOY_DIR_XXX/<arch>, add <arch> to PACKAGE_FEED_ARCHS if it exists. Signed-off-by: Robert Yang <liezhi.yang@windriver.com> --- meta/lib/oe/rootfs.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)