[PATCHv4] Fix recursive mode -st on BUILDDIR setup

[PATCHv4] Fix recursive mode -st on BUILDDIR setup

[Alex Franco]

Removing recursive option from chmod -st on BUILDDIR as it would
take very long on existing build directories

[YOCTO #7669]

Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com>
---
 meta/classes/sanity.bbclass | 9 ++++++---
 scripts/oe-setup-builddir   | 5 ++++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 2864318..29bb619 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -841,9 +841,12 @@ def check_sanity_everybuild(status, d):
     else:
         bb.utils.mkdirhier(tmpdir)
         # Remove setuid, setgid and sticky bits from TMPDIR
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
+        try:
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
+        except OSError:
+            bb.warn("Unable to chmod TMPDIR: %s" % tmpdir)
         with open(checkfile, "w") as f:
             f.write(tmpdir)
 
diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
index f5b7e4e..91bd86b 100755
--- a/scripts/oe-setup-builddir
+++ b/scripts/oe-setup-builddir
@@ -24,7 +24,10 @@ if [ -z "$BUILDDIR" ]; then
 fi
 
 mkdir -p "$BUILDDIR/conf"
-chmod -R -st "$BUILDDIR" 
+
+# Attempting removal of sticky,setuid bits from BUILDDIR, BUILDDIR/conf
+chmod -st "$BUILDDIR" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR"
+chmod -st "$BUILDDIR/conf" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR/conf"
 
 if [ ! -d "$BUILDDIR" ]; then
     echo >&2 "Error: The builddir ($BUILDDIR) does not exist!"
-- 
2.5.1

Re: [PATCHv4] Fix recursive mode -st on BUILDDIR setup

[Patrick Ohly]

On Thu, 2015-09-03 at 16:56 -0500, Alex Franco wrote:
> Removing recursive option from chmod -st on BUILDDIR as it would
> take very long on existing build directories

Okay, so this *is* a problem others are also seeing ;-}

> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> index f5b7e4e..91bd86b 100755
> --- a/scripts/oe-setup-builddir
> +++ b/scripts/oe-setup-builddir
> @@ -24,7 +24,10 @@ if [ -z "$BUILDDIR" ]; then
>  fi
>  
>  mkdir -p "$BUILDDIR/conf"
> -chmod -R -st "$BUILDDIR" 
> +
> +# Attempting removal of sticky,setuid bits from BUILDDIR, BUILDDIR/conf
> +chmod -st "$BUILDDIR" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR"
> +chmod -st "$BUILDDIR/conf" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR/conf"
>  
>  if [ ! -d "$BUILDDIR" ]; then
>      echo >&2 "Error: The builddir ($BUILDDIR) does not exist!"

What was the reasoning behind adding these operations on $BUILDDIR/conf
before the check whether BUILDDIR exists and is a directory? Looks a bit
fishy to me.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCHv4] Fix recursive mode -st on BUILDDIR setup

[Patrick Ohly]

On Fri, 2015-09-04 at 09:17 +0200, Patrick Ohly wrote:
> On Thu, 2015-09-03 at 16:56 -0500, Alex Franco wrote:
> > Removing recursive option from chmod -st on BUILDDIR as it would
> > take very long on existing build directories
> 
> Okay, so this *is* a problem others are also seeing ;-}
> 
> > diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> > index f5b7e4e..91bd86b 100755
> > --- a/scripts/oe-setup-builddir
> > +++ b/scripts/oe-setup-builddir
> > @@ -24,7 +24,10 @@ if [ -z "$BUILDDIR" ]; then
> >  fi
> >  
> >  mkdir -p "$BUILDDIR/conf"
> > -chmod -R -st "$BUILDDIR" 
> > +
> > +# Attempting removal of sticky,setuid bits from BUILDDIR, BUILDDIR/conf
> > +chmod -st "$BUILDDIR" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR"
> > +chmod -st "$BUILDDIR/conf" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR/conf"
> >  
> >  if [ ! -d "$BUILDDIR" ]; then
> >      echo >&2 "Error: The builddir ($BUILDDIR) does not exist!"
> 
> What was the reasoning behind adding these operations on $BUILDDIR/conf
> before the check whether BUILDDIR exists and is a directory? Looks a bit
> fishy to me.

