All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS
@ 2020-11-25 13:48 Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot() Peter Kjellerstedt
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-25 13:48 UTC (permalink / raw)
  To: openembedded-core

The changes in this patch series aim to rectify a problem with pseudo
and its support for ignoring paths. When pseudo compares a path to the
paths specified in PSEUDO_IGNORE_PATHS, it uses a path that has been
canonicalized. However, it does not canonicalize the paths in
PSEUDO_IGNORE_PATHS itself and unless they are canonicalized, they
will not match as intended. Thus the paths in PSEUDO_IGNORE_PATHS
needs to be canonicalized. These patches do that by adding a new
PSEUDO_IGNORE_REAL_PATHS variable, which contains the paths from
PSEUDO_IGNORE_PATHS after having run them through os.path.realpath().

There is also one patch that adds two patches to pseudo to clean up
pseudo_client_ignore_path_chroot(), and they also plug a memory leak.
The patches were brought about as I initially intended to do the
canonicalization in pseudo itself in this function.

I have not tested the change to wic as we do not use it, though I do
not expect it to be problematic.

PATCHv2: Removed some unrelated changes in the second patch.

//Peter

The following changes since commit 3ecf5d9692fec97b37af6a4e6c747a4e2c2ca292:

  uninative: Don't use single sstate for pseudo-native (2020-11-24 15:53:07 +0000)

are available in the Git repository at:

  git://push.yoctoproject.org/poky-contrib pkj/pseudo-ignore

