Re: [PATCH] edksetup.sh: Update help section regarding positional

Re: [PATCH] edksetup.sh: Update help section regarding positional

[Gao, Liming]

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

Re: [PATCH] edksetup.sh: Update help section regarding positional

[Arvind Prasanna]

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
>
>
>

[PATCH v2] edksetup.sh: Update help section regarding positional parameters

[Arvind Prasanna]

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()
-- 
2.7.4

Re: [PATCH] edksetup.sh: Update help section regarding positional

[Arvind Prasanna]

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
>>
>>
>>
>
>

Re: [PATCH v2] edksetup.sh: Update help section regarding positional parameters

[Arvind Prasanna]

Please ignore this and follow my original email thread. Git did not chain
this as I had expected.

- Arvind.



On Tue, Mar 13, 2018 at 1:38 AM, Arvind Prasanna <arvindprasanna@gmail.com>
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.
>
> 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()
> --
> 2.7.4
>
>

Re: [PATCH v2] edksetup.sh: Update help section regarding positional parameters

[Gao, Liming]

Arvind:
  The caller script may set the supported parameters by edksetup.sh. So, the notes may be updated not to set the unknown parameter.

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Arvind Prasanna
> Sent: Tuesday, March 13, 2018 1:44 PM
> To: Arvind Prasanna <arvindprasanna@gmail.com>
> Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH v2] edksetup.sh: Update help section regarding positional parameters
> 
> Please ignore this and follow my original email thread. Git did not chain
> this as I had expected.
> 
> - Arvind.
> 
> 
> 
> On Tue, Mar 13, 2018 at 1:38 AM, Arvind Prasanna <arvindprasanna@gmail.com>
> 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.
> >
> > 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()
> > --
> > 2.7.4
> >
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2] edksetup.sh: Update help section regarding positional parameters

[Arvind Prasanna]

Hi Liming:

Thank you for your reply. Yes, I agree that it is OK to pass supported
positional parameters through a sourcing script. I will reword my patch
considering this.

Thanks!

-Arvind.


-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

I want  to include the same example from the v1 chain for the record here:


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.



On Tue, Mar 13, 2018 at 9:44 AM, Gao, Liming <liming.gao@intel.com> wrote:

> Arvind:
>   The caller script may set the supported parameters by edksetup.sh. So,
> the notes may be updated not to set the unknown parameter.
>
> Thanks
> Liming
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Arvind Prasanna
> > Sent: Tuesday, March 13, 2018 1:44 PM
> > To: Arvind Prasanna <arvindprasanna@gmail.com>
> > Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [PATCH v2] edksetup.sh: Update help section
> regarding positional parameters
> >
> > Please ignore this and follow my original email thread. Git did not chain
> > this as I had expected.
> >
> > - Arvind.
> >
> >
> >
> > On Tue, Mar 13, 2018 at 1:38 AM, Arvind Prasanna <
> arvindprasanna@gmail.com>
> > 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.
> > >
> > > 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()
> > > --
> > > 2.7.4
> > >
> > >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
>

[PATCH v3] edksetup.sh: Update help section regarding positional parameters

[Arvind Prasanna]

It is possible to source edksetup.sh from another script. If the
sourcing script has any set positional parameters that are not supported
by edksetup.sh, the help section is always shown. This patch updates
the help section informing the user about these possible unsupported set
positional parameters, so that, 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 v3:
- Modify the help section to cover only unsupported positional parameters.
- Adapt commit message to the change in the above line

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..6a590df 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, the sourcing script"
+  echo "should have no set positional parameters unsupported by this script."
 }
 
 function SetWorkspace()
-- 
2.7.4