All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 1/3] base-files: profile: Whitespace clean up
@ 2017-04-05 13:46 Peter Kjellerstedt
  2017-04-05 13:46 ` [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists Peter Kjellerstedt
  2017-04-05 13:46 ` [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally Peter Kjellerstedt
  0 siblings, 2 replies; 6+ messages in thread
From: Peter Kjellerstedt @ 2017-04-05 13:46 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/recipes-core/base-files/base-files/profile | 29 +++++++++++++------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index c616616cee..22dfb4f219 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -6,28 +6,29 @@ EDITOR="vi"			# needed for packages like cron, git-commit
 test -z "$TERM" && TERM="vt100"	# Basic terminal capab. For screen etc.
 
 if [ "$HOME" = "ROOTHOME" ]; then
-   PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
+	PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
 fi
 if [ "$PS1" ]; then
-# works for bash and ash (no other shells known to be in use here)
-   PS1='\u@\h:\w\$ '
+	# works for bash and ash (no other shells known to be in use here)
+	PS1='\u@\h:\w\$ '
 fi
 
 if [ -d /etc/profile.d ]; then
-  for i in /etc/profile.d/*.sh ; do
-    if [ -f $i -a -r $i ]; then
-      . $i
-    fi
-  done
-  unset i
+	for i in /etc/profile.d/*.sh; do
+		if [ -f $i -a -r $i ]; then
+			. $i
+		fi
+	done
+	unset i
 fi
 
 if [ -x /usr/bin/resize ] && termpath="`tty`"; then
-  # Make sure we are on a serial console (i.e. the device used starts with /dev/tty),
-  # otherwise we confuse e.g. the eclipse launcher which tries do use ssh
-  case "$termpath" in
-  /dev/tty[A-z]*) resize >/dev/null
-  esac
+	# Make sure we are on a serial console (i.e. the device used starts with
+	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
+	# tries do use ssh
+	case "$termpath" in
+		/dev/tty[A-z]*) resize >/dev/null
+	esac
 fi
 
 export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
-- 
2.12.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists
  2017-04-05 13:46 [PATCHv2 1/3] base-files: profile: Whitespace clean up Peter Kjellerstedt
@ 2017-04-05 13:46 ` Peter Kjellerstedt
  2017-04-11 14:06   ` Richard Purdie
  2017-04-05 13:46 ` [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally Peter Kjellerstedt
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Kjellerstedt @ 2017-04-05 13:46 UTC (permalink / raw)
  To: openembedded-core

This avoids the following error when logging in to a host that does
not have the tty command:

-sh: tty: not found

Reported-by: Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn@axis.com>
Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/recipes-core/base-files/base-files/profile   | 6 +++---
 meta/recipes-core/base-files/base-files_3.0.14.bb | 4 ----
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index 22dfb4f219..ceaf15f799 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -22,12 +22,12 @@ if [ -d /etc/profile.d ]; then
 	unset i
 fi
 
-if [ -x /usr/bin/resize ] && termpath="`tty`"; then
+if command -v resize >/dev/null && command -v tty >/dev/null; then
 	# Make sure we are on a serial console (i.e. the device used starts with
 	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
 	# tries do use ssh
-	case "$termpath" in
-		/dev/tty[A-z]*) resize >/dev/null
+	case $(tty) in
+		/dev/tty[A-z]*) resize >/dev/null;;
 	esac
 fi
 
diff --git a/meta/recipes-core/base-files/base-files_3.0.14.bb b/meta/recipes-core/base-files/base-files_3.0.14.bb
index f56969c52e..ca7bf06353 100644
--- a/meta/recipes-core/base-files/base-files_3.0.14.bb
+++ b/meta/recipes-core/base-files/base-files_3.0.14.bb
@@ -128,10 +128,6 @@ do_install () {
 	install -m 0644 ${WORKDIR}/host.conf ${D}${sysconfdir}/host.conf
 	install -m 0644 ${WORKDIR}/motd ${D}${sysconfdir}/motd
 
-	if [ "/usr/bin" != "${bindir}" ]; then
-		sed -i "s,/usr/bin/resize,${bindir}/resize," ${D}${sysconfdir}/profile
-	fi
-
 	ln -sf /proc/mounts ${D}${sysconfdir}/mtab
 }
 
-- 
2.12.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally
  2017-04-05 13:46 [PATCHv2 1/3] base-files: profile: Whitespace clean up Peter Kjellerstedt
  2017-04-05 13:46 ` [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists Peter Kjellerstedt
@ 2017-04-05 13:46 ` Peter Kjellerstedt
  2017-04-05 14:44   ` Richard Purdie
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Kjellerstedt @ 2017-04-05 13:46 UTC (permalink / raw)
  To: openembedded-core

It is preferred to use `[ <condition> ] || ...` instead of
`[ <negated condition> ] && ...` as the latter leaves $? set to 1.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index ceaf15f799..a062028226 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -3,15 +3,13 @@
 
 PATH="/usr/local/bin:/usr/bin:/bin"
 EDITOR="vi"			# needed for packages like cron, git-commit
-test -z "$TERM" && TERM="vt100"	# Basic terminal capab. For screen etc.
+[ "$TERM" ] || TERM="vt100"	# Basic terminal capab. For screen etc.
 
-if [ "$HOME" = "ROOTHOME" ]; then
-	PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
-fi
-if [ "$PS1" ]; then
-	# works for bash and ash (no other shells known to be in use here)
-	PS1='\u@\h:\w\$ '
-fi
+# Add /sbin & co to $PATH for the root user
+[ "$HOME" != "ROOTHOME" ] || PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
+
+# Set the prompt for bash and ash (no other shells known to be in use here)
+[ -z "$PS1" ] || PS1='\u@\h:\w\$ '
 
 if [ -d /etc/profile.d ]; then
 	for i in /etc/profile.d/*.sh; do
-- 
2.12.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally
  2017-04-05 13:46 ` [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally Peter Kjellerstedt
@ 2017-04-05 14:44   ` Richard Purdie
  2017-04-06  7:21     ` Peter Kjellerstedt
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Purdie @ 2017-04-05 14:44 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Wed, 2017-04-05 at 15:46 +0200, Peter Kjellerstedt wrote:
> It is preferred to use `[ <condition> ] || ...` instead of
> `[ <negated condition> ] && ...` as the latter leaves $? set to 1.

Your patch and the description above don't match. There are changes
here which aren't <negated condition> and are stylistic changes instead
afaict. We're past feature freeze so whilst I'm interested in genuine
bugs, I do not really want code churn which just introduces risk.

Cheers,

Richard

> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/recipes-core/base-files/base-files/profile
> b/meta/recipes-core/base-files/base-files/profile
> index ceaf15f799..a062028226 100644
> --- a/meta/recipes-core/base-files/base-files/profile
> +++ b/meta/recipes-core/base-files/base-files/profile
> @@ -3,15 +3,13 @@
>  
>  PATH="/usr/local/bin:/usr/bin:/bin"
>  EDITOR="vi"			# needed for packages like cron,
> git-commit
> -test -z "$TERM" && TERM="vt100"	# Basic terminal capab. For
> screen etc.
> +[ "$TERM" ] || TERM="vt100"	# Basic terminal capab. For
> screen etc.
>  
> -if [ "$HOME" = "ROOTHOME" ]; then
> -	PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
> -fi
> -if [ "$PS1" ]; then
> -	# works for bash and ash (no other shells known to be in use
> here)
> -	PS1='\u@\h:\w\$ '
> -fi
> +# Add /sbin & co to $PATH for the root user
> +[ "$HOME" != "ROOTHOME" ] ||
> PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
> +
> +# Set the prompt for bash and ash (no other shells known to be in
> use here)
> +[ -z "$PS1" ] || PS1='\u@\h:\w\$ '
>  
>  if [ -d /etc/profile.d ]; then
>  	for i in /etc/profile.d/*.sh; do
> -- 
> 2.12.0
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally
  2017-04-05 14:44   ` Richard Purdie
@ 2017-04-06  7:21     ` Peter Kjellerstedt
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Kjellerstedt @ 2017-04-06  7:21 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

> -----Original Message-----
> From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
> Sent: den 5 april 2017 16:44
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCHv2 3/3] base-files: profile: Simplify
> setting variables conditionally
> 
> On Wed, 2017-04-05 at 15:46 +0200, Peter Kjellerstedt wrote:
> > It is preferred to use `[ <condition> ] || ...` instead of
> > `[ <negated condition> ] && ...` as the latter leaves $? set to 1.
> 
> Your patch and the description above don't match. There are changes
> here which aren't <negated condition> and are stylistic changes instead

Well, the intended meaning was that whatever <condition> was used in the  
first example should be negated in the second one, not to mean an explicit 
negate operator should be used, e.g., if the first <condition> is 
[ -z "$VAR" ], the second one would be [ "$VAR" ], not [ ! -z "$VAR" ].

Maybe my examples are just confusing things and I should just remove
them?

> afaict. We're past feature freeze so whilst I'm interested in genuine
> bugs, I do not really want code churn which just introduces risk.

As long as you take the second patch (which is a genuine bug), I am fine 
(the first one is pure whitespace clean up, so it should be safe to take 
as well).

> Cheers,
> 
> Richard
> 
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> >  meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/meta/recipes-core/base-files/base-files/profile
> > b/meta/recipes-core/base-files/base-files/profile
> > index ceaf15f799..a062028226 100644
> > --- a/meta/recipes-core/base-files/base-files/profile
> > +++ b/meta/recipes-core/base-files/base-files/profile
> > @@ -3,15 +3,13 @@
> >
> >  PATH="/usr/local/bin:/usr/bin:/bin"
> >  EDITOR="vi"			# needed for packages like
> cron,
> > git-commit
> > -test -z "$TERM" && TERM="vt100"	# Basic terminal capab. For
> > screen etc.
> > +[ "$TERM" ] || TERM="vt100"	# Basic terminal capab. For
> > screen etc.
> >
> > -if [ "$HOME" = "ROOTHOME" ]; then
> > -	PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
> > -fi
> > -if [ "$PS1" ]; then
> > -	# works for bash and ash (no other shells known to be in use
> > here)
> > -	PS1='\u@\h:\w\$ '
> > -fi
> > +# Add /sbin & co to $PATH for the root user
> > +[ "$HOME" != "ROOTHOME" ] ||
> > PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
> > +
> > +# Set the prompt for bash and ash (no other shells known to be in
> > use here)
> > +[ -z "$PS1" ] || PS1='\u@\h:\w\$ '
> >
> >  if [ -d /etc/profile.d ]; then
> >  	for i in /etc/profile.d/*.sh; do
> > --
> > 2.12.0

//Peter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists
  2017-04-05 13:46 ` [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists Peter Kjellerstedt
@ 2017-04-11 14:06   ` Richard Purdie
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Purdie @ 2017-04-11 14:06 UTC (permalink / raw)
  To: Peter Kjellerstedt, openembedded-core

On Wed, 2017-04-05 at 15:46 +0200, Peter Kjellerstedt wrote:
> This avoids the following error when logging in to a host that does
> not have the tty command:
> 
> -sh: tty: not found
> 
> Reported-by: Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn@axis.
> com>
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  meta/recipes-core/base-files/base-files/profile   | 6 +++---
>  meta/recipes-core/base-files/base-files_3.0.14.bb | 4 ----
>  2 files changed, 3 insertions(+), 7 deletions(-)

So instead we now break systems which don't have "command" which turns
out to be all our default busybox configuration :(.

Thanks for spotting this Nathan...

Cheers,

Richard


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-11 14:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05 13:46 [PATCHv2 1/3] base-files: profile: Whitespace clean up Peter Kjellerstedt
2017-04-05 13:46 ` [PATCHv2 2/3] base-files: profile: Do not assume that the tty command exists Peter Kjellerstedt
2017-04-11 14:06   ` Richard Purdie
2017-04-05 13:46 ` [PATCHv2 3/3] base-files: profile: Simplify setting variables conditionally Peter Kjellerstedt
2017-04-05 14:44   ` Richard Purdie
2017-04-06  7:21     ` Peter Kjellerstedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.