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

[Darren Hart <dvhart@linux.intel.com>]

On 08/02/2011 04:43 AM, Richard Purdie wrote:
> On Tue, 2011-08-02 at 14:08 +0800, Dexuan Cui wrote:
>> [YOCTO #671]
>>
>> "readlink -f" in Ubuntu 10.04 is buggy: it doesn't ignore a trailing / (e.g.,
>> "readlink -f /tmp/non-existent-dir/" returns nothing, but according to
>> http://www.gnu.org/s/coreutils/manual/coreutils.pdf it should do that --
>> hence we get bug 671. It seems Ubuntu 10.10 or even later Ubuntu 11.04,
>> and other Linux distributions(e.g., Open Suse 11.4) haven't such an issue.
>>
>> So I think we should detect this and ask Ubuntu 10.04 users to avoid supply
>> a path with trailing slash here.
>>
>> Moreever, I also add the detection of non-existent path, e.g.,
>> source oe-init-build-env /non-existent-dir/build
>> can be detected and we'll print an error msg.
>> And, if we get errors in oe-buildenv-internal, we should stop the script
>> and shouldn't further run.
>>
>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> 
> Merged to master, thanks.

For a patch to address a relatively benign bug I thought the standard
procedure would be for it to await feedback for more than 5 hours. I was
hoping to have an opportunity to review this fix as I was working with
the team in root causing the bug.

+ if [ -z "$BDIR" ]; then
+ if expr "$1" : '.*/$' >/dev/null; then
+ echo >&2 "Error: please remove any trailing / in the argument."

This portion of the patch is really not necessary. There is no
meaningful difference between the path with or without the trailing
slash, a much simpler and less noisy solution would be to simply strip
the trailing slash from the user input before doing the rest of the
input validation.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel