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

login
register
mail settings
Submitter Dexuan Cui
Date Aug. 9, 2011, 2:04 p.m.
Message ID <1865303E0DED764181A9D882DEF65FB6B5FF85AF9D@shsmsx502.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/9539/
State New, archived
Headers show

Comments

Dexuan Cui - Aug. 9, 2011, 2:04 p.m.
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@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@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@intel.com>
    Cc: Darren Hart <dvhart@linux.intel.com>
    Cc: Paul Eggleton <paul.eggleton@linux.intel.com>
Darren Hart - Aug. 9, 2011, 3:06 p.m.
On 08/09/2011 07:04 AM, Cui, Dexuan wrote:
> 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@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@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@intel.com>
>     Cc: Darren Hart <dvhart@linux.intel.com>
>     Cc: Paul Eggleton <paul.eggleton@linux.intel.com>

Looks good,

Acked-by: Darren Hart <dvhart@linux.intel.com>

Thanks Dexuan

> 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
Dexuan Cui - Aug. 10, 2011, 3:18 a.m.
Darren Hart wrote on 2011-08-09:
> On 08/09/2011 07:04 AM, Cui, Dexuan wrote:
>> 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=dcu i/
>> bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47
>> 
>> -- Dexuan
>> 
>> commit 2ece3a84d646bb4bf83f3c8aa3067cb99989da47
>> Author: Dexuan Cui <dexuan.cui@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@intel.com>
>>     Cc: Darren Hart <dvhart@linux.intel.com>
>>     Cc: Paul Eggleton <paul.eggleton@linux.intel.com>
> 
> Looks good,
> 
> Acked-by: Darren Hart <dvhart@linux.intel.com>
Thanks, Darren!

Hi RP, could you please pull the patch? 
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=dcui/bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47

Thanks,
-- Dexuan
Richard Purdie - Aug. 10, 2011, 12:21 p.m.
On Wed, 2011-08-10 at 11:18 +0800, Cui, Dexuan wrote:
> Darren Hart wrote on 2011-08-09:
> > On 08/09/2011 07:04 AM, Cui, Dexuan wrote:
> >> 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=dcu i/
> >> bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47
> >> 
> >> -- Dexuan
> >> 
> >> commit 2ece3a84d646bb4bf83f3c8aa3067cb99989da47
> >> Author: Dexuan Cui <dexuan.cui@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@intel.com>
> >>     Cc: Darren Hart <dvhart@linux.intel.com>
> >>     Cc: Paul Eggleton <paul.eggleton@linux.intel.com>
> > 
> > Looks good,
> > 
> > Acked-by: Darren Hart <dvhart@linux.intel.com>
> Thanks, Darren!
> 
> Hi RP, could you please pull the patch? 
> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=dcui/bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47

Merged to master, thanks.

Richard
Richard Purdie - Aug. 10, 2011, 1:04 p.m.
On Wed, 2011-08-10 at 11:18 +0800, Cui, Dexuan wrote:
> Darren Hart wrote on 2011-08-09:
> > On 08/09/2011 07:04 AM, Cui, Dexuan wrote:
> >> 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=dcu i/
> >> bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47
> >> 
> >> -- Dexuan
> >> 
> >> commit 2ece3a84d646bb4bf83f3c8aa3067cb99989da47
> >> Author: Dexuan Cui <dexuan.cui@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@intel.com>
> >>     Cc: Darren Hart <dvhart@linux.intel.com>
> >>     Cc: Paul Eggleton <paul.eggleton@linux.intel.com>
> > 
> > Looks good,
> > 
> > Acked-by: Darren Hart <dvhart@linux.intel.com>
> Thanks, Darren!
> 
> Hi RP, could you please pull the patch? 
> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=dcui/bug-671&id=2ece3a84d646bb4bf83f3c8aa3067cb99989da47

Merged to master, thanks.

Richard

Patch

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