Non-existent parent of $BUILDDIR is caught elsewhere, but pointing
BUILDDIR to a file instead of a directory indeed leads to sub-optimal
error reporting:

$ touch /tmp/foobar
$ . oe-init-build-env /tmp/foobar
mkdir: cannot create directory ‘/tmp/foobar’: Not a directory
chmod: cannot access ‘/tmp/foobar/conf’: Not a directory
Error: The builddir (/tmp/foobar) does not exist!

Not sure whether it's worth fixing, though. Better get this performance
fix included quickly.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCHv4] Fix recursive mode -st on BUILDDIR setup

[Alex Franco]

I agree these operations should take place after those checks.

Alex

On 09/04/2015 02:17 AM, Patrick Ohly wrote:
> On Thu, 2015-09-03 at 16:56 -0500, Alex Franco wrote:
>> Removing recursive option from chmod -st on BUILDDIR as it would
>> take very long on existing build directories
> Okay, so this *is* a problem others are also seeing ;-}
>
>> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
>> index f5b7e4e..91bd86b 100755
>> --- a/scripts/oe-setup-builddir
>> +++ b/scripts/oe-setup-builddir
>> @@ -24,7 +24,10 @@ if [ -z "$BUILDDIR" ]; then
>>   fi
>>   
>>   mkdir -p "$BUILDDIR/conf"
>> -chmod -R -st "$BUILDDIR"
>> +
>> +# Attempting removal of sticky,setuid bits from BUILDDIR, BUILDDIR/conf
>> +chmod -st "$BUILDDIR" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR"
>> +chmod -st "$BUILDDIR/conf" 2>/dev/null || echo "WARNING: unable to chmod $BUILDDIR/conf"
>>   
>>   if [ ! -d "$BUILDDIR" ]; then
>>       echo >&2 "Error: The builddir ($BUILDDIR) does not exist!"
> What was the reasoning behind adding these operations on $BUILDDIR/conf
> before the check whether BUILDDIR exists and is a directory? Looks a bit
> fishy to me.
>

[PATCH] sanity.bbclass: show warning when chmod fails

[Martin Jansa]

From: Alex Franco <alejandro.franco@linux.intel.com>

* for some reason this part of:
  http://patchwork.openembedded.org/patch/102561/
  wasn't ever merged.