Peter Kjellerstedt (4):
  pseudo: Simplify pseudo_client_ignore_path_chroot()
  bitbake.conf: Add all layers (from BBLAYERS) to PSEUDO_IGNORE_PATHS
  bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  wic: Pass canonicalized paths in PSEUDO_IGNORE_PATHS

 meta/classes/image_types_wic.bbclass          |  2 +-
 meta/conf/bitbake.conf                        | 10 +--
 ...ssen-indentation-of-pseudo_client_ig.patch | 69 +++++++++++++++++++
 ...mplify-pseudo_client_ignore_path_chr.patch | 50 ++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  2 +
 scripts/lib/wic/partition.py                  |  2 +-
 6 files changed, 129 insertions(+), 6 deletions(-)
 create mode 100644 meta/recipes-devtools/pseudo/files/0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch
 create mode 100644 meta/recipes-devtools/pseudo/files/0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot()
  2020-11-25 13:48 [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
@ 2020-11-25 13:48 ` Peter Kjellerstedt
  2020-11-30 11:47   ` [OE-core] " Richard Purdie
  2020-11-25 13:48 ` [PATCHv2 2/4] bitbake.conf: Add all layers (from BBLAYERS) to PSEUDO_IGNORE_PATHS Peter Kjellerstedt
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-25 13:48 UTC (permalink / raw)
  To: openembedded-core

This also plugs a memory leak in pseudo_client_ignore_path_chroot().

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 ...ssen-indentation-of-pseudo_client_ig.patch | 69 +++++++++++++++++++
 ...mplify-pseudo_client_ignore_path_chr.patch | 50 ++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  2 +
 3 files changed, 121 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch
 create mode 100644 meta/recipes-devtools/pseudo/files/0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch

diff --git a/meta/recipes-devtools/pseudo/files/0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch b/meta/recipes-devtools/pseudo/files/0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch
new file mode 100644
index 0000000000..e4a5356f5c
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch
@@ -0,0 +1,69 @@
+From 28c760542eecd7c5b35ea88aa2b14d62afbda961 Mon Sep 17 00:00:00 2001
+From: Peter Kjellerstedt <pkj@axis.com>
+Date: Sat, 21 Nov 2020 17:22:38 +0100
+Subject: [PATCH] pseudo_client: Lessen indentation of
+ pseudo_client_ignore_path_chroot()
+
+Change-Id: I739b18efb7a95ce2d2d907d0faf7df539ab9af1c
+---
+ pseudo_client.c | 45 +++++++++++++++++++++++++--------------------
+ 1 file changed, 25 insertions(+), 20 deletions(-)
+
+diff --git a/pseudo_client.c b/pseudo_client.c
+index 116d926..a8bc3dc 100644
+--- a/pseudo_client.c
++++ b/pseudo_client.c
+@@ -1527,28 +1527,33 @@ int pseudo_client_ignore_fd(int fd) {
+ 
+ int pseudo_client_ignore_path_chroot(const char *path, int ignore_chroot) {
+ 	char *env;
+-	if (path) {
+-		if (ignore_chroot && pseudo_chroot && strncmp(path, pseudo_chroot, pseudo_chroot_len) == 0)
+-			return 0;
+-		env = pseudo_get_value("PSEUDO_IGNORE_PATHS");
+-		if (env) {
+-			char *p = env;
+-        	        while (*p) {
+-				char *next = strchr(p, ',');
+-				if (!next)
+-				    next = strchr(p, '\0');
+-				if ((next - p) && !strncmp(path, p, next - p)) {
+-		 			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, "ignoring path: '%s'\n", path);
+-					return 1;
+-				}
+-				if (next && *next != '\0')
+-					p = next+1;
+-				else
+-					break;
+-			}
+-			free(env);
++
++	if (!path)
++		return 0;
++
++	if (ignore_chroot && pseudo_chroot && strncmp(path, pseudo_chroot, pseudo_chroot_len) == 0)
++		return 0;
++
++	env = pseudo_get_value("PSEUDO_IGNORE_PATHS");
++	if (!env)
++		return 0;
++
++	char *p = env;
++	while (*p) {
++		char *next = strchr(p, ',');
++		if (!next)
++			next = strchr(p, '\0');
++		if ((next - p) && !strncmp(path, p, next - p)) {
++ 			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, "ignoring path: '%s'\n", path);
++			return 1;
+ 		}
++		if (next && *next != '\0')
++			p = next+1;
++		else
++			break;
+ 	}
++	free(env);
++
+ 	return 0;
+ }
+ 
diff --git a/meta/recipes-devtools/pseudo/files/0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch b/meta/recipes-devtools/pseudo/files/0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch
new file mode 100644
index 0000000000..be3ab25bd3
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch
@@ -0,0 +1,50 @@
+From a1d61d68777373a50ae23b9dd83b428abe2f748d Mon Sep 17 00:00:00 2001
+From: Peter Kjellerstedt <pkj@axis.com>
+Date: Sat, 21 Nov 2020 17:30:33 +0100
+Subject: [PATCH] pseudo_client: Simplify pseudo_client_ignore_path_chroot()
+
+This also plugs a memory leak by making sure env is freed.
+
+Change-Id: Ia8635fd2c6b1e85919e4743713a85e0b52c28fac
+---
+ pseudo_client.c | 21 ++++++++++-----------
+ 1 file changed, 10 insertions(+), 11 deletions(-)
+
+diff --git a/pseudo_client.c b/pseudo_client.c
+index a8bc3dc..7dc0345 100644
+--- a/pseudo_client.c
++++ b/pseudo_client.c
+@@ -1538,23 +1538,22 @@ int pseudo_client_ignore_path_chroot(const char *path, int ignore_chroot) {
+ 	if (!env)
+ 		return 0;
+ 
++	int ret = 0;
+ 	char *p = env;
+-	while (*p) {
++	while (p) {
+ 		char *next = strchr(p, ',');
+-		if (!next)
+-			next = strchr(p, '\0');
+-		if ((next - p) && !strncmp(path, p, next - p)) {
+- 			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, "ignoring path: '%s'\n", path);
+-			return 1;
+-		}
+-		if (next && *next != '\0')
+-			p = next+1;
+-		else
++		if (next)
++			*next++ = '\0';
++		if (!strncmp(path, p, strlen(p))) {
++			pseudo_debug(PDBGF_PATH | PDBGF_VERBOSE, "ignoring path: '%s'\n", path);
++			ret = 1;
+ 			break;
++		}
++		p = next;
+ 	}
+ 	free(env);
+ 
+-	return 0;
++	return ret;
+ }
+ 
+ int pseudo_client_ignore_path(const char *path) {
diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
index 2e13fec540..50933c4bc1 100644
--- a/meta/recipes-devtools/pseudo/pseudo_git.bb
+++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
@@ -4,6 +4,8 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
            file://0001-configure-Prune-PIE-flags.patch \
            file://fallback-passwd \
            file://fallback-group \
+           file://0002-pseudo_client-Lessen-indentation-of-pseudo_client_ig.patch \
+           file://0003-pseudo_client-Simplify-pseudo_client_ignore_path_chr.patch \
            "
 
 SRCREV = "cca0d7f15b7197095cd587420d31b187620c3093"

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCHv2 2/4] bitbake.conf: Add all layers (from BBLAYERS) to PSEUDO_IGNORE_PATHS
  2020-11-25 13:48 [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot() Peter Kjellerstedt
@ 2020-11-25 13:48 ` Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 4/4] wic: Pass canonicalized " Peter Kjellerstedt
  3 siblings, 0 replies; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-25 13:48 UTC (permalink / raw)
  To: openembedded-core

