Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

> -----Original Message-----
> From: Martin Jansa [mailto:martin.jansa@gmail.com]
> Sent: den 18 maj 2018 12:29
> To: ChenQi <Qi.Chen@windriver.com>
> Cc: Mike Looijmans <mike.looijmans@topic.nl>; openembedded-
> core@lists.openembedded.org; Peter Kjellerstedt
> <peter.kjellerstedt@axis.com>; Burton, Ross <ross.burton@intel.com>
> Subject: Re: [OE-core] [PATCH 1/2] base-files: profile: Do not assume
> that the 'command' command exists
> 
> On Fri, Oct 13, 2017 at 06:07:16PM +0800, ChenQi wrote:
> > On 10/13/2017 05:53 PM, ChenQi wrote:
> > > On 09/18/2017 09:56 PM, Mike Looijmans wrote:
> > >> On 18-09-17 15:24, Mike Looijmans wrote:
> > >>> On 18-09-17 15:08, Burton, Ross wrote:
> > >>>> On 18 September 2017 at 12:31, Mike Looijmans
> > >>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>>
> wrote:
> > >>>>
> > >>>>         This is basically the same change as I first sent a
> patch
> > >>>> for in
> > >>>>         April, and
> > >>>>         last pinged this Friday... The only real difference is
> that
> > >>>> this one
> > >>>>         misses
> > >>>>         passing error output from resize to /dev/null (which it
> > >>>> should do to
> > >>>>         handle
> > >>>>         the case where tty exists, but resize does not).
> > >>>>
> > >>>>
> > >>>>     Yeah, indeed.
> > >>>>
> > >>>>
> > >>>> Apologies for missing that patch!
> > >>>>
> > >>>>     Other problem is that "resize" outputs shell script on
> stdout
> > >>>> to be
> > >>>>     executed, so the proper "total" invokation would be:
> > >>>>
> > >>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
> > >>>>
> > >>>>     The "eval" part is missing in your version...
> > >>>>
> > >>>>
> > >>>> Who is going to submit the One True patch with all the fixes in?
> I
> > >>>> promise to merge it.
> > >>>
> > >>> I'll send the one ring, eh, patch, in a few minutes. I'll merge
> the
> > >>> two into a single as well.
> > >>
> > >> On second thought, just use Peter's patch "as is".
> > >>
> > >> I've been experimenting with the "eval" part and it doesn't behave
> > >> well. Tends to confuse minicom, create garbage, and in particular
> > >> when run from "profile", it seems to result in counterproductive
> > >> COLUMNS=0 and LINES=0.
> > >>
> > >> I'm actually wondering why the call to "resize" is being done at
> all.
> > >> Just calling "resize" has no effect, since it outputs the results
> on
> > >> stdout as shell script, and that is being discarded. Looking at
> the
> > >> commit that introduced it:
> > >>
> > >
> > > Looking at the codes in busybox (console-tools/resize.c), I can see
> > > that tcsetattr is actually called.
> > > The output is due to:
> > >
> > >     if (ENABLE_FEATURE_RESIZE_PRINT)
> > >         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
> > >             w.ws_col, w.ws_row);
> > >
> > > I can confirm that calling 'resize' is needed. Otherwise, you might
> > > get input wrapping on the same line when typing long commands.
> > >
> > > I just tried busybox init based system and I also met '-sh:
> command:
> > > not found' error message.
> >
> > Sorry for the noise. Just noticed that oe-core has enabled 'command'
> for
> > busybox by default. It's my own busybox defconfig that hasn't enabled
> it.
> 
> I was just hit by the same. resize and tty work fine, but command
> applet is missing in my images.
> 
> Is enabling command applet in busybox now hard requirement for all the
> builds or is anyone still planing to send follow-up on this?
> 
> At least to show some more meaningful error when it's not available?
> 
> "you probably need to fix your busybox defconfig, because some old
> thread on oe-core ML says so"
> 
> Cheers,

Wow, here's an old discussion I had forgotten all about. I actually 
do have an updated version of the patch that solves the problem by 
running resize twice, once with stderr piped to /dev/null to 
determine if the resize command exists, and once without stderr 
piped to /dev/null to actually do the resize (since resize uses 
stderr to determine the size of the tty).

I'll send the updated patch in a minute.

> > Best Regards,
> > Chen Qi
> >
> > > I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile:
> > > Avoid using "command" to determine if programs exist) is correct.
> > >
> > > Best Regards,
> > > Chen Qi
> > >
> > >> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
> > >>
> > >> that already has no effect whatsoever. See the man page for
> resize:
> > >> https://linux.die.net/man/1/resize
> > >>
> > >> I also would consider running some program's output as shell
> script a
> > >> bit spooky, it looks like a security hole waiting to be exploited.
> > >>
> > >>
> > >>
> > >> Kind regards,
> > >>
> > >> Mike Looijmans
> > >> System Expert
> > >>
> > >> TOPIC Products
> > >> Materiaalweg 4, NL-5681 RJ Best
> > >> Postbus 440, NL-5680 AK Best
> > >> Telefoon: +31 (0) 499 33 69 79
> > >> E-mail: mike.looijmans@topicproducts.com
> > >> Website: www.topicproducts.com
> > >>
> > >> Please consider the environment before printing this e-mail
> 
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

//Peter