diff mbox series

[3/3] shadow: Drop pointless patch

Message ID 20260421104147.1994119-3-richard.purdie@linuxfoundation.org
State Under Review
Headers show
Series [1/3] pseudo: Update to 1.7.4 | expand

Commit Message

Richard Purdie April 21, 2026, 10:41 a.m. UTC
As far as I can tell, this patch is a no-op and doens't change the code.
As such, I think it just complicates things and can be removed.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 ...nexpected-open-failure-in-chroot-env.patch | 48 -------------------
 meta/recipes-extended/shadow/shadow_4.19.4.bb |  1 -
 2 files changed, 49 deletions(-)
 delete mode 100644 meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch

Comments

ChenQi April 22, 2026, 5:51 a.m. UTC | #1
Hi Richard,

I'm the author of the original patch, so I just double checked it.
This might be counter-intuitive, but this patch does solve some problem 
in pseudo environment.

Without this patch, running a simple 'bitbake ptest-runner' fails. The 
error message is like:
/NOTE: ptest-runner: Performing useradd with [--root 
/path/to/build/tmp/work/x86-64-v3-poky-linux/ptest-runner/2.5.1/recipe-sysroot 
--system --no-create -home --home / --user-group ptest]
useradd: cannot open /etc/passwd
ERROR: ptest-runner: useradd command did not succeed./

Anyway, I see in another patch, you switched from '--root' to 
'--prefix', so I guess this patch is not needed after the switch.

Regards,
Qi