Instead of ignoring ${COREBASE}/meta in PSEUDO_IGNORE_PATHS (which may
or may not ignore all layers depending on how they are named and placed
under ${COREBASE}), ignore all layers.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 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 0d38eac094..9742fe4fe2 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -685,7 +685,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/,${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,${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,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
 
 export PSEUDO_DISABLED = "1"
 #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-25 13:48 [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot() Peter Kjellerstedt
  2020-11-25 13:48 ` [PATCHv2 2/4] bitbake.conf: Add all layers (from BBLAYERS) to PSEUDO_IGNORE_PATHS Peter Kjellerstedt
@ 2020-11-25 13:48 ` Peter Kjellerstedt
  2020-11-26 14:47   ` [OE-core] " Richard Purdie
  2020-11-25 13:48 ` [PATCHv2 4/4] wic: Pass canonicalized " Peter Kjellerstedt
  3 siblings, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-25 13:48 UTC (permalink / raw)
  To: openembedded-core

Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the canonicalized
paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
oe.path.to_real_paths(). This is needed since pseudo's
pseudo_client_ignore_path_chroot() will compare the ignored paths to
paths that have been canonicalized.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/conf/bitbake.conf | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 9742fe4fe2..4862095a1b 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -685,15 +685,16 @@ SRC_URI = ""
 PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
 PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
 PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
+PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
 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,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
 
 export PSEUDO_DISABLED = "1"
 #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
 #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
 #export PSEUDO_LIBDIR = "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
-FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native} PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
+FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native} PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
 FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
-FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1 PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
+FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1 PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
 FAKEROOTNOENV = "PSEUDO_UNLOAD=1"
 FAKEROOTDIRS = "${PSEUDO_LOCALSTATEDIR}"
 PREFERRED_PROVIDER_virtual/fakeroot-native ?= "pseudo-native"
@@ -884,7 +885,8 @@ BB_HASHEXCLUDE_COMMON ?= "TMPDIR FILE PATH PWD BB_TASKHASH BBPATH BBSERVER DL_DI
     BB_WORKERCONTEXT BB_LIMITEDDEPS BB_UNIHASH extend_recipe_sysroot DEPLOY_DIR \
     SSTATE_HASHEQUIV_METHOD SSTATE_HASHEQUIV_REPORT_TASKDATA \
     SSTATE_HASHEQUIV_OWNER CCACHE_TOP_DIR BB_HASHSERVE GIT_CEILING_DIRECTORIES"
