On 25/01/18 11:58, Martin Hundebøll wrote: > Hi Kristian, > > Thanks for your work on this! > > On 2018-01-25 11:33, Kristian Amlie wrote: >> This is a direct followup from the earlier 6602392db3d39 commit in >> wic. It works more or less the same way: The variable specifies a list >> of directories relative to the root of the rootfs, and these >> directories will be excluded from the resulting rootfs image. If an >> entry ends with a slash, only the contents are omitted, not the >> directory itself. >> >> Since the intended use of the variable is to exclude certain >> directories from the rootfs, and then include said directories in >> other partitions, it is not natural for this variable to be respected >> for image creators that create multi partition images. These can turn >> the feature off locally by defining: >> >> do_image_myfs[respect_exclude_path] = "0" >> >> Specifically, "wic" and "multiubi" come with the feature disabled. >> >> Signed-off-by: Kristian Amlie <[email protected]> >> --- >> meta/classes/image.bbclass | 84 >> +++++++++++++++++++++++++++++++++++- >> meta/classes/image_types.bbclass | 1 + >> meta/classes/image_types_wic.bbclass | 1 + >> 3 files changed, 84 insertions(+), 2 deletions(-) >> >> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass >> index 4531aa2..849a19c 100644 >> --- a/meta/classes/image.bbclass >> +++ b/meta/classes/image.bbclass >> @@ -117,7 +117,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', >> >> >> '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'] >> + 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', >> 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', >> 'REPRODUCIBLE_TIMESTAMP_ROOTFS', >> + 'IMAGE_ROOTFS_EXCLUDE_PATH'] >> variables.extend(rootfs_command_variables(d)) >> variables.extend(variable_depends(d)) >> return " ".join(variables) >> @@ -508,8 +509,9 @@ python () { >> d.setVarFlag(task, 'func', '1') >> d.setVarFlag(task, 'fakeroot', '1') >> - d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' >> set_image_size') >> + d.appendVarFlag(task, 'prefuncs', ' ' + debug + ' >> set_image_size prepare_excluded_directories') >> d.prependVarFlag(task, 'postfuncs', ' create_symlinks') >> + d.appendVarFlag(task, 'postfuncs', ' >> cleanup_excluded_directories') >> d.appendVarFlag(task, 'subimages', ' ' + ' '.join(subimages)) >> d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) >> d.appendVarFlag(task, 'vardepsexclude', 'DATETIME DATE ' + ' >> '.join(vardepsexclude)) >> @@ -518,6 +520,84 @@ python () { >> bb.build.addtask(task, 'do_image_complete', after, d) >> } >> +python prepare_excluded_directories() { >> + exclude_var = d.getVar('IMAGE_ROOTFS_EXCLUDE_PATH') >> + if not exclude_var: >> + return >> + >> + taskname = d.getVar("BB_CURRENTTASK") >> + >> + if d.getVarFlag('do_%s' % taskname, 'respect_exclude_path') == '0': >> + bb.debug(1, "'IMAGE_ROOTFS_EXCLUDE_PATH' is set but >> 'respect_exclude_path' variable flag is 0 for this image type, so >> ignoring it") >> + return > > If I understand this correctly, paths like "/data" aren't allowed in > "IMAGE_ROOTFS_EXCLUDE_PATH". That seems counter-intuitive to me, as I > would expect this feature to take absolute paths inside the target image > to be excluded.
My initial idea was absolute paths, but other maintainers disagreed [1]. I don't think it makes sense to change this now, because it has already been included in wic for a long time, and this patch is just mirroring wic's behavior. [1] http://lists.openembedded.org/pipermail/openembedded-core/2016-November/129301.html > If the check is there to avoid "os.path.join()" returning the RHS only > instead of the joined path, then please use "oe.path.join()" instead. Oh, well spotted, I didn't know about oe.path.join(). Strictly speaking it is not necessary since we require a relative path, but it is an extra layer of safety, so yeah, I'll change it to use oe.path.join(). I also noticed there was an unneeded os.path.join() at the rmtree (it joined one single string only), so I removed that one. Rebased patch included with the changes. >> + import shutil >> + from oe.path import copyhardlinktree >> + >> + exclude_list = exclude_var.split() >> + >> + rootfs_orig = d.getVar('IMAGE_ROOTFS') >> + # We need a new rootfs directory we can delete files from. Copy to >> + # workdir. >> + new_rootfs = os.path.realpath(os.path.join(d.getVar("WORKDIR"), >> "rootfs.%s" % taskname)) >> + >> + if os.path.lexists(new_rootfs): >> + shutil.rmtree(os.path.join(new_rootfs)) > > Wouldn't it be sufficient to add "new_rootfs" to "do_image[cleandirs]" ? The directory depends on the task name, so this would be tricky I think. You'd have to know the name upfront, which we don't, since image types can be created by downstream layers. >> ...>> +python cleanup_excluded_directories() { >> + exclude_var = d.getVar('IMAGE_ROOTFS_EXCLUDE_PATH') >> + if not exclude_var: >> + return >> + >> + taskname = d.getVar("BB_CURRENTTASK") >> + >> + if d.getVarFlag('do_%s' % taskname, 'respect_exclude_path') == '0': >> + return >> + >> + import shutil >> + >> + rootfs_dirs_excluded = d.getVar('IMAGE_ROOTFS') >> + rootfs_orig = d.getVar('IMAGE_ROOTFS_ORIG') >> + # This should never happen, since we should have set it to a >> different >> + # directory in the prepare function. >> + assert rootfs_dirs_excluded != rootfs_orig >> + >> + shutil.rmtree(rootfs_dirs_excluded) >> + d.setVar('IMAGE_ROOTFS', rootfs_orig) >> +} > > Are we sure the cleanup is needed? I can image cases where I would like > to have a look in "rootfs_dirs_excluded" to see what how it differs from > "rootfs_orig". I suppose we could. Not sure what weighs heavier, cleaning up or being able to debug. I think personally I vote for cleaning up, since it would be easy to comment this out in a debugging situation. But I can change it if you insist. -- Kristian -- _______________________________________________ Openembedded-core mailing list [email protected] http://lists.openembedded.org/mailman/listinfo/openembedded-core
