Re: [tianocore/edk2] edksetup.sh: rework argument parsing and update usage information (44f7942)

[Leif Lindholm <leif.lindholm@linaro.org>]

Hi Dmitry,

On 25 October 2016 at 14:06, Dmitry <notifications@github.com> wrote:
> https://github.com/tianocore/edk2/blob/master/edksetup.sh#L149
> ./edksetup.sh: line 149: return: can only `return' from a function or sourced script

We don't use github for bugtracking - reports can be made either to
edk2-devel@lists.01.org (added to cc) or on
https://bugzilla.tianocore.org/.

Yes. edksetup.sh must always be run as a sourced script, via 'source'
or '.', as stated in the usage message.

But I concede there is a change in behaviour when invoking it
incorrectly. And the generated error message could be better.

I propose the following update to improve the situation:

>From b24a4ef698ea993535b80dfaee340bd5b70e9f91 Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindholm@linaro.org>
Date: Tue, 25 Oct 2016 18:20:04 +0100
Subject: [PATCH] edksetup.sh: refactor source/execution detection

Commit 44f79425589e ("edksetup.sh: rework argument parsing and...")
added a return call to the top level of the script, triggering the
not entirely user friendly error message
"line 149: return: can only `return' from a function or sourced script"
when the script is executed rather than sourced.

Move the existing test for erroneous invocation first in SetWorkspace()
and add a test that exits rather than returns when the script is
executed directly.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---

For further improvements, I could also change the last hardcoded
edksetup.sh to $SCRIPTNAME, but preferred minimal changes to the
modified code.

 edksetup.sh | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/edksetup.sh b/edksetup.sh
index ec54f9e..634ff99 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -46,6 +46,14 @@ function HelpMsg()

 function SetWorkspace()
 {
+  if [ ! ${BASH_SOURCE[0]} -ef ./edksetup.sh ]
+  then
+    echo Run this script from the base of your tree.  For example:
+    echo "  cd /Path/To/Edk/Root"
+    echo "  . edksetup.sh"
+    exit 1
+  fi
+
   #
   # If WORKSPACE is already set, then we can return right now
   #
@@ -54,14 +62,6 @@ function SetWorkspace()
     return 0
   fi

-  if [ ! ${BASH_SOURCE[0]} -ef ./edksetup.sh ]
-  then
-    echo Run this script from the base of your tree.  For example:
-    echo "  cd /Path/To/Edk/Root"
-    echo "  . edksetup.sh"
-    return 1
-  fi
-
   #
   # Check for BaseTools/BuildEnv before dirtying the user's environment.
   #
@@ -117,6 +117,8 @@ function SourceEnv()
   SetupEnv
 }

+[ "$0" = "./$SCRIPTNAME" ] && HelpMsg && exit 1
+
 I=$#
 while [ $I -gt 0 ]
 do
-- 
2.9.3