Hoi Richard, and thanks for the feedback!
>> archive the package database after the rootfs has been put together as >> *rootfs-pkdbfs.tar.gz, and put it into the deploy folder. >> >> This creates a snapshot of the package mangers state at the point in time >> when >> all dependencies have been resolved and installed; which can be used by >> "follow >> up" images to be built upon. > >I'm torn on this series. On the one hand I can see why it might be >useful. On the other hand: > >* no test cases for it >* no documentation updates >* no real indications in the code on what it is doing (no comments) > point taken, i'll look into the test framework and add something with V3 and i missed the in-source docs... also sth for V3 i'll redistribute the example/documentation from the cover letter amongst the meta/conf/documentation.conf for the variables, and in-source comments any other place in this repo i should be aware off/add documentation to? > >It also copies and pastes a lot of the debugfs code and duplicates it >which makes me wonder if there isn't something better we should be >doing here. the copy&paste comes from the similar steps having to be taken at around the same time. could you elaborate on "something better"? taking different steps, de-duplicating code? ...? > >There is good info in the 0/3 series email but that will get lost once >things merge. good point - i'll incorporate more from the cover-letter into the code itself with the next version. > >> Signed-off-by: Johannes Schneider <[email protected]> >> --- >> meta/classes-recipe/image.bbclass | 45 ++++++++++++++++++++++- >> meta/classes-recipe/image_types.bbclass | 1 + >> meta/conf/bitbake.conf | 1 + >> meta/lib/oe/package_manager/deb/rootfs.py | 1 + >> meta/lib/oe/package_manager/ipk/rootfs.py | 1 + >> meta/lib/oe/package_manager/rpm/rootfs.py | 1 + >> meta/lib/oe/rootfs.py | 35 ++++++++++++++++++ >> 7 files changed, 83 insertions(+), 2 deletions(-) >> >> diff --git a/meta/classes-recipe/image.bbclass >> b/meta/classes-recipe/image.bbclass >> index 28be6c6362..c688c39f15 100644 >> --- a/meta/classes-recipe/image.bbclass >> +++ b/meta/classes-recipe/image.bbclass >> @@ -42,6 +42,9 @@ IMAGE_FEATURES ?= "" >> IMAGE_FEATURES[type] = "list" >> IMAGE_FEATURES[validitems] += "debug-tweaks read-only-rootfs >> read-only-rootfs-delayed-postinsts stateless-rootfs empty-root-password >> allow-empty-password allow-root-login serial-autologin-root >> post-install-logging overlayfs-etc" >> >> +# Generate snapshot of the package database? >> +IMAGE_GEN_PKGDBFS ?= "0" >> + >> # Generate companion debugfs? >> IMAGE_GEN_DEBUGFS ?= "0" >> >> @@ -131,7 +134,8 @@ def rootfs_variables(d): >> >> 'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS', >> 'IMAGE_LINGUAS_COMPLEMENTARY', 'IMAGE_LOCALES_ARCHIVE', >> >> 'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS', >> >> 'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS', >> - 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', >> 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', >> 'REPRODUCIBLE_TIMESTAMP_ROOTFS', 'IMAGE_INSTALL_DEBUGFS'] >> + 'CONVERSIONTYPES', 'IMAGE_GEN_PKGDBFS', >> 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', >> 'PACKAGE_EXCLUDE_COMPLEMENTARY', 'REPRODUCIBLE_TIMESTAMP_ROOTFS', >> + 'IMAGE_INSTALL_DEBUGFS'] >> variables.extend(rootfs_command_variables(d)) >> variables.extend(variable_depends(d)) >> return " ".join(variables) >> @@ -337,6 +341,19 @@ python do_image_qa_setscene () { >> } >> addtask do_image_qa_setscene >> >> +def setup_pkgdbfs_variables(d): >> + d.appendVar('IMAGE_ROOTFS', '-pkgdb') >> + if d.getVar('IMAGE_LINK_NAME'): >> + d.appendVar('IMAGE_LINK_NAME', '-pkgdb') >> + d.appendVar('IMAGE_NAME','-pkgdb') >> + pkgdbfs_image_fstypes = d.getVar('IMAGE_FSTYPES_PKGDBFS') >> + if pkgdbfs_image_fstypes: >> + d.setVar('IMAGE_FSTYPES', pkgdbfs_image_fstypes) >> + >> +python setup_pkgdbfs () { >> + setup_pkgdbfs_variables(d) >> +} >> + >> def setup_debugfs_variables(d): >> d.appendVar('IMAGE_ROOTFS', '-dbg') >> if d.getVar('IMAGE_LINK_NAME'): >> @@ -381,6 +398,11 @@ python () { >> alltypes = d.getVar('IMAGE_FSTYPES').split() >> typedeps = {} >> >> + if d.getVar('IMAGE_GEN_PKGDBFS') == "1": >> + pkgdbfs_fstypes = d.getVar('IMAGE_FSTYPES_PKGDBFS').split() >> + for t in pkgdbfs_fstypes: >> + alltypes.append("pkgdbfs_" + t) >> + >> if d.getVar('IMAGE_GEN_DEBUGFS') == "1": >> debugfs_fstypes = d.getVar('IMAGE_FSTYPES_DEBUGFS').split() >> for t in debugfs_fstypes: >> @@ -393,6 +415,10 @@ python () { >> basetypes[baset]= [] >> if t not in basetypes[baset]: >> basetypes[baset].append(t) >> + pkgdb = "" >> + if t.startswith("pkgdbfs_"): >> + t = t[8:] >> + pkgdb = "pkgdbfs_" >> debug = "" >> if t.startswith("debugfs_"): >> t = t[8:] >> @@ -401,6 +427,13 @@ python () { >> vardeps.add('IMAGE_TYPEDEP:' + t) >> if baset not in typedeps: >> typedeps[baset] = set() >> + deps = [pkgdb + dep for dep in deps] >> + for dep in deps: >> + if dep not in alltypes: >> + alltypes.append(dep) >> + _add_type(dep) >> + basedep = _image_base_type(dep) >> + typedeps[baset].add(basedep) >> deps = [debug + dep for dep in deps] >> for dep in deps: >> if dep not in alltypes: >> @@ -419,6 +452,7 @@ python () { >> >> maskedtypes = (d.getVar('IMAGE_TYPES_MASKED') or "").split() >> maskedtypes = [dbg + t for t in maskedtypes for dbg in ("", "debugfs_")] >> + maskedtypes = [pkgdb + t for t in maskedtypes for pkgdb in ("", >> "pkgdbfs_")] >> >> for t in basetypes: >> vardeps = set() >> @@ -430,6 +464,11 @@ python () { >> continue >> >> localdata = bb.data.createCopy(d) >> + pkgdb = "" >> + if t.startswith("pkgdbfs_"): >> + setup_pkgdbfs_variables(localdata) >> + pkgdb = "setup_pkgdbfs " >> + realt = t[8:] >> debug = "" >> if t.startswith("debugfs_"): >> setup_debugfs_variables(localdata) >> @@ -468,6 +507,8 @@ python () { >> for ctype in sorted(ctypes): >> if bt.endswith("." + ctype): >> type = bt[0:-len(ctype) - 1] >> + if type.startswith("pkgdbfs_"): >> + type = type[8:] >> if type.startswith("debugfs_"): >> type = type[8:] >> # Create input image first. >> @@ -508,7 +549,7 @@ python () { >> d.setVarFlag(task, 'func', '1') >> d.setVarFlag(task, 'fakeroot', '1') >> >> - d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' set_image_size') >> + d.appendVarFlag(task, 'prefuncs', ' ' + debug + pkgdb + ' >> set_image_size') >> d.prependVarFlag(task, 'postfuncs', 'create_symlinks ') >> d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages)) >> d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) > >The above is the particular piece of copy and paste of the debugfs code >which worries me a bit. can you elaborate on how/why? those two variables are set either/or, or neither; and this way the additional task(?) is weaved in exactly once TBH i didn't quite get why it is done this way for the debugfs... but the addition still works with both on, so... open for suggestions on a better way :-) > > >> diff --git a/meta/classes-recipe/image_types.bbclass >> b/meta/classes-recipe/image_types.bbclass >> index 3733bdfc20..03d8852aed 100644 >> --- a/meta/classes-recipe/image_types.bbclass >> +++ b/meta/classes-recipe/image_types.bbclass >> @@ -25,6 +25,7 @@ def imagetypes_getdepends(d): >> >> fstypes = set((d.getVar('IMAGE_FSTYPES') or "").split()) >> fstypes |= set((d.getVar('IMAGE_FSTYPES_DEBUGFS') or "").split()) >> + fstypes |= set((d.getVar('IMAGE_FSTYPES_PKGDBFS') or "").split()) >> >> deprecated = set() >> deps = set() >> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf >> index 6f180d18b0..b03eb80c0b 100644 >> --- a/meta/conf/bitbake.conf >> +++ b/meta/conf/bitbake.conf >> @@ -844,6 +844,7 @@ include conf/bblock.conf >> DL_DIR ?= "${TOPDIR}/downloads" >> SSTATE_DIR ?= "${TOPDIR}/sstate-cache" >> IMAGE_FSTYPES ?= "tar.gz" >> +IMAGE_FSTYPES_PKGDBFS ?= "tar.gz" >> IMAGE_FSTYPES_DEBUGFS ?= "tar.gz" > >bitbake.conf variables go everywhere even for non-images. Whilst I >appreciate the others are there, mainly for historical reasons I'd >prefer not to add to things if we can help it. I keep thinking we >should really have a dedicated conf/imagevars.conf or similar file. ok, so those are already marked as "legacy" and await refactoring ... which is a bit beyond the scope of this feature PR :-S but as noted in the response to your other mail - having the FSTYPE configurable might not be needed in the first place - which would sidestep this issue. > >> INITRAMFS_FSTYPES ?= "cpio.gz" >> diff --git a/meta/lib/oe/package_manager/deb/rootfs.py >> b/meta/lib/oe/package_manager/deb/rootfs.py >> index 1e25b64ed9..43107c8663 100644 >> --- a/meta/lib/oe/package_manager/deb/rootfs.py >> +++ b/meta/lib/oe/package_manager/deb/rootfs.py >> @@ -178,6 +178,7 @@ class PkgRootfs(DpkgOpkgRootfs): >> if self.progress_reporter: >> self.progress_reporter.next_stage() >> >> + self._setup_pkg_db_rootfs(['/var/lib/dpkg']) >> self._setup_dbg_rootfs(['/var/lib/dpkg']) >> >> self.pm.fix_broken_dependencies() >> diff --git a/meta/lib/oe/package_manager/ipk/rootfs.py >> b/meta/lib/oe/package_manager/ipk/rootfs.py >> index ba93eb62ea..64d9bc7969 100644 >> --- a/meta/lib/oe/package_manager/ipk/rootfs.py >> +++ b/meta/lib/oe/package_manager/ipk/rootfs.py >> @@ -319,6 +319,7 @@ class PkgRootfs(DpkgOpkgRootfs): >> >> opkg_lib_dir = self.d.getVar('OPKGLIBDIR') >> opkg_dir = os.path.join(opkg_lib_dir, 'opkg') >> + self._setup_pkg_db_rootfs([opkg_dir]) >> self._setup_dbg_rootfs([opkg_dir]) >> >> execute_pre_post_process(self.d, opkg_post_process_cmds) >> diff --git a/meta/lib/oe/package_manager/rpm/rootfs.py >> b/meta/lib/oe/package_manager/rpm/rootfs.py >> index 3ba5396320..673006c131 100644 >> --- a/meta/lib/oe/package_manager/rpm/rootfs.py >> +++ b/meta/lib/oe/package_manager/rpm/rootfs.py >> @@ -110,6 +110,7 @@ class PkgRootfs(Rootfs): >> if self.progress_reporter: >> self.progress_reporter.next_stage() >> >> + self._setup_pkg_db_rootfs(['/etc/rpm', '/etc/rpmrc', '/etc/dnf', >> '/var/lib/rpm', '/var/cache/dnf', '/var/lib/dnf']) >> self._setup_dbg_rootfs(['/etc/rpm', '/etc/rpmrc', '/etc/dnf', >> '/var/lib/rpm', '/var/cache/dnf', '/var/lib/dnf']) >> >> execute_pre_post_process(self.d, rpm_post_process_cmds) >> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py >> index 8cd48f9450..9268a02531 100644 >> --- a/meta/lib/oe/rootfs.py >> +++ b/meta/lib/oe/rootfs.py >> @@ -106,6 +106,41 @@ class Rootfs(object, metaclass=ABCMeta): >> def _cleanup(self): >> pass >> >> + def _setup_pkg_db_rootfs(self, package_paths): >> + gen_pkg_db_fs = self.d.getVar('IMAGE_GEN_PKGDBFS') or '0' >> + if gen_pkg_db_fs != '1': >> + return > >bb.utils.to_boolean(self.d.getVar('IMAGE_GEN_PKGDBFS')) > thnx: added for V3 > >> + >> + bb.note(" Renaming the original rootfs...") >> + try: >> + shutil.rmtree(self.image_rootfs + '-orig') >> + except: >> + pass > >Please don't use non-specific except: blocks, look for specific >exceptions. > oriented myself on the preexisting code for debugfs - but i'll improve it :-) > >> + bb.utils.rename(self.image_rootfs, self.image_rootfs + '-orig') >> + >> + bb.note(" Creating pkg-db rootfs...") >> + bb.utils.mkdirhier(self.image_rootfs) >> + >> + bb.note(" Copying back package database...") >> + for path in package_paths: >> + bb.utils.mkdirhier(self.image_rootfs + os.path.dirname(path)) >> + if os.path.isdir(self.image_rootfs + '-orig' + path): >> + shutil.copytree(self.image_rootfs + '-orig' + path, >> self.image_rootfs + path, symlinks=True) >> + elif os.path.isfile(self.image_rootfs + '-orig' + path): >> + shutil.copyfile(self.image_rootfs + '-orig' + path, >> self.image_rootfs + path) >> + >> + #### >> + >> + bb.note(" Rename pkg-db rootfs...") >> + try: >> + shutil.rmtree(self.image_rootfs + '-pkgdb') >> + except: >> + pass > >Similar to above. > >> + bb.utils.rename(self.image_rootfs, self.image_rootfs + '-pkgdb') >> + >> + bb.note(" Restoring original rootfs...") >> + bb.utils.rename(self.image_rootfs + '-orig', self.image_rootfs) >> + > >Why does this function need to rename the original directory, create a new one, >then move the other one back? Can't it just create what it needs in the other >path? > heh - true... after the original debugfs-copy was stripped down to this, i overlooked that it could be simplified this way -> adding it to V3 > >Cheers, > >Richard > > > Gruß Johannes
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#195904): https://lists.openembedded.org/g/openembedded-core/message/195904 Mute This Topic: https://lists.openembedded.org/mt/104329518/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
