[OE-core] [PATCH 1/1] oe-init-build-env, scripts/oe-buildenv-internal: add error detecting for $BDIR

[Cui, Dexuan]

Darren Hart wrote on 2011-08-09:
> On 08/08/2011 07:13 PM, Cui, Dexuan wrote:
>> From 593e3506f4d4d9f6030100eac8c8e0af7f8ce3eb Mon Sep 17 00:00:00 2001
>> From: Dexuan Cui <dexuan.cui at intel.com> Date: Thu, 04 Aug 2011 06:53:20
>> +0000 Subject: scripts/oe-buildenv-internal: improve the error
>> detecting for $BDIR
>> 
>> Thanks a lot to Darren Hart and Paul Eggleton's suggestions!
>> 
>> A description of this improvement from Darren is:
>> "
> 
> Drop the above two lines, we don't need these in the permanent git
> history :-) Simply adding a pair of CC lines below the Signed-off-by
> for Paul and myself is sufficient to indicate our involvement. If a
> significant portion of the patch had been authored by either Paul or
> myself, then getting our permission to include our Signed-off-by would be appropriate.
> In this case, a simple CC will suffice.
Ok, got it.

>> >&2 "Error: / is not supported as a build directory."
> 
> I understand wanting to use stderr, but I don't see the >&2 very often
> in our shell scripts. Is this common practice? If so, it's fine, I'm just wondering.
I'm not sure this is common practice.
I'm just following the existing style in scripts/oe-buildenv-internal and
scripts/oe-setup-builddir. :-)

>> + # buggy readlink of Ubuntu 10.04 that doesn't ignore
>> + trailing slash
> 
>              a buggy        s/of/in/
> slashes
Thanks for pointing my mistakes

>> +        # and hence "readlink -f new_dir_to_be_created/" returns empty.
>> +        # See YOCTO #671 for details.
> 
> 
> Please don't include references to bug numbers in the code. Imagine
> what it would look like if we included every bug in the source! :-)
> Reference the bug in the git commit comment header.
OK, got it.

> 
> 
>> +        BDIR=`echo $BDIR | sed -re 's|/+$||'`
>> +
>> +        BDIR=`readlink -f "$BDIR"`
>> +        if [ -z "$BDIR" ]; then
>> +            PARENTDIR=`dirname "$1"`
>> +            echo >&2 "Error: the directory $PARENTDIR does not exist?"
>>              return 1
>>          fi
>>      fi
> 
> With the trivial changes mentioned above, this looks good to me.
Hi Darren, thanks a lot for all the suggestions! I appreciate your efforts to help to make the patch better.
Below is the updated patch (also pasted at the end of the mail):
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=dcui/bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47

Please review it.

Thanks,
-- Dexuan

commit 2ece3a84d646bb4bf83f3c8aa3067cb99989da47
Author: Dexuan Cui <dexuan.cui at intel.com>
Date:   Thu Aug 4 14:53:20 2011 +0800

    scripts/oe-buildenv-internal: improve the error detecting for $BDIR

    The previous fix for a bug in Ubuntu 10.04 readlink,
    be2a2764d8ceb398d81714661e6f199c8b11946c, notified the user when a trailing
    slash was used. As there is no semantic difference, simply remove any
    trailing slashes and proceed without nagging the user.

    See [YOCTO #671] for more details.

    Signed-off-by: Dexuan Cui <dexuan.cui at intel.com>
    Cc: Darren Hart <dvhart at linux.intel.com>
    Cc: Paul Eggleton <paul.eggleton at linux.intel.com>

diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal
index 117b0c5..61ac18c 100755
--- a/scripts/oe-buildenv-internal
+++ b/scripts/oe-buildenv-internal
@@ -28,14 +28,21 @@ if [ "x$BDIR" = "x" ]; then
     if [ "x$1" = "x" ]; then
         BDIR="build"
     else
-        BDIR=`readlink -f "$1"`
-        if [ -z "$BDIR"  ]; then
-            if expr "$1" : '.*/$' >/dev/null; then
-                echo >&2 "Error: please remove any trailing / in the argument."
-            else
-                PARENTDIR=`dirname "$1"`
-                echo >&2 "Error: the directory $PARENTDIR doesn't exist?"
-            fi
+        BDIR="$1"
+        if [ "$BDIR" = "/" ]; then
+            echo >&2 "Error: / is not supported as a build directory."
+            return 1
+        fi
+
+        # Remove any possible trailing slashes. This is used to work around
+        # buggy readlink in Ubuntu 10.04 that doesn't ignore trailing slashes
+        # and hence "readlink -f new_dir_to_be_created/" returns empty.
+        BDIR=`echo $BDIR | sed -re 's|/+$||'`
+
+        BDIR=`readlink -f "$BDIR"`
+        if [ -z "$BDIR" ]; then
+            PARENTDIR=`dirname "$1"`
+            echo >&2 "Error: the directory $PARENTDIR does not exist?"
             return 1
         fi
     fi