-BB_HASHBASE_WHITELIST ?= "${BB_HASHEXCLUDE_COMMON} PSEUDO_IGNORE_PATHS BUILDHISTORY_DIR SSTATE_DIR "
+BB_HASHBASE_WHITELIST ?= "${BB_HASHEXCLUDE_COMMON} PSEUDO_IGNORE_PATHS \
+    PSEUDO_IGNORE_REAL_PATHS BUILDHISTORY_DIR SSTATE_DIR "
 BB_HASHCONFIG_WHITELIST ?= "${BB_HASHEXCLUDE_COMMON} DATE TIME SSH_AGENT_PID \
     SSH_AUTH_SOCK PSEUDO_BUILD BB_ENV_EXTRAWHITE DISABLE_SANITY_CHECKS \
     PARALLEL_MAKE BB_NUMBER_THREADS BB_ORIGENV BB_INVALIDCONF BBINCLUDED \

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCHv2 4/4] wic: Pass canonicalized paths in PSEUDO_IGNORE_PATHS
  2020-11-25 13:48 [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
                   ` (2 preceding siblings ...)
  2020-11-25 13:48 ` [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
@ 2020-11-25 13:48 ` Peter Kjellerstedt
  3 siblings, 0 replies; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-25 13:48 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/classes/image_types_wic.bbclass | 2 +-
 scripts/lib/wic/partition.py         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image_types_wic.bbclass b/meta/classes/image_types_wic.bbclass
index 286e0f5d54..9c1605c29c 100644
--- a/meta/classes/image_types_wic.bbclass
+++ b/meta/classes/image_types_wic.bbclass
@@ -5,7 +5,7 @@ WICVARS ?= "\
            IMAGE_LINK_NAME IMAGE_ROOTFS INITRAMFS_FSTYPES INITRD INITRD_LIVE ISODIR RECIPE_SYSROOT_NATIVE \
            ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR TARGET_SYS \
            KERNEL_IMAGETYPE MACHINE INITRAMFS_IMAGE INITRAMFS_IMAGE_BUNDLE INITRAMFS_LINK_NAME APPEND \
-           ASSUME_PROVIDED PSEUDO_IGNORE_PATHS"
+           ASSUME_PROVIDED PSEUDO_IGNORE_REAL_PATHS"
 
 inherit ${@bb.utils.contains('INITRAMFS_IMAGE_BUNDLE', '1', 'kernel-artifact-names', '', d)}
 
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index ebe250b00d..e910fd3fa0 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -211,7 +211,7 @@ class Partition():
             pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % pseudo_dir
             pseudo += "export PSEUDO_PASSWD=%s;" % rootfs_dir
             pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
-            pseudo += "export PSEUDO_IGNORE_PATHS=%s;" % (rootfs + "," + (get_bitbake_var("PSEUDO_IGNORE_PATHS") or ""))
+            pseudo += "export PSEUDO_IGNORE_PATHS=%s;" % (os.path.realpath(rootfs) + "," + (get_bitbake_var("PSEUDO_IGNORE_REAL_PATHS") or ""))
             pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
         else:
             pseudo = None

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-25 13:48 ` [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
@ 2020-11-26 14:47   ` Richard Purdie
  2020-11-26 20:38     ` Peter Kjellerstedt
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2020-11-26 14:47 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the canonicalized
> paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
> oe.path.to_real_paths(). This is needed since pseudo's
> pseudo_client_ignore_path_chroot() will compare the ignored paths to
> paths that have been canonicalized.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  meta/conf/bitbake.conf | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

This looks like a good way to fix this, except I have a strong dislike
of "REAL" variables.

> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index 9742fe4fe2..4862095a1b 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -685,15 +685,16 @@ SRC_URI = ""
>  PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
>  PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
>  PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
> +PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
>  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,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
>  
>  export PSEUDO_DISABLED = "1"
>  #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
>  #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
>  #export PSEUDO_LIBDIR = "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
> -FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native} PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
> +FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native} PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
>  FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
> -FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1 PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
> +FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1 PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
> 

I'm wondering if we create a function in lib/oe/utils.py and then use
something like:

PSEUDO_IGNORE_PATHS=${@oe.utils.realpath("PSEUDO_IGNORE_REAL_PATHS")}

?

Cheers,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-26 14:47   ` [OE-core] " Richard Purdie
@ 2020-11-26 20:38     ` Peter Kjellerstedt
  2020-11-26 23:41       ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-26 20:38 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 26 november 2020 15:47
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in
> PSEUDO_IGNORE_PATHS
> 
> On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> > Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the
> canonicalized
> > paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
> > oe.path.to_real_paths(). This is needed since pseudo's
> > pseudo_client_ignore_path_chroot() will compare the ignored paths to
> > paths that have been canonicalized.
> >
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> >  meta/conf/bitbake.conf | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> This looks like a good way to fix this, except I have a strong dislike
> of "REAL" variables.

Well, I named it PSEUDO_IGNORE_REAL_PATHS to match os.path.realpath(). 
I can name it, e.g., PSEUDO_IGNORE_CANONICAL_PATHS if you prefer that?

> 
> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > index 9742fe4fe2..4862095a1b 100644
> > --- a/meta/conf/bitbake.conf
> > +++ b/meta/conf/bitbake.conf
> > @@ -685,15 +685,16 @@ SRC_URI = ""
> >  PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
> >  PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
> >  PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
> > +PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for
> path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
> >  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,${COREBA
> SE}/scripts,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
> >
> >  export PSEUDO_DISABLED = "1"
> >  #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
> >  #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
> >  #export PSEUDO_LIBDIR =
> "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
> > -FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
> > +FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
> >  FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
> > -FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
> > +FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
> >
> 
> I'm wondering if we create a function in lib/oe/utils.py and then use
> something like:
> 
> PSEUDO_IGNORE_PATHS=${@oe.utils.realpath("PSEUDO_IGNORE_REAL_PATHS")}

There already is an oe.path.realpath(). However, I am not sure when 
it is expected to be used instead of os.path.realpath().

I initially created an oe.path.realpaths(d, var, separator=','), but 
after I had rewritten it a couple of times I was down to just the 
single list comprehension and then I felt as I could just as well 
use it directly since I don't see a great need for a common function 
to canonicalize a list of paths. But if you prefer that solution, I 
can restore it.

> 
> ?
> 
> Cheers,
> 
> Richard

//Peter


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-26 20:38     ` Peter Kjellerstedt
@ 2020-11-26 23:41       ` Richard Purdie
  2020-11-30 17:44         ` Peter Kjellerstedt
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2020-11-26 23:41 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Thu, 2020-11-26 at 20:38 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Sent: den 26 november 2020 15:47
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> > core@lists.openembedded.org
> > Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize
> > paths in
> > PSEUDO_IGNORE_PATHS
> > 
> > On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> > > Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the
> > canonicalized
> > > paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
> > > oe.path.to_real_paths(). This is needed since pseudo's
> > > pseudo_client_ignore_path_chroot() will compare the ignored paths
> > > to
> > > paths that have been canonicalized.
> > > 
> > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > ---
> > >  meta/conf/bitbake.conf | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > This looks like a good way to fix this, except I have a strong
> > dislike
> > of "REAL" variables.
> 
> Well, I named it PSEUDO_IGNORE_REAL_PATHS to match
> os.path.realpath(). 
> I can name it, e.g., PSEUDO_IGNORE_CANONICAL_PATHS if you prefer
> that?

My point was less about the name and more that I'd prefer not to have
an indirect variable which isn't useful.

> > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > > index 9742fe4fe2..4862095a1b 100644
> > > --- a/meta/conf/bitbake.conf
> > > +++ b/meta/conf/bitbake.conf
> > > @@ -685,15 +685,16 @@ SRC_URI = ""
> > >  PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
> > >  PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
> > >  PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
> > > +PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for
> > path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
> > >  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,${COREBA
> > SE}/scripts,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
> > >  export PSEUDO_DISABLED = "1"
> > >  #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
> > >  #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
> > >  #export PSEUDO_LIBDIR =
> > "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
> > > -FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
> > > +FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
> > >  FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
> > > -FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
> > > +FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
> > 
> > I'm wondering if we create a function in lib/oe/utils.py and then use
> > something like:
> > 
> > PSEUDO_IGNORE_PATHS=${@oe.utils.realpath("PSEUDO_IGNORE_REAL_PATHS")}
> 
> There already is an oe.path.realpath(). However, I am not sure when 
> it is expected to be used instead of os.path.realpath().

That is a bad choice of name now I look at it! I was thinking more
about the use of a function.

> I initially created an oe.path.realpaths(d, var, separator=','), but 
> after I had rewritten it a couple of times I was down to just the 
> single list comprehension and then I felt as I could just as well 
> use it directly since I don't see a great need for a common function 
> to canonicalize a list of paths. But if you prefer that solution, I 
> can restore it.

I don't really mind the list comprehension directly or a function, I'd
just prefer not to have an indirect variable around to confuse people.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot()
  2020-11-25 13:48 ` [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot() Peter Kjellerstedt
@ 2020-11-30 11:47   ` Richard Purdie
  2020-12-01  1:20     ` Peter Kjellerstedt
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2020-11-30 11:47 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

Hi Peter,

On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> This also plugs a memory leak in pseudo_client_ignore_path_chroot().
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>

I make no claim to understand what happened but I tried patches 1 and 2
in a build over the weekend and with the first two present:

https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/1602
https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/1584
https://autobuilder.yoctoproject.org/typhoon/#/builders/86/builds/1586
https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/1610

I then removed this one leaving only patch 2 and the build was fine.

So there is something odd in this patch but I've not looked into what
or why only selftest is affected.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-26 23:41       ` Richard Purdie
@ 2020-11-30 17:44         ` Peter Kjellerstedt
  2020-12-01  7:58           ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-11-30 17:44 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 27 november 2020 00:42
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in
> PSEUDO_IGNORE_PATHS
> 
> On Thu, 2020-11-26 at 20:38 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > > Sent: den 26 november 2020 15:47
> > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> > > core@lists.openembedded.org
> > > Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize
> > > paths in
> > > PSEUDO_IGNORE_PATHS
> > >
> > > On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> > > > Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the
> > > canonicalized
> > > > paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
> > > > oe.path.to_real_paths(). This is needed since pseudo's
> > > > pseudo_client_ignore_path_chroot() will compare the ignored paths
> > > > to
> > > > paths that have been canonicalized.
> > > >
> > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > > ---
> > > >  meta/conf/bitbake.conf | 8 +++++---
> > > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > This looks like a good way to fix this, except I have a strong
> > > dislike
> > > of "REAL" variables.
> >
> > Well, I named it PSEUDO_IGNORE_REAL_PATHS to match
> > os.path.realpath().
> > I can name it, e.g., PSEUDO_IGNORE_CANONICAL_PATHS if you prefer
> > that?
> 
> My point was less about the name and more that I'd prefer not to have
> an indirect variable which isn't useful.
> 
> > > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > > > index 9742fe4fe2..4862095a1b 100644
> > > > --- a/meta/conf/bitbake.conf
> > > > +++ b/meta/conf/bitbake.conf
> > > > @@ -685,15 +685,16 @@ SRC_URI = ""
> > > >  PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
> > > >  PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
> > > >  PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
> > > > +PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for
> > > path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
> > > >  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,${COREBA
> > > SE}/scripts,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
> > > >  export PSEUDO_DISABLED = "1"
> > > >  #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
> > > >  #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
> > > >  #export PSEUDO_LIBDIR =
> > > "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
> > > > -FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
> > > > +FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
> > > >  FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
> > > > -FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
> > > > +FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
> > >
> > > I'm wondering if we create a function in lib/oe/utils.py and then use
> > > something like:
> > >
> > > PSEUDO_IGNORE_PATHS=${@oe.utils.realpath("PSEUDO_IGNORE_REAL_PATHS")}
> >
> > There already is an oe.path.realpath(). However, I am not sure when
> > it is expected to be used instead of os.path.realpath().
> 
> That is a bad choice of name now I look at it! I was thinking more
> about the use of a function.
> 
> > I initially created an oe.path.realpaths(d, var, separator=','), but
> > after I had rewritten it a couple of times I was down to just the
> > single list comprehension and then I felt as I could just as well
> > use it directly since I don't see a great need for a common function
> > to canonicalize a list of paths. But if you prefer that solution, I
> > can restore it.
> 
> I don't really mind the list comprehension directly or a function, I'd
> just prefer not to have an indirect variable around to confuse people.

Ok, I take that to mean that you instead want me to, e.g., use a function 
to canonicalize the paths where ${PSEUDO_IGNORE_REAL_PATHS} is used now? 
I.e., something like this (line broken here for readability):

  FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} \
                 PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} \
                 PSEUDO_PASSWD=${PSEUDO_PASSWD} \
                 PSEUDO_NOSYMLINKEXP=1 \
                 PSEUDO_IGNORE_PATHS=${@oe.paths.canonicalize(d, 'PSEUDO_IGNORE_PATHS')} \
                 PSEUDO_DISABLED=0"
  FAKEROOTENV[vardeps] += "PSEUDO_IGNORE_PATHS"