On 4/21/26 18:41, Richard Purdie via lists.openembedded.org wrote:
> As far as I can tell, this patch is a no-op and doens't change the code.
> As such, I think it just complicates things and can be removed.
>
> Signed-off-by: Richard Purdie<richard.purdie@linuxfoundation.org>
> ---
>   ...nexpected-open-failure-in-chroot-env.patch | 48 -------------------
>   meta/recipes-extended/shadow/shadow_4.19.4.bb |  1 -
>   2 files changed, 49 deletions(-)
>   delete mode 100644 meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch
>
> diff --git a/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch b/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch
> deleted file mode 100644
> index 699269ed643..00000000000
> --- a/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -From f7b765c022e4cad9140ac44712885c66e149abdc Mon Sep 17 00:00:00 2001
> -From: Chen Qi<Qi.Chen@windriver.com>
> -Date: Thu, 17 Jul 2014 15:53:34 +0800
> -Subject: [PATCH] commonio.c-fix-unexpected-open-failure-in-chroot-env
> -
> -Upstream-Status: Inappropriate [OE specific]
> -
> -commonio.c: fix unexpected open failure in chroot environment
> -
> -When using commands with '-R <newroot>' option in our pseudo environment,
> -we would usually get the 'Pemission Denied' error. This patch serves as
> -a workaround to this problem.
> -
> -Note that this patch doesn't change the logic in the code, it just expands
> -the codes.
> -
> -Signed-off-by: Chen Qi<Qi.Chen@windriver.com>
> ----
> - lib/commonio.c | 16 ++++++++++++----
> - 1 file changed, 12 insertions(+), 4 deletions(-)
> -
> -diff --git a/lib/commonio.c b/lib/commonio.c
> -index 4d83e83..9ee0e13 100644
> ---- a/lib/commonio.c
> -+++ b/lib/commonio.c
> -@@ -604,10 +604,18 @@ int commonio_open (struct commonio_db *db, int mode)
> - 	db->cursor = NULL;
> - 	db->changed = false;
> -
> --	fd = open (db->filename,
> --	             (db->readonly ? O_RDONLY : O_RDWR)
> --	           | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC);
> --	saved_errno = errno;
> -+	if (db->readonly) {
> -+		fd = open (db->filename,
> -+			   (true ? O_RDONLY : O_RDWR)
> -+			   | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC);
> -+		saved_errno = errno;
> -+	} else {
> -+		fd = open (db->filename,
> -+			   (false ? O_RDONLY : O_RDWR)
> -+			   | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW| O_CLOEXEC);
> -+		saved_errno = errno;
> -+	}
> -+
> - 	db->fp = NULL;
> - 	if (fd >= 0) {
> - #ifdef WITH_TCB
> diff --git a/meta/recipes-extended/shadow/shadow_4.19.4.bb b/meta/recipes-extended/shadow/shadow_4.19.4.bb
> index 3ab9ae9c287..94f155641cc 100644
> --- a/meta/recipes-extended/shadow/shadow_4.19.4.bb
> +++ b/meta/recipes-extended/shadow/shadow_4.19.4.bb
> @@ -23,7 +23,6 @@ SRC_URI:append:class-target = " \
>              "
>   
>   SRC_URI:append:class-native = " \
> -file://commonio.c-fix-unexpected-open-failure-in-chroot-env.patch \
>              file://disable_syslog.patch \
>              file://notallylog.patch \
>              "
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#235665):https://lists.openembedded.org/g/openembedded-core/message/235665
> Mute This Topic:https://lists.openembedded.org/mt/118935407/7304865
> Group Owner:openembedded-core+owner@lists.openembedded.org
> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@eng.windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Richard Purdie April 22, 2026, 6:44 a.m. UTC | #2
Hi Qi,

On Wed, 2026-04-22 at 13:51 +0800, ChenQi wrote:
>  I'm the author of the original patch, so I just double checked it.
>
> This might be counter-intuitive, but this patch does solve some
> problem in pseudo environment.
> 
> Without this patch, running a simple 'bitbake ptest-runner' fails.
> The error message is like:
> 
> NOTE: ptest-runner: Performing useradd with [--root
> /path/to/build/tmp/work/x86-64-v3-poky-linux/ptest-
> runner/2.5.1/recipe-sysroot --system --no-create -home --home / --
> user-group ptest]
>  useradd: cannot open /etc/passwd
>  ERROR: ptest-runner: useradd command did not succeed.
> 
> Anyway, I see in another patch, you switched from '--root' to '--
> prefix', so I guess this patch is not needed after the switch.

I did suspect it was working around some issue but the patch does not
explain what and in theory it should be no change. I therefore sent a
"remove it " patch to work out if it was still needed after 12 years
and if so, what the real issue was.

It turns out that shadow-native on systems using fortify source would
break when building something like ptest-runner. Initially I tested on
a system without fortify source where you don't see the issue.

Looking at the useradd binary from shadow, it was optimising open() to
call __open_2(), and it turns out we're missing a wrapper for that in
pseudo.

I've therefore added it to pseudo and that should remove the need for
the patch. A local test on a system I reproduced it on did show it
being fixed.

We still needed to do this even with the --prefix change, I'll probably
hold off that until the next development cycle though.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch b/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch
deleted file mode 100644
index 699269ed643..00000000000
--- a/meta/recipes-extended/shadow/files/commonio.c-fix-unexpected-open-failure-in-chroot-env.patch
+++ /dev/null
@@ -1,48 +0,0 @@ 
-From f7b765c022e4cad9140ac44712885c66e149abdc Mon Sep 17 00:00:00 2001
-From: Chen Qi <Qi.Chen@windriver.com>
-Date: Thu, 17 Jul 2014 15:53:34 +0800
-Subject: [PATCH] commonio.c-fix-unexpected-open-failure-in-chroot-env
-
-Upstream-Status: Inappropriate [OE specific]
-
-commonio.c: fix unexpected open failure in chroot environment
-
-When using commands with '-R <newroot>' option in our pseudo environment,
-we would usually get the 'Pemission Denied' error. This patch serves as
-a workaround to this problem.
-
-Note that this patch doesn't change the logic in the code, it just expands
-the codes.
-
-Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
----
- lib/commonio.c | 16 ++++++++++++----
- 1 file changed, 12 insertions(+), 4 deletions(-)
-
-diff --git a/lib/commonio.c b/lib/commonio.c
-index 4d83e83..9ee0e13 100644
---- a/lib/commonio.c
-+++ b/lib/commonio.c
-@@ -604,10 +604,18 @@ int commonio_open (struct commonio_db *db, int mode)
- 	db->cursor = NULL;
- 	db->changed = false;
- 
--	fd = open (db->filename,
--	             (db->readonly ? O_RDONLY : O_RDWR)
--	           | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC);
--	saved_errno = errno;
-+	if (db->readonly) {
-+		fd = open (db->filename,
-+			   (true ? O_RDONLY : O_RDWR)
-+			   | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC);
-+		saved_errno = errno;
-+	} else {
-+		fd = open (db->filename,
-+			   (false ? O_RDONLY : O_RDWR)
-+			   | O_NOCTTY | O_NONBLOCK | O_NOFOLLOW| O_CLOEXEC);
-+		saved_errno = errno;
-+	}
-+
- 	db->fp = NULL;
- 	if (fd >= 0) {
- #ifdef WITH_TCB
diff --git a/meta/recipes-extended/shadow/shadow_4.19.4.bb b/meta/recipes-extended/shadow/shadow_4.19.4.bb
index 3ab9ae9c287..94f155641cc 100644
--- a/meta/recipes-extended/shadow/shadow_4.19.4.bb
+++ b/meta/recipes-extended/shadow/shadow_4.19.4.bb
@@ -23,7 +23,6 @@  SRC_URI:append:class-target = " \
            "
 
 SRC_URI:append:class-native = " \
-           file://commonio.c-fix-unexpected-open-failure-in-chroot-env.patch \
            file://disable_syslog.patch \
            file://notallylog.patch \
            "