* [PATCH] edksetup.sh: Update help section regarding positional @ 2018-03-10 16:11 Arvind Prasanna 2018-03-14 11:25 ` Leif Lindholm 0 siblings, 1 reply; 6+ messages in thread From: Arvind Prasanna @ 2018-03-10 16:11 UTC (permalink / raw) To: edk2-devel It is possible to source edksetup.sh from another script. If the calling/sourcing script has any positional parameters set, those are incorrectly accounted for in edksetup.sh while sourcing it resulting in the the help section always being shown. This patch updates the help section advising the user about these set positional parameters so they can be unset prior to sourcing edksetup.sh. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> --- edksetup.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/edksetup.sh b/edksetup.sh index 93d6525..a3d5560 100755 --- a/edksetup.sh +++ b/edksetup.sh @@ -42,6 +42,8 @@ function HelpMsg() echo Please note: This script must be \'sourced\' so the environment can be changed. echo ". $SCRIPTNAME" echo "source $SCRIPTNAME" + echo "If this script is being sourced from another script, please ensure that the" + echo "sourcing/calling script has no set postional parameters." } function SetWorkspace() -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] edksetup.sh: Update help section regarding positional 2018-03-10 16:11 [PATCH] edksetup.sh: Update help section regarding positional Arvind Prasanna @ 2018-03-14 11:25 ` Leif Lindholm 2018-03-16 6:18 ` Arvind Prasanna 0 siblings, 1 reply; 6+ messages in thread From: Leif Lindholm @ 2018-03-14 11:25 UTC (permalink / raw) To: Arvind Prasanna; +Cc: edk2-devel Hi Arvind, On Sat, Mar 10, 2018 at 11:11:58AM -0500, Arvind Prasanna wrote: > It is possible to source edksetup.sh from another script. If the > calling/sourcing script has any positional parameters set, those are > incorrectly accounted for in edksetup.sh while sourcing it resulting in > the the help section always being shown. This patch updates the help > section advising the user about these set positional parameters so they > can be unset prior to sourcing edksetup.sh. This is really just one of the unpleasantries of sourcing shell scripts. Since the current script could only ever work with bash anyway (and not sh, dash, ...) I don't know that we could do much better - and there's nothing about the problem that is specific to this script. As an aside, since we _know_ this only works on bash, you can also make use of the bash side effect that if you pass any arguments to the sourced script, it gets its own copies of $#, $0, $1 and so on. For ancient backwards compatility, the parameter BaseTools is ignored if provided on the command line. So you could always just use edksetup.sh BaseTools Alternatively, you can use edksetup.sh --reconfig which also ensures you are always using the latest toolchain templates. Regards, Leif > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> > --- > edksetup.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/edksetup.sh b/edksetup.sh > index 93d6525..a3d5560 100755 > --- a/edksetup.sh > +++ b/edksetup.sh > @@ -42,6 +42,8 @@ function HelpMsg() > echo Please note: This script must be \'sourced\' so the environment can be changed. > echo ". $SCRIPTNAME" > echo "source $SCRIPTNAME" > + echo "If this script is being sourced from another script, please ensure that the" > + echo "sourcing/calling script has no set postional parameters." > } > > function SetWorkspace() > -- > 2.7.4 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] edksetup.sh: Update help section regarding positional 2018-03-14 11:25 ` Leif Lindholm @ 2018-03-16 6:18 ` Arvind Prasanna 0 siblings, 0 replies; 6+ messages in thread From: Arvind Prasanna @ 2018-03-16 6:18 UTC (permalink / raw) To: Leif Lindholm; +Cc: edk2-devel Hi Leif: Thank you for your feedback. I concur with you that it is a bash side effect, desired or undesired :) . I ran into this issue with positional parameters and thought a small message would be helpful to users. I will no longer pursue this issue. Thanks, Arvind. On Wed, Mar 14, 2018 at 7:25 AM, Leif Lindholm <leif.lindholm@linaro.org> wrote: > Hi Arvind, > > On Sat, Mar 10, 2018 at 11:11:58AM -0500, Arvind Prasanna wrote: > > It is possible to source edksetup.sh from another script. If the > > calling/sourcing script has any positional parameters set, those are > > incorrectly accounted for in edksetup.sh while sourcing it resulting in > > the the help section always being shown. This patch updates the help > > section advising the user about these set positional parameters so they > > can be unset prior to sourcing edksetup.sh. > > This is really just one of the unpleasantries of sourcing shell > scripts. > > Since the current script could only ever work with bash anyway (and > not sh, dash, ...) I don't know that we could do much better - and > there's nothing about the problem that is specific to this script. > > As an aside, since we _know_ this only works on bash, you can also > make use of the bash side effect that if you pass any arguments to the > sourced script, it gets its own copies of $#, $0, $1 and so on. > > For ancient backwards compatility, the parameter BaseTools is ignored > if provided on the command line. So you could always just use > edksetup.sh BaseTools > > Alternatively, you can use > edksetup.sh --reconfig > which also ensures you are always using the latest toolchain > templates. > > Regards, > > Leif > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> > > --- > > edksetup.sh | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/edksetup.sh b/edksetup.sh > > index 93d6525..a3d5560 100755 > > --- a/edksetup.sh > > +++ b/edksetup.sh > > @@ -42,6 +42,8 @@ function HelpMsg() > > echo Please note: This script must be \'sourced\' so the environment > can be changed. > > echo ". $SCRIPTNAME" > > echo "source $SCRIPTNAME" > > + echo "If this script is being sourced from another script, please > ensure that the" > > + echo "sourcing/calling script has no set postional parameters." > > } > > > > function SetWorkspace() > > -- > > 2.7.4 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1520669933-8602-1-git-send-email-arvindprasanna@gmail.com>]
[parent not found: <CAK-=9LqFN3c_ZOnFO=fAnnk6wqKxmbF+LXhek2x6tkXNB37e-g@mail.gmail.com>]
* Re: [PATCH] edksetup.sh: Update help section regarding positional [not found] ` <CAK-=9LqFN3c_ZOnFO=fAnnk6wqKxmbF+LXhek2x6tkXNB37e-g@mail.gmail.com> @ 2018-03-12 14:55 ` Gao, Liming 2018-03-12 17:17 ` Arvind Prasanna 0 siblings, 1 reply; 6+ messages in thread From: Gao, Liming @ 2018-03-12 14:55 UTC (permalink / raw) To: Arvind Prasanna, Zhu, Yonghong; +Cc: edk2-devel@lists.01.org Arvind: Could you give one example to explain this usage? ---------- Forwarded message ---------- From: Arvind Prasanna <arvindprasanna@gmail.com<mailto:arvindprasanna@gmail.com>> Date: Sat, Mar 10, 2018 at 3:18 AM Subject: [PATCH] edksetup.sh: Update help section regarding positional To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Arvind Prasanna <arvindprasanna@gmail.com<mailto:arvindprasanna@gmail.com>> It is possible to source edksetup.sh from another script. If the calling/sourcing script has any positional parameters set, those are incorrectly accounted for in edksetup.sh while sourcing it resulting in the the help section always being shown. This patch updates the help section advising the user about these set positional parameters so they can be unset prior to sourcing edksetup.sh. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com<mailto:arvindprasanna@gmail.com>> --- edksetup.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/edksetup.sh b/edksetup.sh index 93d6525..a3d5560 100755 --- a/edksetup.sh +++ b/edksetup.sh @@ -42,6 +42,8 @@ function HelpMsg() echo Please note: This script must be \'sourced\' so the environment can be changed. echo ". $SCRIPTNAME" echo "source $SCRIPTNAME" + echo "If this script is being sourced from another script, please ensure that the" + echo "sourcing/calling script has no set postional parameters." } function SetWorkspace() -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] edksetup.sh: Update help section regarding positional 2018-03-12 14:55 ` Gao, Liming @ 2018-03-12 17:17 ` Arvind Prasanna 2018-03-13 5:41 ` Arvind Prasanna 0 siblings, 1 reply; 6+ messages in thread From: Arvind Prasanna @ 2018-03-12 17:17 UTC (permalink / raw) To: Gao, Liming; +Cc: Zhu, Yonghong, edk2-devel@lists.01.org Hi Liming Gao: Here is a simple example that highlights the issue: $ ls -l total 8 drwxrwxr-x 49 arvind arvind 4096 Mar 12 12:35 edk2 -rwxrwxr-x 1 arvind arvind 113 Mar 12 12:37 sourcing_script.sh $ cat sourcing_script.sh #!/bin/bash echo "I have been passed $# arguments" EDK2_PATH=${PWD}/../edk2 cd ${EDK2_PATH} source edksetup.sh Case 1) Let us call sourcing_script.sh with no arguments $ ./sourcing_script.sh I have been passed 0 arguments Loading previous configuration from /home/arvind/edk2/Conf/BuildEnv.sh WORKSPACE: /home/arvind/edk2 EDK_TOOLS_PATH: /home/arvind/edk2/BaseTools CONF_PATH: /home/arvind/edk2/Conf Everything looks good. Case 2) Let us call sourcing_script.sh with say two arguments $ ./sourcing_script.sh arg1 arg2 I have been passed 2 arguments Usage: edksetup.sh [Options] The system environment variable, WORKSPACE, is always set to the current working directory. Options: --help, -h, -? Print this help screen and exit. --reconfig Overwrite the WORKSPACE/Conf/*.txt files with the template files from the BaseTools/Conf directory. Please note: This script must be 'sourced' so the environment can be changed. . edksetup.sh source edksetup.sh This is the case I intend to bring out. I have not passed any arguments (intentionally) to edksetup.sh. but it always defaults to the help case with the current edksetup.sh script. In edksetup.sh in line 120, it counts the number of positional parameters "I=$#" and then does a switch-case on these arguments. In case 2, these arguments are "arg1" and "arg2" which has nothing to do with edksetup.sh. If I were to clear all the positional arguments passed to sourcing_script.sh, using "shift $#" prior to sourcing edk2setup.sh, it works as expected. I do not want to fix anything in the code as this might not be a common case but I would like to let the user know that all the positional parameters that exist in the caller script will affect sourcing edksetup.sh. This is how I would recommend the help section be with my patch: $ ./sourcing_script.sh arg1 arg2 I have been passed 2 arguments Usage: edksetup.sh [Options] The system environment variable, WORKSPACE, is always set to the current working directory. Options: --help, -h, -? Print this help screen and exit. --reconfig Overwrite the WORKSPACE/Conf/*.txt files with the template files from the BaseTools/Conf directory. Please note: This script must be 'sourced' so the environment can be changed. . edksetup.sh source edksetup.sh If this script is being sourced from another script, please ensure that the sourcing/calling script has no set postional parameters. Please let me know your feedback. Thanks! - Arvind. P.S. I just realized my patch has a typo in the word "positional". I will fix it. On Mon, Mar 12, 2018 at 10:55 AM, Gao, Liming <liming.gao@intel.com> wrote: > Arvind: > > Could you give one example to explain this usage? > > > > ---------- Forwarded message ---------- > From: *Arvind Prasanna* <arvindprasanna@gmail.com> > Date: Sat, Mar 10, 2018 at 3:18 AM > Subject: [PATCH] edksetup.sh: Update help section regarding positional > To: edk2-devel@lists.01.org > Cc: Arvind Prasanna <arvindprasanna@gmail.com> > > > It is possible to source edksetup.sh from another script. If the > calling/sourcing script has any positional parameters set, those are > incorrectly accounted for in edksetup.sh while sourcing it resulting in > the the help section always being shown. This patch updates the help > section advising the user about these set positional parameters so they > can be unset prior to sourcing edksetup.sh. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> > --- > edksetup.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/edksetup.sh b/edksetup.sh > index 93d6525..a3d5560 100755 > --- a/edksetup.sh > +++ b/edksetup.sh > @@ -42,6 +42,8 @@ function HelpMsg() > echo Please note: This script must be \'sourced\' so the environment > can be changed. > echo ". $SCRIPTNAME" > echo "source $SCRIPTNAME" > + echo "If this script is being sourced from another script, please > ensure that the" > + echo "sourcing/calling script has no set postional parameters." > } > > function SetWorkspace() > -- > 2.7.4 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] edksetup.sh: Update help section regarding positional 2018-03-12 17:17 ` Arvind Prasanna @ 2018-03-13 5:41 ` Arvind Prasanna 0 siblings, 0 replies; 6+ messages in thread From: Arvind Prasanna @ 2018-03-13 5:41 UTC (permalink / raw) To: Gao, Liming; +Cc: Zhu, Yonghong, edk2-devel@lists.01.org I am not sure why git send mail did not include my patch in the chain but here is my version 2: It is possible to source edksetup.sh from another script. If the calling/sourcing script has any positional parameters set, those are incorrectly accounted for in edksetup.sh while sourcing it resulting in the the help section always being shown. This patch updates the help section advising the user about these set positional parameters so they can be unset prior to sourcing edksetup.sh. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> Changes in v2: - Fixed a typo. - Minor rewording. --- edksetup.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/edksetup.sh b/edksetup.sh index 93d6525..e85fbf2 100755 --- a/edksetup.sh +++ b/edksetup.sh @@ -42,6 +42,8 @@ function HelpMsg() echo Please note: This script must be \'sourced\' so the environment can be changed. echo ". $SCRIPTNAME" echo "source $SCRIPTNAME" + echo "If this script is being sourced from another script, please ensure that the" + echo "sourcing script has no set positional parameters." } function SetWorkspace() Thanks! -Arvind. On Mon, Mar 12, 2018 at 1:17 PM, Arvind Prasanna <arvindprasanna@gmail.com> wrote: > Hi Liming Gao: > > Here is a simple example that highlights the issue: > > > $ ls -l > total 8 > drwxrwxr-x 49 arvind arvind 4096 Mar 12 12:35 edk2 > -rwxrwxr-x 1 arvind arvind 113 Mar 12 12:37 sourcing_script.sh > > $ cat sourcing_script.sh > > #!/bin/bash > echo "I have been passed $# arguments" > EDK2_PATH=${PWD}/../edk2 > cd ${EDK2_PATH} > source edksetup.sh > > > Case 1) Let us call sourcing_script.sh with no arguments > > $ ./sourcing_script.sh > I have been passed 0 arguments > Loading previous configuration from /home/arvind/edk2/Conf/BuildEnv.sh > WORKSPACE: /home/arvind/edk2 > EDK_TOOLS_PATH: /home/arvind/edk2/BaseTools > CONF_PATH: /home/arvind/edk2/Conf > > Everything looks good. > > > Case 2) Let us call sourcing_script.sh with say two arguments > > $ ./sourcing_script.sh arg1 arg2 > I have been passed 2 arguments > Usage: edksetup.sh [Options] > > The system environment variable, WORKSPACE, is always set to the current > working directory. > > Options: > --help, -h, -? Print this help screen and exit. > > --reconfig Overwrite the WORKSPACE/Conf/*.txt files with the > template files from the BaseTools/Conf directory. > > Please note: This script must be 'sourced' so the environment can be > changed. > . edksetup.sh > source edksetup.sh > > > This is the case I intend to bring out. I have not passed any arguments > (intentionally) to edksetup.sh. but it always defaults to the help case > with the current edksetup.sh script. > > In edksetup.sh in line 120, it counts the number of positional parameters > "I=$#" and then does a switch-case on these arguments. In case 2, these > arguments are "arg1" and "arg2" which has nothing to do with edksetup.sh. > > If I were to clear all the positional arguments passed to > sourcing_script.sh, using "shift $#" prior to sourcing edk2setup.sh, it > works as expected. I do not want to fix anything in the code as this might > not be a common case but I would like to let the user know that all the > positional parameters that exist in the caller script will affect sourcing > edksetup.sh. > > > > This is how I would recommend the help section be with my patch: > > $ ./sourcing_script.sh arg1 arg2 > I have been passed 2 arguments > Usage: edksetup.sh [Options] > > The system environment variable, WORKSPACE, is always set to the current > working directory. > > Options: > --help, -h, -? Print this help screen and exit. > > --reconfig Overwrite the WORKSPACE/Conf/*.txt files with the > template files from the BaseTools/Conf directory. > > Please note: This script must be 'sourced' so the environment can be > changed. > . edksetup.sh > source edksetup.sh > If this script is being sourced from another script, please ensure that the > sourcing/calling script has no set postional parameters. > > Please let me know your feedback. > > Thanks! > > - Arvind. > > P.S. I just realized my patch has a typo in the word "positional". I will > fix it. > > On Mon, Mar 12, 2018 at 10:55 AM, Gao, Liming <liming.gao@intel.com> > wrote: > >> Arvind: >> >> Could you give one example to explain this usage? >> >> >> >> ---------- Forwarded message ---------- >> From: *Arvind Prasanna* <arvindprasanna@gmail.com> >> Date: Sat, Mar 10, 2018 at 3:18 AM >> Subject: [PATCH] edksetup.sh: Update help section regarding positional >> To: edk2-devel@lists.01.org >> Cc: Arvind Prasanna <arvindprasanna@gmail.com> >> >> >> It is possible to source edksetup.sh from another script. If the >> calling/sourcing script has any positional parameters set, those are >> incorrectly accounted for in edksetup.sh while sourcing it resulting in >> the the help section always being shown. This patch updates the help >> section advising the user about these set positional parameters so they >> can be unset prior to sourcing edksetup.sh. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Arvind Prasanna <arvindprasanna@gmail.com> >> --- >> edksetup.sh | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/edksetup.sh b/edksetup.sh >> index 93d6525..a3d5560 100755 >> --- a/edksetup.sh >> +++ b/edksetup.sh >> @@ -42,6 +42,8 @@ function HelpMsg() >> echo Please note: This script must be \'sourced\' so the environment >> can be changed. >> echo ". $SCRIPTNAME" >> echo "source $SCRIPTNAME" >> + echo "If this script is being sourced from another script, please >> ensure that the" >> + echo "sourcing/calling script has no set postional parameters." >> } >> >> function SetWorkspace() >> -- >> 2.7.4 >> >> >> > > ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-16 6:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-10 16:11 [PATCH] edksetup.sh: Update help section regarding positional Arvind Prasanna 2018-03-14 11:25 ` Leif Lindholm 2018-03-16 6:18 ` Arvind Prasanna [not found] <1520669933-8602-1-git-send-email-arvindprasanna@gmail.com> [not found] ` <CAK-=9LqFN3c_ZOnFO=fAnnk6wqKxmbF+LXhek2x6tkXNB37e-g@mail.gmail.com> 2018-03-12 14:55 ` Gao, Liming 2018-03-12 17:17 ` Arvind Prasanna 2018-03-13 5:41 ` Arvind Prasanna
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox