[PATCHv4] Fix recursive mode -st on BUILDDIR setup

Submitted by Alex Franco on Sept. 3, 2015, 9:56 p.m. | Patch ID: 102561

Details

Message ID 1441317404-6663-1-git-send-email-alejandro.franco@linux.intel.com
State New
Headers show

Commit Message

Alex Franco Sept. 3, 2015, 9:56 p.m.
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(-)

Patch hide | download patch | download mbox

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!"

Comments

Patrick Ohly Sept. 4, 2015, 7:17 a.m.
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.
Patrick Ohly Sept. 4, 2015, 7:23 a.m.
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.
Alex Franco Sept. 4, 2015, 8:20 p.m.
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.
>