[PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Awais Belal]

The script runs a 'cat' on the script and if it isn't present in the
sdk the cat command waits on the std input and hence the installation
process simply sits there.

Signed-off-by: Awais Belal <awais_belal@mentor.com>
---
 meta/files/toolchain-shar-relocate.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meta/files/toolchain-shar-relocate.sh b/meta/files/toolchain-shar-relocate.sh
index e3c10018ef..02a05664c6 100644
--- a/meta/files/toolchain-shar-relocate.sh
+++ b/meta/files/toolchain-shar-relocate.sh
@@ -3,6 +3,12 @@ if ! xargs --version > /dev/null 2>&1; then
 	exit 1
 fi
 
+# check if we have a valid env-setup script
+if [ ! -f "$env_setup_script" ]; then
+	echo "Main environment-setup file not found. Abort!"
+	exit 1
+fi
+
 # fix dynamic loader paths in all ELF SDK binaries
 native_sysroot=$($SUDO_EXEC cat $env_setup_script |grep 'OECORE_NATIVE_SYSROOT='|cut -d'=' -f2|tr -d '"')
 dl_path=$($SUDO_EXEC find $native_sysroot/lib -name "ld-linux*")
-- 
2.17.1

Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Paul Barker]

On Wed, 17 Jun 2020 at 21:48, Awais Belal <Awais_Belal@mentor.com> wrote:
>
> The script runs a 'cat' on the script and if it isn't present in the
> sdk the cat command waits on the std input and hence the installation
> process simply sits there.

That sort of error would typically be caused by the variable being
unset rather than the variable being set to the path of a nonexistent
file. I don't know much about the context this script runs in though
so I may be missing something obvious.

>
> Signed-off-by: Awais Belal <awais_belal@mentor.com>
> ---
>  meta/files/toolchain-shar-relocate.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/meta/files/toolchain-shar-relocate.sh b/meta/files/toolchain-shar-relocate.sh
> index e3c10018ef..02a05664c6 100644
> --- a/meta/files/toolchain-shar-relocate.sh
> +++ b/meta/files/toolchain-shar-relocate.sh
> @@ -3,6 +3,12 @@ if ! xargs --version > /dev/null 2>&1; then
>         exit 1
>  fi
>
> +# check if we have a valid env-setup script
> +if [ ! -f "$env_setup_script" ]; then
> +       echo "Main environment-setup file not found. Abort!"
> +       exit 1
> +fi
> +
>  # fix dynamic loader paths in all ELF SDK binaries
>  native_sysroot=$($SUDO_EXEC cat $env_setup_script |grep 'OECORE_NATIVE_SYSROOT='|cut -d'=' -f2|tr -d '"')
>  dl_path=$($SUDO_EXEC find $native_sysroot/lib -name "ld-linux*")
> --
> 2.17.1



-- 
Paul Barker
Konsulko Group

Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Mikko Rapeli]

On Thu, Jun 18, 2020 at 08:50:21AM +0100, Paul Barker wrote:
> On Wed, 17 Jun 2020 at 21:48, Awais Belal <Awais_Belal@mentor.com> wrote:
> >
> > The script runs a 'cat' on the script and if it isn't present in the
> > sdk the cat command waits on the std input and hence the installation
> > process simply sits there.
> 
> That sort of error would typically be caused by the variable being
> unset rather than the variable being set to the path of a nonexistent
> file. I don't know much about the context this script runs in though
> so I may be missing something obvious.

I'd change the script to run with bash and "set -euo pipefail" to capture
errors like this early.

-Mikko

> >
> > Signed-off-by: Awais Belal <awais_belal@mentor.com>
> > ---
> >  meta/files/toolchain-shar-relocate.sh | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/meta/files/toolchain-shar-relocate.sh b/meta/files/toolchain-shar-relocate.sh
> > index e3c10018ef..02a05664c6 100644
> > --- a/meta/files/toolchain-shar-relocate.sh
> > +++ b/meta/files/toolchain-shar-relocate.sh
> > @@ -3,6 +3,12 @@ if ! xargs --version > /dev/null 2>&1; then
> >         exit 1
> >  fi
> >
> > +# check if we have a valid env-setup script
> > +if [ ! -f "$env_setup_script" ]; then
> > +       echo "Main environment-setup file not found. Abort!"
> > +       exit 1
> > +fi
> > +
> >  # fix dynamic loader paths in all ELF SDK binaries
> >  native_sysroot=$($SUDO_EXEC cat $env_setup_script |grep 'OECORE_NATIVE_SYSROOT='|cut -d'=' -f2|tr -d '"')
> >  dl_path=$($SUDO_EXEC find $native_sysroot/lib -name "ld-linux*")
> > --
> > 2.17.1
> 
> 
> 
> -- 
> Paul Barker
> Konsulko Group

> 

Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Awais Belal]

> That sort of error would typically be caused by the variable being
> unset rather than the variable being set to the path of a nonexistent
> file. I don't know much about the context this script runs in though
> so I may be missing something obvious.

You're right but a "! -f" takes care of both the cases where the var is unset of the file isn't present.

BR,
Awais

