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....@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....@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....@intel.com> Cc: Darren Hart <dvh...@linux.intel.com> Cc: Paul Eggleton <paul.eggle...@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 _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core