diff mbox

[edk2] edk2/edksetup.sh patch to solve command line parameter

Message ID CAF7UmSyXsjp-TmKLKXhKcMEoOkv6PAbRjPSCkTwOv=A8J94R3Q@mail.gmail.com
State New
Headers show

Commit Message

Leif Lindholm Jan. 29, 2014, 4:50 p.m. UTC
Actually, there is another issue with the new edksetup.sh - it returns
success (0) regardless of whether the executions succeeds or not.

The attached patch resolves both of these issues.

Contributed-under: TianoCore Contribution Agreement 1.0


On 29 January 2014 16:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> Hi Liming,
>
> This commit (15192) does not do what was described.
>
> It permits '. edksetup.sh BaseTool', rather than what was described in the
> text - '. edksetup.sh BaseTools'. (missing a trailing 's')
>
> Regards,
>
> Leif
>
>
> On 27 January 2014 07:46, Gao, Liming <liming.gao@intel.com> wrote:
>
>>  Parmeshwr:
>>
>>   Your patch is good. I will help commit it.
>>
>>   Signed-off-by: Gao, Liming <liming.gao@intel.com>
>>
>>
>>
>> Thanks
>>
>> Liming
>>
>> *From:* Parmeshwr_Prasad@Dell.com [mailto:Parmeshwr_Prasad@Dell.com]
>> *Sent:* Monday, January 27, 2014 3:00 PM
>> *To:* Gao, Liming
>> *Cc:* edk2-commits@lists.sourceforge.net
>> *Subject:* RE: edk2/edksetup.sh patch to solve command line parameter
>>
>>
>>
>> Hi Liming,
>>
>>
>>
>> Please find new patch for edksetupp.sh.
>>
>>
>>
>> I have changes according to your comments.
>>
>> It can handle following cases.
>>
>> 1-      Handle more than one parameter
>>
>> 2-      Handle if first parameter is not "-?, -h, --help or BaseTool".
>>
>> 3-      Any other thing to display error message.
>>
>>
>>
>> Please let me know with your comment.
>>
>>
>>
>> Regards
>>
>> Parmeshwr Prasad
>>
>>
>>
>> *From:* Gao, Liming [mailto:liming.gao@intel.com <liming.gao@intel.com>]
>> *Sent:* Friday, January 24, 2014 8:01 PM
>> *To:* Prasad, Parmeshwr
>> *Subject:* RE: edk2/edksetup.sh patch to solve command line parameter
>>
>>
>>
>> Yes. If user follows it, it should work. So, I expect the behavior is:
>>
>> 1.       No parameter, edksetup.sh will set up environment.
>>
>> 2.       BaseTools parameter, edksetup.sh will set up environment.
>>
>> 3.       Other parameter, edksetup.sh will print help message.
>>
>>
>>
>> Thanks
>>
>> Liming
>>
>> *From:* Parmeshwr_Prasad@Dell.com [mailto:Parmeshwr_Prasad@Dell.com<Parmeshwr_Prasad@Dell.com>]
>>
>> *Sent:* Friday, January 24, 2014 7:10 PM
>> *To:* Gao, Liming
>> *Subject:* RE: edk2/edksetup.sh patch to solve command line parameter
>>
>>
>>
>> Hi Liming
>>
>>
>>
>> I got your point. I saw user manual do we give any other parameter except
>> "*BaseTools"*
>>
>> In parameter to edksetup.sh ?
>>
>>
>>
>> If I am not wrong than this is the point you are talking about.
>>
>> *ln -s /home/usr/BaseTools /home/usr/Edk2Workspace/Conf/BaseToolsSource*
>>
>> 4. Run "*. edksetup.sh BaseTools*" under the workspace's directory to set
>>
>> system environment, such as WORKSPACE, EDK_TOOLS_PATH etc.
>>
>>
>>
>> Regards
>>
>> Parmeshwr
>>
>>
>>
>> *From:* Prasad, Parmeshwr
>> *Sent:* Friday, January 24, 2014 4:26 PM
>> *To:* edk2-devel@lists.sourceforge.net; liming.gao@intel.com
>>
>> *Subject:* Re: [edk2] edk2/edksetup.sh patch to solve command line
>> parameter
>>
>>
>>
>> Hi Liming,
>>
>>
>>
>> See below two example-
>>
>>
>>
>>
>>
>> 1-      param@param-opensource:~/Development/edk2$ source edksetup.sh -h-
>>
>> Loading previous configuration from $WORKSPACE/Conf/BuildEnv.sh
>>
>> WORKSPACE: /home/param/Development/edk2
>>
>> EDK_TOOLS_PATH: /home/param/Development/edk2/BaseTools
>>
>>
>>
>> *In above example edksetup.sh is not able to handle "-h-" parameter it
>> mean it can handle only "-?, -h,--help".*
>>
>> *If we give any other parameter except above mentioned. It cannot handle.
>> It mean error handling is required.*
>>
>> *Even the help message is not looking good.*
>>
>>
>>
>>
>>
>> 2-      param@param-opensource:~/Development/edk2$ source edksetup.sh -h
>>
>> BaseTools Usage: '. edksetup.sh'
>>
>>
>>
>> Please note: This script must be 'sourced' so the environment can be
>> changed.
>>
>> (Either '. edksetup.sh' or 'source edksetup.sh')
>>
>>
>>
>> This is expected behavior.
>>
>>
>>
>> If this patch is not looking good, suggest me how it can be made better.
>>
>>
>>
>> Regards
>>
>> Parmeshwr
>>
>>
>>
>>
>>
>> *From:* Gao, Liming [mailto:liming.gao@intel.com <liming.gao@intel.com>]
>> *Sent:* Friday, January 24, 2014 3:06 PM
>> *To:* Prasad, Parmeshwr
>> *Cc:* edk2-devel@lists.sourceforge.net
>> *Subject:* Re: [edk2] edk2/edksetup.sh patch to solve command line
>> parameter
>>
>>
>>
>> Hi,
>>
>>   I have two comments.
>>
>> 1.       BaseTools parameter is required to be supported for
>> compatibility, because this usage is mentioned in EDKII_UserManual.pdf
>> document. Some users have used it. In fact, ". edksetup.sh BaseTools" is
>> same to ". edksetup.sh".
>>
>> 2.       In below script, BaseTools/BuildEnv $* can be cleanup to remove
>> $*, because no parameter is required.
>>
>>
>>
>> if [ -z "$WORKSPACE" ]
>>
>> then
>>
>>   . BaseTools/BuildEnv $*
>>
>> else
>>
>>   . $WORKSPACE/BaseTools/BuildEnv $*
>>
>> fi
>>
>>
>>
>> Thanks
>>
>> Liming
>>
>> *From:* Parmeshwr_Prasad@Dell.com [mailto:Parmeshwr_Prasad@Dell.com<Parmeshwr_Prasad@Dell.com>]
>>
>> *Sent:* Thursday, January 23, 2014 4:02 PM
>> *To:* edk2-commits@lists.sourceforge.net
>> *Subject:* edk2/edksetup.sh patch to solve command line parameter
>>
>>
>>
>> Hi All,
>>
>>
>>
>> I see there is a problem in "edksetup.sh" file. It accept one parameter
>> "-?, -h, --help" for printing help message.
>>
>> If we give any other parameter to this it goes and set old environment
>> with help message.  Expected behavior should be either to print
>>
>> The help message or set environment. It is not able to handle any garbage
>> parameter.
>>
>> I am sending patch for this problem, please review and commit to main
>> stream.
>>
>>
>>
>> Even help message was not clear I changes that also.
>>
>>
>>
>> *Incorrect behavior :*
>>
>> :~/Development/edk2$ source edksetup.sh ---jjdcncn
>>
>> Loading previous configuration from $WORKSPACE/Conf/BuildEnv.sh
>>
>> WORKSPACE: /home/param/Development/edk2
>>
>> EDK_TOOLS_PATH: /home/param/Development/edk2/BaseTools
>>
>>
>>
>> Correct behavior:
>>
>> :~/Development/edk2$ source edksetup.sh -h
>>
>> BaseTools Usage: '. edksetup.sh'
>>
>>
>>
>> Please note: This script must be 'sourced' so the environment can be
>> changed.
>>
>> (Either '. edksetup.sh' or 'source edksetup.sh')
>>
>>
>>
>> Correct behavior:
>>
>> :~/Development/edk2$ source edksetup.sh
>>
>> Loading previous configuration from $WORKSPACE/Conf/BuildEnv.sh
>>
>> WORKSPACE: /home/param/Development/edk2
>>
>> EDK_TOOLS_PATH: /home/param/Development/edk2/BaseTools
>>
>>
>>
>>
>>
>> *Best Regards,*
>>
>> *Parmeshwr Prasad*
>>
>> Tel : +91-9743262018
>>
>> [image: cid:image002.png@01CE781A.38F61FE0]
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> CenturyLink Cloud: The Leader in Enterprise Cloud Services.
>> Learn Why More Businesses Are Choosing CenturyLink Cloud For
>> Critical Workloads, Development Environments & Everything In Between.
>> Get a Quote or Start a Free Trial Today.
>>
>> http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>
>>
>
------------------------------------------------------------------------------
WatchGuard Dimension instantly turns raw network data into actionable 
security intelligence. It gives you real-time visual feedback on key
security issues and trends.  Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
diff mbox