________________________________________
From: Paul Barker <pbarker@konsulko.com>
Sent: Thursday, June 18, 2020 12:50 PM
To: Belal, Awais
Cc: openembedded-core
Subject: Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

On Wed, 17 Jun 2020 at 21:48, Awais Belal <Awais_Belal@mentor.com> wrote:
>
> The script runs a 'cat' on the script and if it isn't present in the
> sdk the cat command waits on the std input and hence the installation
> process simply sits there.

That sort of error would typically be caused by the variable being
unset rather than the variable being set to the path of a nonexistent
file. I don't know much about the context this script runs in though
so I may be missing something obvious.

>
> Signed-off-by: Awais Belal <awais_belal@mentor.com>
> ---
>  meta/files/toolchain-shar-relocate.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/meta/files/toolchain-shar-relocate.sh b/meta/files/toolchain-shar-relocate.sh
> index e3c10018ef..02a05664c6 100644
> --- a/meta/files/toolchain-shar-relocate.sh
> +++ b/meta/files/toolchain-shar-relocate.sh
> @@ -3,6 +3,12 @@ if ! xargs --version > /dev/null 2>&1; then
>         exit 1
>  fi
>
> +# check if we have a valid env-setup script
> +if [ ! -f "$env_setup_script" ]; then
> +       echo "Main environment-setup file not found. Abort!"
> +       exit 1
> +fi
> +
>  # fix dynamic loader paths in all ELF SDK binaries
>  native_sysroot=$($SUDO_EXEC cat $env_setup_script |grep 'OECORE_NATIVE_SYSROOT='|cut -d'=' -f2|tr -d '"')
>  dl_path=$($SUDO_EXEC find $native_sysroot/lib -name "ld-linux*")
> --
> 2.17.1



--
Paul Barker
Konsulko Group

Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Awais Belal]

> I'd change the script to run with bash and "set -euo pipefail" to capture
> errors like this early.

The toolchain-shar-relocate script comes in through the replacement of SDK_PRE_INSTALL_COMMAND in toolchain-shar-extract so it cannot decide to use bash itself. Also, I don't think it would be a good idea to introduce a specific shell binding for such scripts.

BR,
Awais

________________________________________
From: Mikko.Rapeli@bmw.de <Mikko.Rapeli@bmw.de>
Sent: Thursday, June 18, 2020 1:04 PM
To: pbarker@konsulko.com
Cc: Belal, Awais; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

On Thu, Jun 18, 2020 at 08:50:21AM +0100, Paul Barker wrote:
> On Wed, 17 Jun 2020 at 21:48, Awais Belal <Awais_Belal@mentor.com> wrote:
> >
> > The script runs a 'cat' on the script and if it isn't present in the
> > sdk the cat command waits on the std input and hence the installation
> > process simply sits there.
>
> That sort of error would typically be caused by the variable being
> unset rather than the variable being set to the path of a nonexistent
> file. I don't know much about the context this script runs in though
> so I may be missing something obvious.

I'd change the script to run with bash and "set -euo pipefail" to capture
errors like this early.

-Mikko

> >
> > Signed-off-by: Awais Belal <awais_belal@mentor.com>
> > ---
> >  meta/files/toolchain-shar-relocate.sh | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/meta/files/toolchain-shar-relocate.sh b/meta/files/toolchain-shar-relocate.sh
> > index e3c10018ef..02a05664c6 100644
> > --- a/meta/files/toolchain-shar-relocate.sh
> > +++ b/meta/files/toolchain-shar-relocate.sh
> > @@ -3,6 +3,12 @@ if ! xargs --version > /dev/null 2>&1; then
> >         exit 1
> >  fi
> >
> > +# check if we have a valid env-setup script
> > +if [ ! -f "$env_setup_script" ]; then
> > +       echo "Main environment-setup file not found. Abort!"
> > +       exit 1
> > +fi
> > +
> >  # fix dynamic loader paths in all ELF SDK binaries
> >  native_sysroot=$($SUDO_EXEC cat $env_setup_script |grep 'OECORE_NATIVE_SYSROOT='|cut -d'=' -f2|tr -d '"')
> >  dl_path=$($SUDO_EXEC find $native_sysroot/lib -name "ld-linux*")
> > --
> > 2.17.1
>
>
>
> --
> Paul Barker
> Konsulko Group

> 

Re: [OE-core] [PATCH] toolchain-shar-relocate.sh: check for environment-setup beforehand

[Mikko Rapeli]

Hi,

On Thu, Jun 18, 2020 at 10:13:34AM +0000, Belal, Awais wrote:
> > I'd change the script to run with bash and "set -euo pipefail" to capture
> > errors like this early.
> 
> The toolchain-shar-relocate script comes in through the replacement of SDK_PRE_INSTALL_COMMAND in toolchain-shar-extract so it cannot decide to use bash itself. Also, I don't think it would be a good idea to introduce a specific shell binding for such scripts.

I've come to the conclusion over the last 20 years that only way to maintain
sanity with shell scripts is "set -euxo pipefail" right after #!/bin/bash :)

Might work with other shells too but on Linux (build) systems bash is always there.
Tiny embedded targets are a different story...

But you decide which path to take in this case.

Cheers,

-Mikko