Patchwork populate_sdk_base: add auto-completion in setup

login
register
mail settings
Submitter Dennis Meier
Date June 17, 2014, 1:16 p.m.
Message ID <1403011013-29977-1-git-send-email-meier.dennis@siemens.com>
Download mbox | patch
Permalink /patch/73945/
State New
Headers show

Comments

Dennis Meier - June 17, 2014, 1:16 p.m.
Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
---
 meta/classes/populate_sdk_base.bbclass |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Laurentiu Palcu - June 17, 2014, 2:03 p.m.
On Tue, Jun 17, 2014 at 03:16:53PM +0200, Dennis Meier wrote:
> Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
> ---
>  meta/classes/populate_sdk_base.bbclass |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
> index 35d837d..1182425 100644
> --- a/meta/classes/populate_sdk_base.bbclass
> +++ b/meta/classes/populate_sdk_base.bbclass
> @@ -173,9 +173,8 @@ if [ $verbose = 1 ] ; then
>  	set -x
>  fi
>  
> -printf "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): "
>  if [ "$target_sdk_dir" = "" ]; then
> -	read target_sdk_dir
> +	read -e -p "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): " target_sdk_dir
>  	[ "$target_sdk_dir" = "" ] && target_sdk_dir=$DEFAULT_INSTALL_DIR
>  else
>  	echo "$target_sdk_dir"
Moving the above printf in the 'if' branch will make this 'echo' look
strange when the target directory is provided with the -d option... I
believe you can remove this 'echo' completely though. There is another
message, later, informing the user where the SDK gets installed.

laurentiu

> -- 
> 1.7.10.4
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Otavio Salvador - June 17, 2014, 2:39 p.m.
On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier <meier.dennis@siemens.com> wrote:
> Signed-off-by: Dennis Meier <meier.dennis@siemens.com>

I do like the idea but how portable is it?

For example in my envinronment it does not work as I use zsh.
Richard Purdie - June 18, 2014, 10:15 a.m.
On Tue, 2014-06-17 at 11:39 -0300, Otavio Salvador wrote:
> On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier <meier.dennis@siemens.com> wrote:
> > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
> 
> I do like the idea but how portable is it?
> 
> For example in my envinronment it does not work as I use zsh.

The script in question has "/bin/bash" at the top so this shouldn't be
an issue?

Cheers,

Richard
Otavio Salvador - June 18, 2014, 11:29 a.m.
On Wed, Jun 18, 2014 at 7:15 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Tue, 2014-06-17 at 11:39 -0300, Otavio Salvador wrote:
>> On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier <meier.dennis@siemens.com> wrote:
>> > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
>>
>> I do like the idea but how portable is it?
>>
>> For example in my envinronment it does not work as I use zsh.
>
> The script in question has "/bin/bash" at the top so this shouldn't be
> an issue?

In this case I second it :-)
Dennis Meier - June 24, 2014, 10:01 a.m.
> -----Original Message-----
> From: otavio.salvador@gmail.com [mailto:otavio.salvador@gmail.com] On
> Behalf Of Otavio Salvador
> Sent: Dienstag, 17. Juni 2014 16:40
> To: Meier, Dennis
> Cc: Patches and discussions about the oe-core layer
> Subject: Re: [OE-core] [PATCH] populate_sdk_base: add auto-completion
> in setup
> 
> On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier
> <meier.dennis@siemens.com> wrote:
> > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
> 
> I do like the idea but how portable is it?
> 
> For example in my envinronment it does not work as I use zsh.

Yeah that's what I was afraid of. We figured that the script might still be invoked by bash because it starts with "#!/bin/bash" and thus it might work, but if you tested it and it didn't work for you I think it doesn't make sense to apply this patch.

Regards,
Dennis
Paul Eggleton - June 25, 2014, 4:26 p.m.
On Tuesday 24 June 2014 10:01:36 Meier, Dennis wrote:
> > -----Original Message-----
> > From: otavio.salvador@gmail.com [mailto:otavio.salvador@gmail.com] On
> > Behalf Of Otavio Salvador
> > Sent: Dienstag, 17. Juni 2014 16:40
> > To: Meier, Dennis
> > Cc: Patches and discussions about the oe-core layer
> > Subject: Re: [OE-core] [PATCH] populate_sdk_base: add auto-completion
> > in setup
> > 
> > On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier
> > 
> > <meier.dennis@siemens.com> wrote:
> > > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
> > 
> > I do like the idea but how portable is it?
> > 
> > For example in my envinronment it does not work as I use zsh.
> 
> Yeah that's what I was afraid of. We figured that the script might still be
> invoked by bash because it starts with "#!/bin/bash" and thus it might
> work, but if you tested it and it didn't work for you I think it doesn't
> make sense to apply this patch.

