Fixes for oe-init-build-env

These updates fix three small problems with oe-init-build-env.

//Peter

The following changes since commit 419ef63ba5741ab67c91cbb96ee0c11e44975cb0:

atom-pc: add i965 Mesa driver so GL works on i965 onwards (2013-04-05 12:44:28 +0100)

are available in the git repository at:

git://git.yoctoproject.org/poky-contrib pkj/oe-init-build-env
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=pkj/oe-init-build-env

Peter Kjellerstedt (3):
oe-setup-builddir: Allow $OECORENOTESCONF to not exist
oe-buildenv-internal: Only add to $PATH if needed
oe-init-build-env: Make it use the correct $OEROOT with zsh

 oe-init-build-env            | 2 ++
 scripts/oe-buildenv-internal | 5 +++--
 scripts/oe-setup-builddir    | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

1.7.11.7

---
 scripts/oe-setup-builddir | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 76e1778..d5d8d98 100755
@@ -118,5 +118,5 @@ EOM
 if [ "x" = "x$OECORENOTESCONF" ]; then
     OECORENOTESCONF="$OEROOT/meta/conf/conf-notes.txt"
 fi
-cat $OECORENOTESCONF
+[ ! -r "$OECORENOTESCONF" ] || cat $OECORENOTESCONF
 unset OECORENOTESCONF

1.7.11.7

If $PATH already has the needed paths at the beginning, there is no need to add them again. This allows rerunning oe-init-build-env for the same directory without having $PATH increase unnecessarily every time.

---
 scripts/oe-buildenv-internal | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 0a4d324..df95389 100755
@@ -74,8 +74,9 @@ if ! (test -d "$BITBAKEDIR"); then
     return 1
 fi
 
-PATH="${OEROOT}/scripts:$BITBAKEDIR/bin/:$PATH"
-unset BITBAKEDIR
+NEWPATHS="${OEROOT}/scripts:$BITBAKEDIR/bin/:"
+[ "${PATH#$NEWPATHS}" != "$PATH" ] || PATH="$NEWPATHS$PATH"
+unset BITBAKEDIR NEWPATHS
 
 # Used by the runqemu script
 export BUILDDIR

1.7.11.7

This is certainly a welcome addition in functionality, but it relies on the pattern remaining at the start of the PATH (i.e. the user hasn't played with PATH in any way). Could we not use the ${parameter/pattern/string} parameter expansion instead (e.g. "${PATH/$NEWPATHS/}") so it doesn't matter whether the user has further modified the PATH?

Best regards,
Trevor

Unfortunately I think this is specific to bash, so it may not be portable. Maybe the equivalent can be achieved with sed however.

Cheers,
Paul

Paul Eggleton
Intel Open Source Technology Centre

Also, neither version matches the : separator, which means we could in theory get false positive matches.

Christopher Larson

The colon separator is included at the end of $NEWPATHS so it should not be possible for any false positives.

The intention of the patch was to avoid $PATH growing when rerunning oe-init-build-env in the same directory multiple times. I only match at the start of $PATH as I wanted to maintain the current functionality that guarantees that the paths are added to the beginning of $PATH and therefore searched before anything else.

I did not want to go into analyzing $PATH which would have been needed to remove paths for other build trees. Also, I do not actually see this as possible since the static parts of the added paths (/scripts and /bin/) are to generic to be able to remove those paths without the risk of accidentally removing something unrelated.

However, one thing that can easily be done is to instead reorder “$SOMEPATHS:$NEWPATHS$SOMEOTHERPATHS” to “$NEWPATHS$SOMEPATHS:$SOMEOTHERPATHS”. That should maintain the property of having the new paths at the beginning, while at the same time not grow $PATH unnecessarily if the paths are already present, but preceded by some other paths.

I will provide a second version of the patch with the relevant part changed to:

# Make sure our paths are at the beginning of $PATH NEWPATHS="${OEROOT}/scripts:$BITBAKEDIR/bin:" PATH=$NEWPATHS$(echo $PATH | sed -e "s|:$NEWPATHS|:|g" -e "s|^$NEWPATHS||") unset BITBAKEDIR NEWPATHS

//Peter

From: [email protected] [mailto:[email protected]] On Behalf Of Chris Larson
Sent: den 9 april 2013 00:40
To: Paul Eggleton
Cc: Trevor Woerner; Peter Kjellerstedt; Patches and discussions about the oe-core layer
Subject: Re: [OE-core] [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

On Mon, Apr 8, 2013 at 10:48 AM, Paul Eggleton <[email protected]<mailto:[email protected]>> wrote:

Unfortunately I think this is specific to bash, so it may not be portable. Maybe the equivalent can be achieved with sed however.

Also, neither version matches the : separator, which means we could in theory get false positive matches. --
Christopher Larson

Ah, right, thanks for the clarification :)

Christopher Larson

Under which shells do we expect a Yocto build to succeed? The latest version of this change wouldn't work under tcsh, for example. (Not that I'm suggesting it should!) Isn't there already an inherent Bourne-ish expectation?

Whoops! My bad.

sh -> yes
bash -> not so much

Let me rephrase: are bash-specific features to be so feared?

If we can avoid them, yes. The stuff we absolutely must not have is:

* bashisms in scripts that start with #!/bin/sh as opposed to #!/bin/bash (and for scripts that are installed on the target we try to avoid the latter as it just means bash has to be installed on the target as well, which is often not desirable)

* bashisms in shell functions within recipes since these are executed under /bin/sh

Even if we were to ignore alternative shells such as zsh and tcsh, bashisms in the above cases will cause breakages on Ubuntu (where /bin/sh is dash by default). From the denzil (Yocto Project 1.2) release onwards we have been stamping out bashisms and trying to avoid introducing any new ones.

Cheers,
Paul

Paul Eggleton
Intel Open Source Technology Centre

well we try to not depend on bash or any other shell. e.g. I can use dash with no problems

dash, ash, bash and zsh. POSIX compliant shells.

These are the places I try to verify scripts that refer to /bin/sh.

--Mark

---
 oe-init-build-env | 2 ++
 1 file changed, 2 insertions(+)

index 67eddcd..68af7b5 100755
@@ -30,6 +30,8 @@ if [ -z "$ZSH_NAME" ] && [ "x$0" = "x./oe-init-build-env" ]; then
 else
    if [ -n "$BASH_SOURCE" ]; then
       OEROOT="`dirname $BASH_SOURCE`"
+   elif [ -n "$ZSH_NAME" ]; then
+      OEROOT="`dirname $0`"
    else
       OEROOT="`pwd`"
    fi

1.7.11.7