[PATCH 0/3] Fixes for oe-init-build-env

[PATCH 0/3] Fixes for oe-init-build-env

[Peter Kjellerstedt]

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

[PATCH 1/3] oe-setup-builddir: Allow $OECORENOTESCONF to not exist

[Peter Kjellerstedt]

Signed-off-by: Peter Kjellerstedt <pkj@axis.com>
---
 scripts/oe-setup-builddir | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
index 76e1778..d5d8d98 100755
--- a/scripts/oe-setup-builddir
+++ b/scripts/oe-setup-builddir
@@ -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

[PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Peter Kjellerstedt]

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.

Signed-off-by: Peter Kjellerstedt <pkj@axis.com>
---
 scripts/oe-buildenv-internal | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/oe-buildenv-internal b/scripts/oe-buildenv-internal
index 0a4d324..df95389 100755
--- a/scripts/oe-buildenv-internal
+++ b/scripts/oe-buildenv-internal
@@ -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

[PATCH 3/3] oe-init-build-env: Make it use the correct $OEROOT with zsh

[Peter Kjellerstedt]

Signed-off-by: Peter Kjellerstedt <pkj@axis.com>
---
 oe-init-build-env | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/oe-init-build-env b/oe-init-build-env
index 67eddcd..68af7b5 100755
--- a/oe-init-build-env
+++ b/oe-init-build-env
@@ -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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Trevor Woerner]

On Fri, Apr 5, 2013 at 12:59 PM, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> +[ "${PATH#$NEWPATHS}" != "$PATH" ] || PATH="$NEWPATHS$PATH"

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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Paul Eggleton]

On Monday 08 April 2013 13:31:50 Trevor Woerner wrote:
> On Fri, Apr 5, 2013 at 12:59 PM, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> > +[ "${PATH#$NEWPATHS}" != "$PATH" ] || PATH="$NEWPATHS$PATH"
> 
> 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?

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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Chris Larson]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Mon, Apr 8, 2013 at 10:48 AM, Paul Eggleton <
paul.eggleton@linux.intel.com> wrote:

> On Monday 08 April 2013 13:31:50 Trevor Woerner wrote:
> > On Fri, Apr 5, 2013 at 12:59 PM, Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com> wrote:
> > > +[ "${PATH#$NEWPATHS}" != "$PATH" ] || PATH="$NEWPATHS$PATH"
> >
> > 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?
>
> 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

[-- Attachment #2: Type: text/html, Size: 1458 bytes --]

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Peter Kjellerstedt]

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

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: kergoth@gmail.com [mailto:kergoth@gmail.com] 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 <paul.eggleton@linux.intel.com<mailto:paul.eggleton@linux.intel.com>> wrote:
On Monday 08 April 2013 13:31:50 Trevor Woerner wrote:
> On Fri, Apr 5, 2013 at 12:59 PM, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com<mailto:peter.kjellerstedt@axis.com>> wrote:
> > +[ "${PATH#$NEWPATHS}" != "$PATH" ] || PATH="$NEWPATHS$PATH"
>
> 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?
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

[-- Attachment #2: Type: text/html, Size: 7480 bytes --]

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Chris Larson]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

On Tue, Apr 9, 2013 at 3:01 AM, Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

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


Ah, right, thanks for the clarification :)
-- 
Christopher Larson

[-- Attachment #2: Type: text/html, Size: 581 bytes --]

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Trevor Woerner]

On Mon, Apr 8, 2013 at 1:48 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> Unfortunately I think this is specific to bash, so it may not be portable.
> Maybe the equivalent can be achieved with sed however.

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?

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Trevor Woerner]

On Tue, Apr 9, 2013 at 1:29 PM, Trevor Woerner <twoerner@gmail.com> wrote:
> Under which shells do we expect a Yocto build to succeed?

Whoops! My bad.

sh -> yes
bash -> not so much

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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Khem Raj]

On Apr 9, 2013, at 10:29 AM, Trevor Woerner <twoerner@gmail.com> wrote:

> On Mon, Apr 8, 2013 at 1:48 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
>> Unfortunately I think this is specific to bash, so it may not be portable.
>> Maybe the equivalent can be achieved with sed however.
> 
> 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?
> 

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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Paul Eggleton]

On Tuesday 09 April 2013 13:34:43 Trevor Woerner wrote:
> On Tue, Apr 9, 2013 at 1:29 PM, Trevor Woerner <twoerner@gmail.com> wrote:
> > Under which shells do we expect a Yocto build to succeed?
> 
> 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

Re: [PATCH 2/3] oe-buildenv-internal: Only add to $PATH if needed

[Mark Hatle]

On 4/9/13 12:29 PM, Trevor Woerner wrote:
> On Mon, Apr 8, 2013 at 1:48 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
>> Unfortunately I think this is specific to bash, so it may not be portable.
>> Maybe the equivalent can be achieved with sed however.
>
> 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?

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

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

--Mark

>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>