alternatively:

  FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} \
                 PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} \
                 PSEUDO_PASSWD=${PSEUDO_PASSWD} \
                 PSEUDO_NOSYMLINKEXP=1 \
                 PSEUDO_IGNORE_PATHS=${@oe.paths.canonicalize(d.getVar('PSEUDO_IGNORE_PATHS'))} \
                 PSEUDO_DISABLED=0"

I am not sure whether there is a preference for utility functions 
to take d and the name of a bitbake variable as arguments, or to 
take the variable's value. The former is a little bit shorter, but 
on the other hand it requires the extra vardeps declaration.

There is a minor drawback with this solution, and that is that I 
cannot use that function in wic. So I will use the list comprehension 
directly, with a comment referring to the oe.path.canonicalize() 
function.

> Cheers,
> 
> Richard

//Peter


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot()
  2020-11-30 11:47   ` [OE-core] " Richard Purdie
@ 2020-12-01  1:20     ` Peter Kjellerstedt
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Kjellerstedt @ 2020-12-01  1:20 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: den 30 november 2020 12:47
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCHv2 1/4] pseudo: Simplify
> pseudo_client_ignore_path_chroot()
> 
> Hi Peter,
> 
> On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> > This also plugs a memory leak in pseudo_client_ignore_path_chroot().
> >
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> 
> I make no claim to understand what happened but I tried patches 1 and 2
> in a build over the weekend and with the first two present:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/1602
> https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/1584
> https://autobuilder.yoctoproject.org/typhoon/#/builders/86/builds/1586
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/1610
> 
> I then removed this one leaving only patch 2 and the build was fine.
> 
> So there is something odd in this patch but I've not looked into what
> or why only selftest is affected.
> 
> Cheers,
> 
> Richard