Well, it depends - Otavio, when you say "it does not work" did you test it? 
#!/bin/bash should ensure bash is actually used.

Cheers,
Paul
Otavio Salvador - June 25, 2014, 4:58 p.m.
On Wed, Jun 25, 2014 at 1:26 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> On Tuesday 24 June 2014 10:01:36 Meier, Dennis wrote:
>> > -----Original Message-----
>> > From: otavio.salvador@gmail.com [mailto:otavio.salvador@gmail.com] On
>> > Behalf Of Otavio Salvador
>> > Sent: Dienstag, 17. Juni 2014 16:40
>> > To: Meier, Dennis
>> > Cc: Patches and discussions about the oe-core layer
>> > Subject: Re: [OE-core] [PATCH] populate_sdk_base: add auto-completion
>> > in setup
>> >
>> > On Tue, Jun 17, 2014 at 10:16 AM, Dennis Meier
>> >
>> > <meier.dennis@siemens.com> wrote:
>> > > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
>> >
>> > I do like the idea but how portable is it?
>> >
>> > For example in my envinronment it does not work as I use zsh.
>>
>> Yeah that's what I was afraid of. We figured that the script might still be
>> invoked by bash because it starts with "#!/bin/bash" and thus it might
>> work, but if you tested it and it didn't work for you I think it doesn't
>> make sense to apply this patch.
>
> Well, it depends - Otavio, when you say "it does not work" did you test it?
> #!/bin/bash should ensure bash is actually used.

Having the shebang should be fine. I tried it byhand to check if it
was compatible.

So I am fine with the patch.
Richard Purdie - June 29, 2014, 8:01 a.m.
On Tue, 2014-06-17 at 17:03 +0300, Laurentiu Palcu wrote:
> On Tue, Jun 17, 2014 at 03:16:53PM +0200, Dennis Meier wrote:
> > Signed-off-by: Dennis Meier <meier.dennis@siemens.com>
> > ---
> >  meta/classes/populate_sdk_base.bbclass |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
> > index 35d837d..1182425 100644
> > --- a/meta/classes/populate_sdk_base.bbclass
> > +++ b/meta/classes/populate_sdk_base.bbclass
> > @@ -173,9 +173,8 @@ if [ $verbose = 1 ] ; then
> >  	set -x
> >  fi
> >  
> > -printf "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): "
> >  if [ "$target_sdk_dir" = "" ]; then
> > -	read target_sdk_dir
> > +	read -e -p "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): " target_sdk_dir
> >  	[ "$target_sdk_dir" = "" ] && target_sdk_dir=$DEFAULT_INSTALL_DIR
> >  else
> >  	echo "$target_sdk_dir"
> Moving the above printf in the 'if' branch will make this 'echo' look
> strange when the target directory is provided with the -d option... I
> believe you can remove this 'echo' completely though. There is another
> message, later, informing the user where the SDK gets installed.

I've applied a second patch to tweak this, thanks for the review.

Richard

Patch

diff --git a/meta/classes/populate_sdk_base.bbclass b/meta/classes/populate_sdk_base.bbclass
index 35d837d..1182425 100644
--- a/meta/classes/populate_sdk_base.bbclass
+++ b/meta/classes/populate_sdk_base.bbclass
@@ -173,9 +173,8 @@  if [ $verbose = 1 ] ; then
 	set -x
 fi
 
-printf "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): "
 if [ "$target_sdk_dir" = "" ]; then
-	read target_sdk_dir
+	read -e -p "Enter target directory for SDK (default: $DEFAULT_INSTALL_DIR): " target_sdk_dir
 	[ "$target_sdk_dir" = "" ] && target_sdk_dir=$DEFAULT_INSTALL_DIR
 else
 	echo "$target_sdk_dir"