Created attachment 4761 [details] pseudo patch for pseudo_client_ignore_path_chroot During migration from Yocto 3.1.1 to Yocto 3.2 one of our packages failed to build with error ERROR: lpack-20070629-r0 do_package: Error executing a python function in exec_python_func() autogenerated: The stack trace of python calls that resulted in this exception/failure was: File: 'exec_python_func() autogenerated', lineno: 2, function: <module> 0001: *** 0002:perform_packagecopy(d) 0003: File: '/work/yocto/build-yocto/build/../layers/poky/meta/classes/package.bbclass', lineno: 826, function: perform_ packagecopy 0822: rpath_replace (dvar, d) 0823:} 0824:perform_packagecopy[cleandirs] = "${PKGD}" 0825:perform_packagecopy[dirs] = "${PKGD}" *** 0826: 0827:# We generate a master list of directories to process, we start by 0828:# seeding this list with reasonable defaults, then load from 0829:# the fs-perms.txt files 0830:python fixup_perms () { File: '/usr/lib/python3.8/subprocess.py', lineno: 411, function: check_output 0407: # Explicitly passing input=None was previously equivalent to passing an 0408: # empty string. That is maintained here for backwards compatibility. 0409: kwargs['input'] = '' if kwargs.get('universal_newlines', False) else b'' 0410: *** 0411: return run(*popenargs, stdout=PIPE, timeout=timeout, check=True, 0412: **kwargs).stdout 0413: 0414: 0415:class CompletedProcess(object): File: '/usr/lib/python3.8/subprocess.py', lineno: 512, function: run 0508: # We don't call process.wait() as .__exit__ does that for us. 0509: raise 0510: retcode = process.poll() 0511: if check and retcode: *** 0512: raise CalledProcessError(retcode, process.args, 0513: output=stdout, stderr=stderr) 0514: return CompletedProcess(process.args, retcode, stdout, stderr) 0515: 0516: Exception: subprocess.CalledProcessError: Command 'tar -cf - -C /work/yocto/build-yocto/build/tmp/work/cortexa9t2h f-neon-poky-linux-gnueabi/lpack/20070629-r0/image -p -S . | tar -xf - -C /work/yocto/build-yocto/build/tmp/work/co rtexa9t2hf-neon-poky-linux-gnueabi/lpack/20070629-r0/package' returned non-zero exit status 2. Subprocess output: tar: ./usr/lib/lua/5.1/pack.so: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: ./usr/lib/lua/5.1: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: ./usr/lib/lua: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: ./usr/lib: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: ./usr: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: .: Cannot change ownership to uid 0, gid 0: Operation not permitted tar: Exiting with failure status due to previous errors After digging Yocto metadata i found, that problem was in new pseudo variable PSEUDO_IGNORE_PATHS (added in commit 84d006b9fb9c131fb626a090d9e4a76cc47eaf84 in git.yoctoproject.org/poky, branch gatesgarth). To be more precise, PSEUDO_IGNORE_PATHS contains item with ${S} value of the recipe. In our case that was PSEUDO_IGNORE_PATHS = "...,/work/yocto/build-yocto/build/tmp/work/co rtexa9t2hf-neon-poky-linux-gnueabi/lpack/20070629-r0/pack,..." The last path segment here 'pack' was the reason of the error. It was forcing 'package' directory in the recipe WORKDIR to be filtered out and ignored by pseudo. After looking through pseudo sources i found function that actually applies PSEUDO_IGNORE_PATHS - pseudo_client_ignore_path_chroot (added in f3ae0c96e012426a8bcb6f881d1c8c30f9673b14, branch oe-core). In applied patch i suggest fixes of this issue, that distinguish partial path segments and full path segments, so paths '/var/some/path' and '/var/some/pa' are treated as two different paths, and same for '.../recipe-sysroot' - it would not ignore '.../recipe-sysroot-sysroot' anymore. This changes logic of PSEUDO_IGNORE_PATHS usage slightly - now it is required to specify full path to be ignored. That is, some changes would be required for PSEUDO_IGNORE_PATHS definition in bitbake.conf - all paths have to be specified exactly: -PSEUDO_IGNORE_PATHS = "/usr/,/etc/,/lib,/dev/,${T},${WORKDIR}/recipe-sysroot,${SSTATE_DIR},${STAMPS_DIR},${WORKDIR}/pkgdata-sysroot,${TMPDIR}/sstate-control,${DEPLOY_DIR},${WORKDIR}/deploy-,${TMPDIR}/buildstats,${WORKDIR}/sstate-build-package_,${WORKDIR}/sstate-install-package_,${WORKDIR}/sstate-build-image_complete,${TMPDIR}/sysroots-components,${BUILDHISTORY_DIR},${TMPDIR}/pkgdata,${TOPDIR}/cache,${COREBASE}/scripts,${COREBASE}/meta,${CCACHE_DIR}" +PSEUDO_IGNORE_PATHS = "/usr,/etc,/lib,/dev,${T},${WORKDIR}/recipe-sysroot,${WORKDIR}/recipe-sysroot-native,${SSTATE_DIR},${STAMPS_DIR},${WORKDIR}/pkgdata-sysroot,${TMPDIR}/sstate-control,${DEPLOY_DIR},${WORKDIR}/deploy-ipks,${WORKDIR}/deploy-source-date-epoch,${TMPDIR}/buildstats,${WORKDIR}/sstate-install-package_qa,${WORKDIR}/sstate-install-package_write_ipk,${WORKDIR}/sstate-build-image_complete,${TMPDIR}/sysroots-components,${BUILDHISTORY_DIR},${TMPDIR}/pkgdata,${TOPDIR}/cache,${COREBASE}/scripts,${COREBASE}/meta,${CCACHE_DIR}" Here is my questions: 1) feedback about changes in the patch 2) where should i post the patch, as it is not actually oe-core project and project http://git.yoctoproject.org/cgit/cgit.cgi/pseudo is closed for public contributions Posted all this here as was written here http://git.yoctoproject.org/cgit/cgit.cgi/pseudo/tree/README
Dorinda, Is this something you'd like to work on. We need a sanity check to see if ${S} and the paths in pseudo ignore paths overlap and error if they do overlap.
oh cool, i'd love to work on this, let me think of a solution thank you.
Created attachment 4764 [details] pseudo patch for pseudo_client_ignore_path_chroot Found a bug in my own patch. Attached *PATCH v2* to this task
(In reply to comment #1) > Dorinda, Is this something you'd like to work on. > We need a sanity check to see if ${S} and the paths in pseudo ignore paths > overlap and error if they do overlap. Hm, that was an issue after all, that should be fixed, not hidden behind some Yocto heuristic mechanisms. I mean such sanity checks are obviously useful, but only for protection; the issue will still be presented in the code despite those protections. The issue is that pseudo code works unstable while processing PSEUDO_IGNORE_PATHS environment variable. The best way to implement PSEUDO_IGNORE_PATHS application in my opinion would be by using globs, or at most regular expressions. Richard's Purdie decision to just compare strings is arguable, but he is an architect after all, so i just followed his ideas in my patch. But once again, the code works unstable - what if similar bug will appears during some other variable matching, not ${S}? How many variables are there in every package build? And all of them are potentially buggy. Please consider my approach once again. Maybe we should call Richard Purdie for his opinion? I was fixing exactly his code after all.
Hi, you may have a point there i guess but am not very familiar pseudo. Nonetheless i submitted a patch to oe-core mailing list here @ https://lists.openembedded.org/g/openembedded-core/message/147373 although it's still a WIP.
(In reply to comment #5) > Hi, you may have a point there i guess but am not very familiar pseudo. > Nonetheless i submitted a patch to oe-core mailing list here @ > https://lists.openembedded.org/g/openembedded-core/message/147373 although > it's still a WIP. Copying ny reply on mailing list. The point was that ${S}, that is ignored (and should be ignored), is part of some other path, that is NOT in PSEUDO_IGNORE_PATHS and should not be ignored. Example: /some/long/path/and/finally/s_directory -- this is ${S}, that should be ignored; also it is part of PSEUDO_IGNORE_PATHS... and that's why it is actually ignored /some/long/path/and/finally/s_directory_with_some_suffix -- this is some another directory, that should not be ignored, but because it is prefixed by ${S}, it incorrectly is ignored My actual case: S == /<build-root>/build/tmp/work/<SoC-tunes>-poky-linux-gnieabi/lpack/<version>/pack -- it is in PSEUDO_IGNORE_PATHS, and it should be ignored; pay attention to the last path segment, pack /<build-root>/build/tmp/work/<SoC-tunes>-poky-linux-gnieabi/lpack/<version>/package -- this should not have be ignored, but incorrectly ${S} was applied to it, because of the algorithm in pseudo So, what you should do in sanity check is actually check overlaps of ${S} with the list of important directories, that should not be ignored, not with PSEUDO_IGNORE_PATHS. Where to get this list of important directories - that's a question... The only thing i can imagine right now - just hardcode it like shown below. Also, consider that ${S} is only one case of overlapping.
In mailing list https://lists.openembedded.org/g/openembedded-core/topic/patch_sanity_bbclass/80117826 i saw Chen Qi suggested the opposite strategy for pseudo path filtering. If this new strategy would be applied, this task 14193 may be closed. With PSEUDO_CONSIDER_PATHS the problem in this task would be solved anyways - neither mine, nor Dorinda's patch would be necessary.
Rightly or wrongly (and it can be argued either way), PSEUDO_IGNORE_PATHS does simple prefix matching. Whilst we could force that to be more exact path matching, we could lose flexibility and the configuration here wouldn't work since we do use that matching capability to minimise the list of paths needed in PSEUDO_IGNORE_PATHS. I've replied to Dorina's patch on the mailing list detailing the kind of sanity test we need to ensure people at least don't hit problems like reported here and get a better error message explaining the problem. Whether or not we move to the path inclusion method rather than exclusion remains to be seen, it was advised explicitly against by the pseudo authors. Patches for pseudo and discussion can be sent to the mailing list as detailed in the README but its unlikely we'd change the matching mechanism at this point as detailed above. The sanity test is however important to stop people being caught out by this.
Hi All, Patch submitted at https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=785b5489aba6aae6919abdd3faf54abf2439bec6
The sanity test is merged in 6e4bd8cabcdedf4b52345ef5eb421f71d0f19b1d.
Sanity test has been added, not sure we can change the rest of the implementation without causing efficiency issues or breakage with the way it is being used in OE-Core. Path inclusion rather than exclusion may be the other route to explore too, many people are requesting that.