I believe I have found the problem. I had missed the edge case where 
two commas are specified immediately after each other, or there is a
leading/trailing comma. This would effectively cause pseudo to ignore 
all paths. Thankfully the fix is trivial.

I have verified that I could repeat the oe-selftest problems for the 
wic.Wic2 tests, and that they are back to working with my updated 
code.

I will wait with sending an updated patch series until I have a 
response regarding how you want me to handle the third patch in 
the series (see my other mail from earlier today).

//Peter


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS
  2020-11-30 17:44         ` Peter Kjellerstedt
@ 2020-12-01  7:58           ` Richard Purdie
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Purdie @ 2020-12-01  7:58 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Mon, 2020-11-30 at 17:44 +0000, Peter Kjellerstedt wrote:
> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-
> > core@lists.openembedded.org> On Behalf Of Richard Purdie
> > Sent: den 27 november 2020 00:42
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> > core@lists.openembedded.org
> > Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize paths in
> > PSEUDO_IGNORE_PATHS
> > 
> > On Thu, 2020-11-26 at 20:38 +0000, Peter Kjellerstedt wrote:
> > > > -----Original Message-----
> > > > From: Richard Purdie <richard.purdie@linuxfoundation.org>
> > > > Sent: den 26 november 2020 15:47
> > > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> > > > core@lists.openembedded.org
> > > > Subject: Re: [OE-core] [PATCHv2 3/4] bitbake.conf: Canonicalize
> > > > paths in
> > > > PSEUDO_IGNORE_PATHS
> > > > 
> > > > On Wed, 2020-11-25 at 14:48 +0100, Peter Kjellerstedt wrote:
> > > > > Introduce PSEUDO_IGNORE_REAL_PATHS and make it contain the
> > > > canonicalized
> > > > > paths from PSEUDO_IGNORE_PATHS, obtained by passing the latter to
> > > > > oe.path.to_real_paths(). This is needed since pseudo's
> > > > > pseudo_client_ignore_path_chroot() will compare the ignored paths
> > > > > to
> > > > > paths that have been canonicalized.
> > > > > 
> > > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > > > > ---
> > > > >  meta/conf/bitbake.conf | 8 +++++---
> > > > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > > 
> > > > This looks like a good way to fix this, except I have a strong
> > > > dislike
> > > > of "REAL" variables.
> > > 
> > > Well, I named it PSEUDO_IGNORE_REAL_PATHS to match
> > > os.path.realpath().
> > > I can name it, e.g., PSEUDO_IGNORE_CANONICAL_PATHS if you prefer
> > > that?
> > 
> > My point was less about the name and more that I'd prefer not to have
> > an indirect variable which isn't useful.
> > 
> > > > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > > > > index 9742fe4fe2..4862095a1b 100644
> > > > > --- a/meta/conf/bitbake.conf
> > > > > +++ b/meta/conf/bitbake.conf
> > > > > @@ -685,15 +685,16 @@ SRC_URI = ""
> > > > >  PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
> > > > >  PSEUDO_PASSWD ?= "${STAGING_DIR_TARGET}:${PSEUDO_SYSROOT}"
> > > > >  PSEUDO_SYSROOT = "${COMPONENTS_DIR}/${BUILD_ARCH}/pseudo-native"
> > > > > +PSEUDO_IGNORE_REAL_PATHS = "${@','.join(os.path.realpath(path) for
> > > > path in (d.getVar('PSEUDO_IGNORE_PATHS') or '').split(','))}"
> > > > >  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,${COREBA
> > > > SE}/scripts,${@','.join(d.getVar('BBLAYERS').split())},${CCACHE_DIR}"
> > > > >  export PSEUDO_DISABLED = "1"
> > > > >  #export PSEUDO_PREFIX = "${STAGING_DIR_NATIVE}${prefix_native}"
> > > > >  #export PSEUDO_BINDIR = "${STAGING_DIR_NATIVE}${bindir_native}"
> > > > >  #export PSEUDO_LIBDIR =
> > > > "${STAGING_DIR_NATIVE}$PSEUDOBINDIR/../lib/pseudo/lib
> > > > > -FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > > > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > > > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=1"
> > > > > +FAKEROOTBASEENV = "PSEUDO_BINDIR=${PSEUDO_SYSROOT}${bindir_native}
> > > > PSEUDO_LIBDIR=${PSEUDO_SYSROOT}${prefix_native}/lib/pseudo/lib
> > > > PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=1"
> > > > >  FAKEROOTCMD = "${PSEUDO_SYSROOT}${bindir_native}/pseudo"
> > > > > -FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > > > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_PATHS} PSEUDO_DISABLED=0"
> > > > > +FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native}
> > > > PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR}
> > > > PSEUDO_PASSWD=${PSEUDO_PASSWD} PSEUDO_NOSYMLINKEXP=1
> > > > PSEUDO_IGNORE_PATHS=${PSEUDO_IGNORE_REAL_PATHS} PSEUDO_DISABLED=0"
> > > > 
> > > > I'm wondering if we create a function in lib/oe/utils.py and then use
> > > > something like:
> > > > 
> > > > PSEUDO_IGNORE_PATHS=${@oe.utils.realpath("PSEUDO_IGNORE_REAL_PATHS")}
> > > 
> > > There already is an oe.path.realpath(). However, I am not sure when
> > > it is expected to be used instead of os.path.realpath().
> > 
> > That is a bad choice of name now I look at it! I was thinking more
> > about the use of a function.
> > 
> > > I initially created an oe.path.realpaths(d, var, separator=','), but
> > > after I had rewritten it a couple of times I was down to just the
> > > single list comprehension and then I felt as I could just as well
> > > use it directly since I don't see a great need for a common function
> > > to canonicalize a list of paths. But if you prefer that solution, I
> > > can restore it.
> > 
> > I don't really mind the list comprehension directly or a function, I'd
> > just prefer not to have an indirect variable around to confuse people.
> 
> Ok, I take that to mean that you instead want me to, e.g., use a function 
> to canonicalize the paths where ${PSEUDO_IGNORE_REAL_PATHS} is used now? 
> I.e., something like this (line broken here for readability):
> 
>   FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} \
>                  PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} \
>                  PSEUDO_PASSWD=${PSEUDO_PASSWD} \
>                  PSEUDO_NOSYMLINKEXP=1 \
>                  PSEUDO_IGNORE_PATHS=${@oe.paths.canonicalize(d, 'PSEUDO_IGNORE_PATHS')} \
>                  PSEUDO_DISABLED=0"
>   FAKEROOTENV[vardeps] += "PSEUDO_IGNORE_PATHS"
> 
> alternatively:
> 
>   FAKEROOTENV = "PSEUDO_PREFIX=${PSEUDO_SYSROOT}${prefix_native} \
>                  PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} \
>                  PSEUDO_PASSWD=${PSEUDO_PASSWD} \
>                  PSEUDO_NOSYMLINKEXP=1 \
>                  PSEUDO_IGNORE_PATHS=${@oe.paths.canonicalize(d.getVar('PSEUDO_IGNORE_PATHS'))} \
>                  PSEUDO_DISABLED=0"
> 
> I am not sure whether there is a preference for utility functions 
> to take d and the name of a bitbake variable as arguments, or to 
> take the variable's value. The former is a little bit shorter, but 
> on the other hand it requires the extra vardeps declaration.
> 
> There is a minor drawback with this solution, and that is that I 
> cannot use that function in wic. So I will use the list comprehension 
> directly, with a comment referring to the oe.path.canonicalize() 
> function.