Patch

From f15cbde4d459b2859eff3a8d2f62cd41727f886f Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindholm@linaro.org>
Date: Wed, 29 Jan 2014 16:46:31 +0000
Subject: [PATCH] fixes for new edksetup.sh

---
 edksetup.sh |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/edksetup.sh b/edksetup.sh
index 39b76e1..c1b57d7 100755
--- a/edksetup.sh
+++ b/edksetup.sh
@@ -30,6 +30,7 @@  function HelpMsg()
   echo Please note: This script must be \'sourced\' so the environment can be changed.
   echo ". edksetup.sh" 
   echo "source edksetup.sh"
+  return 1
 }
 
 function SetupEnv()
@@ -51,7 +52,6 @@  if [ \
    ]
 then
   HelpMsg
-  return
 else
   SetupEnv "$*"
 fi
@@ -59,11 +59,14 @@  fi
 if [ $# -gt 1 ] 
 then
   HelpMsg
-  return
-elif [ $# -eq 1 ] && [ "$1" != "BaseTool" ]
+elif [ $# -eq 1 ] && [ "$1" != "BaseTools" ]
 then
   HelpMsg
-  return
 fi 
+RETVAL=$?
+if [ $RETVAL -ne 0 ]
+then
+  return $RETVAL
+fi
 SourceEnv "$*"
 
-- 
1.7.10.4