Re: [PATCH] glibc-locale: Rewrite do_install using install utility instead of cp

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

[-- Attachment #1: Type: text/plain, Size: 6774 bytes --]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
> core-bounces@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 7 februari 2019 10:01
> To: Khem Raj <raj.khem@gmail.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] glibc-locale: Rewrite do_install using
> install utility instead of cp
> 
> On Wed, 2019-02-06 at 16:35 -0800, Khem Raj wrote:
> > This has been a constant source of trouble for build failures due to
> host-user-contaminated QA
> > errors of sort
> >
> > ERROR: QA Issue: glibc-locale: /glibc-binary-localedata-ca-
> es+valencia/usr/lib/locale/ca_ES@valencia/LC_MONETARY is owned by uid
> 3004, which is the same as the user running bitbake. This may be due to
> host contamination [host-user-contaminated]
> >
> > So far we have tried to mould cp command into not carrying the build
> > user permissions into install area but it is never entirely fixed
> since
> > the issue keeps popping up in various scenes
> >
> > This patch replaces use of cp with install utility and specifies
> install
> > mode for files explcitly
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >  meta/recipes-core/glibc/glibc-locale.inc | 44 ++++++++++++++--------
> --
> >  1 file changed, 25 insertions(+), 19 deletions(-)
> >
> > diff --git a/meta/recipes-core/glibc/glibc-locale.inc b/meta/recipes-
> core/glibc/glibc-locale.inc
> > index 6384f9cbf1..9b256a5108 100644
> > --- a/meta/recipes-core/glibc/glibc-locale.inc
> > +++ b/meta/recipes-core/glibc/glibc-locale.inc
> > @@ -72,27 +72,33 @@ FILES_localedef = "${bindir}/localedef"
> >  LOCALETREESRC = "${COMPONENTS_DIR}/${PACKAGE_ARCH}/glibc-stash-
> locale"
> >
> >  do_install () {
> > -	mkdir -p ${D}${bindir} ${D}${datadir}
> > -	if [ -n "$(ls ${LOCALETREESRC}/${bindir})" ]; then
> > -		cp -R --no-dereference --preserve=mode,links
> ${LOCALETREESRC}/${bindir}/* ${D}${bindir}
> > -	fi
> > -	if [ -n "$(ls ${LOCALETREESRC}/${localedir})" ]; then
> > -		mkdir -p ${D}${localedir}
> > -		cp -R --no-dereference --preserve=mode,links
> ${LOCALETREESRC}/${localedir}/* ${D}${localedir}
> > -	fi
> > +        install -d ${D}${bindir}
> > +        find "${LOCALETREESRC}/${bindir}" -maxdepth 1 -type f \
> > +        -exec install -m 0755 -t "${D}${bindir}" {} \;
> > +
> > +        for d in . $(find "${LOCALETREESRC}/${localedir}" -type d -
> printf '%P ') ; do
> > +                install -d "${D}${localedir}/$d"
> > +                find "${LOCALETREESRC}/${localedir}/$d" -maxdepth 1
> -type f \
> > +                -exec install -m 0644 -t "${D}${localedir}/$d" {} \;
> > +        done
> >  	if [ ${@d.getVar('PACKAGE_NO_GCONV')} -eq 0 ]; then
> > -		mkdir -p ${D}${libdir}
> > -		if [ -e ${LOCALETREESRC}/${libdir}/gconv ]; then
> > -			cp -R --no-dereference --
> preserve=mode,links ${LOCALETREESRC}/${libdir}/gconv ${D}${libdir}
> > -		fi
> > -		if [ -e ${LOCALETREESRC}/${datadir}/i18n ]; then
> > -			cp -R --no-dereference --
> preserve=mode,links ${LOCALETREESRC}/${datadir}/i18n ${D}${datadir}
> > -		fi
> > -	fi
> > -	if [ -e ${LOCALETREESRC}/${datadir}/locale ]; then
> > -		cp -R --no-dereference --preserve=mode,links
> ${LOCALETREESRC}/${datadir}/locale ${D}${datadir}
> > +                for d in . $(find "${LOCALETREESRC}/${libdir}/gconv"
> -type d -printf '%P ') ; do
> > +                        install -d "${D}${libdir}/gconv/$d"
> > +                        find "${LOCALETREESRC}/${libdir}/gconv/$d" -
> maxdepth 1 -type f \
> > +                        -exec install -m 0755 -t
> "${D}${libdir}/gconv/$d" {} \;
> > +                done
> > +                for d in . $(find "${LOCALETREESRC}/${datadir}/i18n"
> -type d -printf '%P ') ; do
> > +                        install -d "${D}${datadir}/i18n/$d"
> > +                        find "${LOCALETREESRC}/${datadir}/i18n/$d" -
> maxdepth 1 -type f \
> > +                        -exec install -m 0644 -t
> "${D}${datadir}/i18n/$d" {} \;
> > +                done
> >  	fi
> > -	cp -R --no-dereference --preserve=mode,links
> ${LOCALETREESRC}/SUPPORTED ${WORKDIR}
> > +        for d in . $(find "${LOCALETREESRC}/${datadir}/locale" -type
> d -printf '%P ') ; do
> > +                install -d "${D}${datadir}/locale/$d"
> > +                find "${LOCALETREESRC}/${datadir}/locale/$d" -
> maxdepth 1 -type f \
> > +                -exec install -m 0644 -t "${D}${datadir}/locale/$d"
> {} \;
> > +        done
> > +	install -m 0644 ${LOCALETREESRC}/SUPPORTED
> ${WORKDIR}/SUPPORTED
> >  }
> >
> >  inherit libc-package
> 
> The trouble is this is a workaround. The cp commands should work and
> there is some underlying issue in pseudo causing this. We really need
> to figure out what that is since this will likely just mean we see the
> problem somewhere else :(
> 
> I still suspect some kind of inode number reuse problem which these cp
> commands trigger...
> 
> Cheers,
> 
> Richard

For the record, even if I think the patch was the right thing to do, it 
did not help the situation. The first build I did after it was integrated, 
I was bit by the following:

  Failed to determine the user name of UID 323 for 
  tmp/work/core2-64-poky-linux/glibc-locale/2.29-r0/package/usr/lib/locale

where UID 323 is my UID. If you do not recognize the error message, it is 
because it comes from a local patch we have for rpm to protect us from any 
incorrect UIDs/GIDs causing incorrect RPMs to be generated (see below). I 
checked the build directory and the /usr/lib/locale directory was the only 
file or directory with an incorrect owner, which is typical of these 
pseudo related problems with incorrect owners.

[rpm will by default silently fall back to use root for any files where it 
cannot determine the name of the user or group based on its UID/GID. This 
can be due to missing information in the /etc/passwd or /etc/groups files, 
typically due to relying on another recipe in DEPENDS to create the user 
without also having an RDEPENDS on the package that creates the user, or 
because of pseudo messing up. After this bit us once where a device in 
/dev ended up in an rpm in the sstate cache with root as group instead of 
the intended video group, which in turn caused the video application to 
fail as it was no longer allowed to open its device, I created the patch. 
It is causing a fair bit of grief as it causes builds to fail randomly, 
but at least we don't end up with an incorrect sstate anymore, which is 
worse. I am not sure this patch is suitable for integration to OE-Core, 
but I have attached it if anyone is interested.] 

//Peter


[-- Attachment #2: 0001-rpm-Make-it-an-error-if-a-UID-GID-cannot-be-turned-i.patch --]
[-- Type: application/octet-stream, Size: 3889 bytes --]

From 0f96b74bce4cf071ee5c22c4776f1352a1f43812 Mon Sep 17 00:00:00 2001
From: Peter Kjellerstedt <pkj@axis.com>
Date: Fri, 3 Nov 2017 13:44:40 +0100
Subject: [PATCH] rpm: Make it an error if a UID/GID cannot be turned into a
 name

Before, any unknown UID or GID would result in the name of the current
user (typically root) being silently used instead, which leads to
undeterministic behavior depending on whether the required user or
group name is available in /etc/passwd or /etc/group, or not.

This is only appropriate for the native and nativesdk versions.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 ...error-if-a-UID-GID-cannot-be-turned-into-.patch | 49 ++++++++++++++++++++++
 meta/recipes-devtools/rpm/rpm_4.14.2.1.bb          |  2 +
 2 files changed, 51 insertions(+)
 create mode 100644 meta/recipes-devtools/rpm/files/0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch

diff --git a/meta/recipes-devtools/rpm/files/0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch b/meta/recipes-devtools/rpm/files/0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch
new file mode 100644
index 0000000000..a9467dc298
--- /dev/null
+++ b/meta/recipes-devtools/rpm/files/0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch
@@ -0,0 +1,49 @@
+From 4777dc38c7554787caf9a37575c3e2912783ed23 Mon Sep 17 00:00:00 2001
+From: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
+Date: Fri, 3 Nov 2017 13:41:09 +0100
+Subject: [PATCH] Make it an error if a UID/GID cannot be turned into a name
+
+Before, any unknown UID or GID would result in the name of the current
+user (typically root) being silently used instead, which leads to
+undeterministic behavior depending on whether the required user or
+group name is available in /etc/passwd or /etc/group, or not.
+
+Upstream-Status: Inappropriate
+Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
+
+---
+ build/files.c | 18 +++++++++++-------
+ 1 file changed, 11 insertions(+), 7 deletions(-)
+
+diff --git a/build/files.c b/build/files.c
+index 585d73e3f..f92ebd9e9 100644
+--- a/build/files.c
++++ b/build/files.c
+@@ -1373,13 +1373,17 @@ static rpmRC addFile(FileList fl, const char * diskPath,
+     } else {
+ 	fileGname = rpmugGname(fileGid);
+     }
+-	
+-    /* Default user/group to builder's user/group */
+-    if (fileUname == NULL)
+-	fileUname = rpmugUname(getuid());
+-    if (fileGname == NULL)
+-	fileGname = rpmugGname(getgid());
+-    
++
++    /* Fail if the user and/or group name cannot be determined */
++    if (fileUname == NULL) {
++	rpmlog(RPMLOG_ERR, _("Failed to determine the user name of UID %d for %s\n"), fileUid, diskPath);
++	goto exit;
++    }
++    if (fileGname == NULL) {
++	rpmlog(RPMLOG_ERR, _("Failed to determine the group name of GID %d for %s\n"), fileGid, diskPath);
++	goto exit;
++    }
++
+     /* S_XXX macro must be consistent with type in find call at check-files script */
+     if (check_fileList && (S_ISREG(fileMode) || S_ISLNK(fileMode))) {
+ 	appendStringBuf(check_fileList, diskPath);
+-- 
+2.12.0
+
diff --git a/meta/recipes-devtools/rpm/rpm_4.14.2.1.bb b/meta/recipes-devtools/rpm/rpm_4.14.2.1.bb
index 063f4269a5..3316c7d60f 100644
--- a/meta/recipes-devtools/rpm/rpm_4.14.2.1.bb
+++ b/meta/recipes-devtools/rpm/rpm_4.14.2.1.bb
@@ -42,6 +42,8 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \
            file://0001-rpm-rpmio.c-restrict-virtual-memory-usage-if-limit-s.patch \
            file://0016-rpmscript.c-change-logging-level-around-scriptlets-t.patch \
            "
+SRC_URI_append_class-native = " file://0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch"
+SRC_URI_append_class-nativesdk = " file://0001-Make-it-an-error-if-a-UID-GID-cannot-be-turned-into-.patch"
 
 PE = "1"
 SRCREV = "4a9440006398646583f0d9ae1837dad2875013aa"
-- 
2.12.0