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

[Martin Jansa <martin.jansa@gmail.com>]

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

On Thu, Feb 07, 2019 at 10:59:18AM -0600, Joshua Watt wrote:
> On Thu, Feb 7, 2019 at 10:44 AM Joshua Watt <jpewhacker@gmail.com> wrote:
> >
> > Martin,
> >
> > Do you by any chance build in Docker (or some other transient container)? We recently found an issue with building in docker where when the primary container command would exit, causing the container to terminate. When the container terminates, the kernel unceremoniously sends SIGKILL to all its processes. This plays *very* poorly with pseudo because pseudo keeps it's entire database in a in-memory sqlite database and only flushes it out to disk on a periodic interval, or when it get SIGTERM (maybe SIGQUIT?) or exits normally. By default the pseudo daemon will also hang around for 20(?) seconds waiting for a new connection from a pseudo client, meaning that when our docker container was exiting, there were potentially several pseudo dameons sitting around waiting for a client with unflushed databases in memory that suddenly got a SIGKILL and all their data was lost. This was particularly bad if you aborted a build while it was running, or some build failure occurred. Since the pseudo database was lost, the only way to correct these errors was to clean and rebuild.
> >
> > I think pseudo could handle this better and keep the database on disk instead of in memory, but in the short term you can hack bitbake to pass the -S flag when invoking the pseudo client which will make it tell the dameon to shutdown (and thus flush the database) if it is the last connection.
> 
> FWIW, this patch makes it much easier to do that
> http://lists.openembedded.org/pipermail/bitbake-devel/2019-February/019813.html
> Just add
>  FAKEROOTCMD += "-S"
> in local.conf

Hi Joshua,

most of the build failures caused by this weren't from the builds in
Docker (or any other container), just plain Ubuntu (various versions
14.04 - 18.04) with just the prerequisites installed.

When I get access to some bigger build farm (probably in few months) I
can run that for loop reproducer from the ticket again with
FAKEROOTCMD += "-S"
to see if it makes any difference.

Regards,

> > On Thu, 2019-02-07 at 17:17 +0100, Martin Jansa wrote:
> >
> > There are other rare reproducers which don't involve cp/install (e.g. older qmake was triggering it quite often as well), I've mentioned some in:
> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=12434
> >
> > glibc-locale is the most common one for sure as pretty much everybody is building that one, I don't have objection converting this from cp to install to prevent this random error (very rare random failure doesn't really help with #12434 reproducer and whoever work on it, can either revert this change or try to reproduce with something else) - I've included few hundreds of INSANE_SKIPs to ignore _random_ host-user-contaminated QAs in LG layers to prevent our builds failing randomly and the one for glibc-locale is the longest part:
> > https://github.com/webosose/meta-webosose/blob/master/meta-webos/recipes-core/glibc/glibc-locale_%25.bbappend
> >
> > Regards,
> >
> > On Thu, Feb 7, 2019 at 3:50 PM Khem Raj <raj.khem@gmail.com> wrote:
> >
> > On Thu, Feb 7, 2019 at 1:00 AM Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> > >
> > > 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 :(
> >
> > so far this is the only place where it shows in world builds that
> > meta-oe does which includes many layers.
> >
> > >
> > > I still suspect some kind of inode number reuse problem which these cp
> > > commands trigger...
> >
> > cp and install are not same when it comes to how they operate
> > underneath. install is better when it comes to what we are doing here
> > it also gives better control of permissions and mods,as a recipe writer
> > someone would prefer then to let the tool ( cp ) assume it.
> >
> > So there might be a pseudo issue, I dont disagree but not obvious and
> > I believe using cp is subpar anyway.
> >
> > >
> > > Cheers,
> > >
> > > Richard
> > >
> >
> > --
> >
> > Joshua Watt <JPEWhacker@gmail.com>

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 201 bytes --]