On Wed, 2021-01-20 at 11:27 +0000, Peter Kjellerstedt wrote: > > -----Original Message----- > > From: [email protected] <openembedded- > > [email protected]> On Behalf Of Yu, Mingli > > Sent: den 20 januari 2021 08:05 > > To: [email protected] > > Subject: [OE-core] [PATCH v2] bitbake.conf: use ${RECIPE_SYSROOT} > > for PSEUDO_IGNORE_PATHS > > > > From: Mingli Yu <[email protected]> > > [cut] > > > Signed-off-by: Mingli Yu <[email protected]> > > --- > > meta/conf/bitbake.conf | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf > > index af1b3b8c3e..312b544d41 100644 > > --- a/meta/conf/bitbake.conf > > +++ b/meta/conf/bitbake.conf > > @@ -686,7 +686,7 @@ SRC_URI = "" > > PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/" > > PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}" > > PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native" > > -PSEUDO_IGNORE_PATHS = > > "/usr/,/etc/,/lib,/dev/,/run/,${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,${CCACHE_DIR}" > > +PSEUDO_IGNORE_PATHS = > > "/usr/,/etc/,/lib,/dev/,/run/,${T},${RECIPE_SYSROOT},${RECIPE_SYSRO > > OT_NATIVE},${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,${CCACHE_DIR}" > > This is not related to the actual change of the above patch, but > rather > triggered by it. Given how long the above lines are, and the fact > that > it is nearly impossible to see what paths actually make them up and > what is being changed, would it make sense to reformat the definition > of PSEUDO_IGNORE_PATHS like this: > > PSEUDO_IGNORE_PATHS = "" > PSEUDO_IGNORE_PATHS .= "/usr/," > PSEUDO_IGNORE_PATHS .= "/etc/," > PSEUDO_IGNORE_PATHS .= "/lib," > PSEUDO_IGNORE_PATHS .= "/dev/," > PSEUDO_IGNORE_PATHS .= "/run/," > PSEUDO_IGNORE_PATHS .= "${T}," > PSEUDO_IGNORE_PATHS .= "${RECIPE_SYSROOT}," > PSEUDO_IGNORE_PATHS .= "${RECIPE_SYSROOT_NATIVE}," > PSEUDO_IGNORE_PATHS .= "${SSTATE_DIR}," > PSEUDO_IGNORE_PATHS .= "${STAMPS_DIR}," > PSEUDO_IGNORE_PATHS .= "${WORKDIR}/pkgdata-sysroot," > PSEUDO_IGNORE_PATHS .= "${TMPDIR}/sstate-control," > PSEUDO_IGNORE_PATHS .= "${DEPLOY_DIR}," > PSEUDO_IGNORE_PATHS .= "${WORKDIR}/deploy-," > PSEUDO_IGNORE_PATHS .= "${TMPDIR}/buildstats," > PSEUDO_IGNORE_PATHS .= "${WORKDIR}/sstate-build-package_," > PSEUDO_IGNORE_PATHS .= "${WORKDIR}/sstate-install-package_," > PSEUDO_IGNORE_PATHS .= "${WORKDIR}/sstate-build-image_complete," > PSEUDO_IGNORE_PATHS .= "${TMPDIR}/sysroots-components," > PSEUDO_IGNORE_PATHS .= "${BUILDHISTORY_DIR}," > PSEUDO_IGNORE_PATHS .= "${TMPDIR}/pkgdata," > PSEUDO_IGNORE_PATHS .= "${TOPDIR}/cache," > PSEUDO_IGNORE_PATHS .= "${COREBASE}/scripts," > PSEUDO_IGNORE_PATHS .= "${CCACHE_DIR}," > > If this is acceptable, then I can send a patch to do it.
One reason we've not done that is primarily parse overhead, this adds both more to parse and makes the variable history a lot more complex. I do try and keep half an eye on the performance situation as whilst one change might not make that much of a difference, it does all add up and we have a nightmare there already. I do admit the variable is a lot more complex than I'd originally thought/hoped. I am also hoping that once we do get this sorted, it becomes something we don't have to alter often. > I would prefer to add that comma to the last line so that all > lines follow the same format, which avoids having to modify > multiple lines when adding/removing entries. I have verified the > code in pseudo and it is ok to have a trailing comma (or multiple > consecutive commas for that matter). The recently added > oe.path.canonicalize() function needs to be modified to skip > empty paths as well (no idea why I missed that when I added it). I'm not sure the trailing comma is a good idea to encourage like this as it will cause code churn, it also implies CWD may be in the ignore path due to the behaviour of PATH (although ignore does work differently for good reason). > That leaves a question of what to do with other classes that add > to the PSEUDO_IGNORE_PATHS variable. They currently typically do > PSEUDO_IGNORE_PATHS .= ",foo", but if a trailing comma is added > above, it would be more appropriate to change them to do > PSEUDO_IGNORE_PATHS .= "foo,". They will continue to work as they > are, however, there is the risk that someone wanting to add to the > PSEUDO_IGNORE_PATHS variable in their own layer looks at the above > code and then adds something like PSEUDO_IGNORE_PATHS_append = "foo," > without realizing there are other classes adding paths using ",foo". > My guess is that there are not many users of PSEUDO_IGNORE_PATHS > outside of OE-Core yet that we can still change this without > adverse effects. I think there are deeper issues this patch hints at with the code/approach itself and I worry this is going to distract from those issues :( With the queued patches to add this to dunfell, I'm also concerned in that direction. Cheers, Richard
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#147032): https://lists.openembedded.org/g/openembedded-core/message/147032 Mute This Topic: https://lists.openembedded.org/mt/79976352/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