The version which avoids the vardeps is probably slightly neater
overall. I wasn't sure if using the list comprehension here would be
too ugly or not but I'm happy just not to have another variable name
used here, thanks!

The wic issue is unfortunate but hard to avoid.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-12-01  7:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-25 13:48 [PATCHv2 0/4] Support symbolic links in paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
2020-11-25 13:48 ` [PATCHv2 1/4] pseudo: Simplify pseudo_client_ignore_path_chroot() Peter Kjellerstedt
2020-11-30 11:47   ` [OE-core] " Richard Purdie
2020-12-01  1:20     ` Peter Kjellerstedt
2020-11-25 13:48 ` [PATCHv2 2/4] bitbake.conf: Add all layers (from BBLAYERS) to PSEUDO_IGNORE_PATHS Peter Kjellerstedt
2020-11-25 13:48 ` [PATCHv2 3/4] bitbake.conf: Canonicalize paths in PSEUDO_IGNORE_PATHS Peter Kjellerstedt
2020-11-26 14:47   ` [OE-core] " Richard Purdie
2020-11-26 20:38     ` Peter Kjellerstedt
2020-11-26 23:41       ` Richard Purdie
2020-11-30 17:44         ` Peter Kjellerstedt
2020-12-01  7:58           ` Richard Purdie
2020-11-25 13:48 ` [PATCHv2 4/4] wic: Pass canonicalized " Peter Kjellerstedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.