[YOCTO #7669]

Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com>
Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/classes/sanity.bbclass | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 2eb744f..34f8618 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -839,9 +839,12 @@ def check_sanity_everybuild(status, d):
     else:
         bb.utils.mkdirhier(tmpdir)
         # Remove setuid, setgid and sticky bits from TMPDIR
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
-        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
+        try:
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
+            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
+        except OSError:
+            bb.warn("Unable to chmod TMPDIR: %s" % tmpdir)
         with open(checkfile, "w") as f:
             f.write(tmpdir)
 
-- 
2.5.3

Re: [PATCH] sanity.bbclass: show warning when chmod fails

[Christopher Larson]

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

On Wed, Sep 23, 2015 at 9:13 AM, Martin Jansa <martin.jansa@gmail.com>
wrote:

> From: Alex Franco <alejandro.franco@linux.intel.com>
>
> * for some reason this part of:
>   http://patchwork.openembedded.org/patch/102561/
>   wasn't ever merged.
>
> [YOCTO #7669]
>
> Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com>
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/classes/sanity.bbclass | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 2eb744f..34f8618 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -839,9 +839,12 @@ def check_sanity_everybuild(status, d):
>      else:
>          bb.utils.mkdirhier(tmpdir)
>          # Remove setuid, setgid and sticky bits from TMPDIR
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
> +        try:
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
> +        except OSError:
> +            bb.warn("Unable to chmod TMPDIR: %s" % tmpdir)
>

I'd suggest showing the actual error, here. E.g. "Unable to chmod TMPDIR:
%s" % exc. (The error message from the exception usually includes the path
on disk already, so probably don't need both tmpdir and exc).
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2629 bytes --]

Re: [PATCH] sanity.bbclass: show warning when chmod fails

[Martin Jansa]

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

On Wed, Sep 23, 2015 at 06:13:56PM +0200, Martin Jansa wrote:
> From: Alex Franco <alejandro.franco@linux.intel.com>
> 
> * for some reason this part of:
>   http://patchwork.openembedded.org/patch/102561/
>   wasn't ever merged.

I've changed the configuration on my jenkins servers, so that now it
doesn't fail.

In case someone needs that as well then you need to use mode, uid and
git parameters for mounting tmpfs, e.g.

none /home/jenkins/oe/world/shr-core/tmp-glibc/ tmpfs noatime,nodev,nosuid,size=74G,mode=777,users,exec,uid=3004,gid=3004 0 0

where 3004 is uid and gid of jenkins user which is running the bitbake.



This change is still needed, because following exception shows so much about
bbclass sanity, that average user will get confused and eventually insane.

ERROR: Execution of event handler 'check_sanity_eventhandler' failed
Traceback (most recent call last):
  File "check_sanity_eventhandler(e)", line 6, in check_sanity_eventhandler(e=<bb.event.SanityCheck object at 0x57ec3d0>)
  File "sanity.bbclass", line 34, in check_sanity(sanity_data=<bb.data_smart.DataSmart object at 0x57d5e10>)
  File "sanity.bbclass", line 155, in check_sanity_everybuild(status=<SanityStatus object at 0x57d5490>, d=<bb.data_smart.DataSmart object at 0x57d5e10>)
OSError: [Errno 1] Operation not permitted: '/home/jenkins/workspace/luneos-unstable/webos-ports/tmp-glibc'

ERROR: Command execution failed: Traceback (most recent call last):
  File "/home/jenkins/workspace/luneos-unstable/webos-ports/bitbake/lib/bb/command.py", line 101, in runAsyncCommand
    self.cooker.updateCache()
  File "/home/jenkins/workspace/luneos-unstable/webos-ports/bitbake/lib/bb/cooker.py", line 1503, in updateCache
    bb.event.fire(bb.event.SanityCheck(False), self.data)
  File "/home/jenkins/workspace/luneos-unstable/webos-ports/bitbake/lib/bb/event.py", line 170, in fire
    fire_class_handlers(event, d)
  File "/home/jenkins/workspace/luneos-unstable/webos-ports/bitbake/lib/bb/event.py", line 109, in fire_class_handlers
    execute_handler(name, handler, event, d)
  File "/home/jenkins/workspace/luneos-unstable/webos-ports/bitbake/lib/bb/event.py", line 81, in execute_handler
    ret = handler(event)
  File "check_sanity_eventhandler(e)", line 6, in check_sanity_eventhandler
  File "sanity.bbclass", line 34, in check_sanity
  File "sanity.bbclass", line 155, in check_sanity_everybuild
OSError: [Errno 1] Operation not permitted: '/home/jenkins/workspace/luneos-unstable/webos-ports/tmp-glibc'


Summary: There were 2 ERROR messages shown, returning a non-zero exit code.
Command exited with non-zero status 1


> 
> [YOCTO #7669]
> 
> Signed-off-by: Alex Franco <alejandro.franco@linux.intel.com>
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/classes/sanity.bbclass | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 2eb744f..34f8618 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -839,9 +839,12 @@ def check_sanity_everybuild(status, d):
>      else:
>          bb.utils.mkdirhier(tmpdir)
>          # Remove setuid, setgid and sticky bits from TMPDIR
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
> -        os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
> +        try:
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISUID)
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISGID)
> +            os.chmod(tmpdir, os.stat(tmpdir).st_mode & ~ stat.S_ISVTX)
> +        except OSError:
> +            bb.warn("Unable to chmod TMPDIR: %s" % tmpdir)
>          with open(checkfile, "w") as f:
>              f.write(tmpdir)
>  
> -- 
> 2.5.3
> 

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

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