[gentoo-catalyst] [PATCH] livecdfs-update.sh: Escape ampersands in STARTX sed expression

[gentoo-catalyst] [PATCH] livecdfs-update.sh: Escape ampersands in STARTX sed expression

[W. Trevor King]

From: "W. Trevor King" <wking@tremily.us>

From sed(1):

  s/regexp/replacement/
    Attempt to match regexp against the pattern space.  If successful,
    replace that portion matched with replacement.  The replacement
    may contain the special character & to refer to that portion of
    the pattern space which matched, and the special escapes \1
    through \9 to refer to the corresponding matching sub-expressions
    in the regexp.

This means that the old expression (with unescaped ampersands) lead
to:

  source /etc/profile ##STARTX##STARTX su - ${first_user} -c startx

when we want:

  source /etc/profile && su - ${first_user} -c startx

with ${first_user} expanded in both cases.
---
 targets/support/livecdfs-update.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/targets/support/livecdfs-update.sh b/targets/support/livecdfs-update.sh
index 77d694e..fda3e36 100644
--- a/targets/support/livecdfs-update.sh
+++ b/targets/support/livecdfs-update.sh
@@ -389,7 +389,7 @@ esac
 if [ -e /etc/startx ]
 then
 	sed -i \
-		"s:##STARTX:source /etc/profile && su - ${first_user} -c startx:" \
+		"s:##STARTX:source /etc/profile \&\& su - ${first_user} -c startx:" \
 		/root/.bashrc
 fi
 
-- 
1.8.2.rc0.16.g20a599e

[gentoo-catalyst] [PATCH] livecd-bashrc: Avoid a startx race by restricting to tty1

[W. Trevor King]

From: "W. Trevor King" <wking@tremily.us>

Otherwise several virtual consoles may notice the existence of
/etc/startx, and spawn simultaneous X servers.  This way we only spawn
a single X server, regardless of timing.

A better solution here is probably to add a "start" or "x-server"
service to /etc/init.d/, but that's more work than I'm up to at the
moment.
---
 livecd/files/livecd-bashrc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/livecd/files/livecd-bashrc b/livecd/files/livecd-bashrc
index 983e657..7abfbd4 100644
--- a/livecd/files/livecd-bashrc
+++ b/livecd/files/livecd-bashrc
@@ -4,7 +4,7 @@ if [ ! "$(grep nox /proc/cmdline)" ]
 then
 	if [ -x /usr/bin/X ]
 	then
-		if [ -e /etc/startx ]
+		if [ -e /etc/startx ] && [ "$(tty)" == /dev/tty1 ]
 		then
 			rm -f /etc/startx
 			##STARTX
-- 
1.8.2.rc0.16.g20a599e

[gentoo-catalyst] [PATCH v2 0/2] Fix livecdfs-update.sh startx handling

[W. Trevor King]

From: "W. Trevor King" <wking@tremily.us>

Changes since v1:

* Even with escaped ampersands, the old startx expression didn't work
  very well with a Bash shell.  The XSESSION environment variable is
  set via /etc/profile which Bash only sources if it thinks its in a
  login shell.  This version reworks the startx su call so Bash *does*
  source the profile.

W. Trevor King (2):
  livecd-bashrc: Avoid a startx race by restricting to tty1
  livecdfs-update.sh: Use `bash --login` to spawn startx

 livecd/files/livecd-bashrc         | 2 +-
 targets/support/livecdfs-update.sh | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

-- 
1.8.2.rc0.16.g20a599e

[gentoo-catalyst] [PATCH v2 1/2] livecd-bashrc: Avoid a startx race by restricting to tty1

[W. Trevor King]

From: "W. Trevor King" <wking@tremily.us>

Otherwise several virtual consoles may notice the existence of
/etc/startx, and spawn simultaneous X servers.  This way we only spawn
a single X server, regardless of timing.

A better solution here is probably to add a "start" or "x-server"
service to /etc/init.d/, but that's more work than I'm up to at the
moment.
---
 livecd/files/livecd-bashrc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/livecd/files/livecd-bashrc b/livecd/files/livecd-bashrc
index 983e657..7abfbd4 100644
--- a/livecd/files/livecd-bashrc
+++ b/livecd/files/livecd-bashrc
@@ -4,7 +4,7 @@ if [ ! "$(grep nox /proc/cmdline)" ]
 then
 	if [ -x /usr/bin/X ]
 	then
-		if [ -e /etc/startx ]
+		if [ -e /etc/startx ] && [ "$(tty)" == /dev/tty1 ]
 		then
 			rm -f /etc/startx
 			##STARTX
-- 
1.8.2.rc0.16.g20a599e

[gentoo-catalyst] [PATCH v2 2/2] livecdfs-update.sh: Use `bash --login` to spawn startx

[W. Trevor King]

From: "W. Trevor King" <wking@tremily.us>

Starting a "login" version of Bash via `su` is tricky.  The naive:

  su - ${first_user} -c startx

fails because `su - ...` clears a number of environment variables (so
the prefixed `source /etc/profile` doesn't accomplish anything), but
Bash isn't started with the `--login` option, so it doesn't source
/etc/profile internally.  From bash(1):

  A login shell is one whose first character of argument zero is a -,
  or one started with the --login option.
  ...
  An interactive shell is one started without non-option arguments and
  without the -c option whose standard input and error are both
  connected to terminals (as determined by isatty(3)), or one started
  with the -i option...
  ...
  When bash is invoked as an interactive login shell, or as a
  non-interactive shell with the --login option, it first reads and
  executes commands from the file /etc/profile, if that file exists.
  After reading that file, it looks for ~/.bash_profile,
  ~/.bash_login, and ~/.profile, in that order, and reads and executes
  commands from the first one that exists and is readable.  The
  --noprofile option may be used when the shell is started to inhibit
  this behavior.

In order to get the login-style profile loading with a non-interactive
`su` invocation, you need to use something like:

  echo "${command}" | su - "${user}"

This starts a login shell and pipes the command in via stdin, which
seems to fake Bash into thinking its running from an interactive
terminal.  Not the most elegant, but the other implementations I can
think of are even worse:

  su - "${user}" -c "bash --login -c ${command}"
  su - "${user}" -c 'source /etc/profile &&
      (source .bash_profile || ...) && ${command}"

The old expression was broken anyway due to unescaped ampersands in
the sed expression.  From sed(1):

  s/regexp/replacement/
    Attempt to match regexp against the pattern space.  If successful,
    replace that portion matched with replacement.  The replacement
    may contain the special character & to refer to that portion of
    the pattern space which matched, and the special escapes \1
    through \9 to refer to the corresponding matching sub-expressions
    in the regexp.

This means that the old expression (with unescaped ampersands) lead
to:

  source /etc/profile ##STARTX##STARTX su - ${first_user} -c startx

with ${first_user} expanded.  This commented out startx, so it was
never run.
---
 targets/support/livecdfs-update.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/targets/support/livecdfs-update.sh b/targets/support/livecdfs-update.sh
index 77d694e..0ac41dd 100644
--- a/targets/support/livecdfs-update.sh
+++ b/targets/support/livecdfs-update.sh
@@ -388,9 +388,7 @@ esac
 # We want the first user to be used when auto-starting X
 if [ -e /etc/startx ]
 then
-	sed -i \
-		"s:##STARTX:source /etc/profile && su - ${first_user} -c startx:" \
-		/root/.bashrc
+	sed -i "s:##STARTX:echo startx | su - '${first_user}':" /root/.bashrc
 fi
 
 if [ -e /lib/rcscripts/addons/udev-start.sh ]
-- 
1.8.2.rc0.16.g20a599e

Re: [gentoo-catalyst] [PATCH v2 2/2] livecdfs-update.sh: Use `bash --login` to spawn startx

[Matt Turner]

On Wed, Mar 6, 2013 at 9:02 AM, W. Trevor King <wking@tremily.us> wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> Starting a "login" version of Bash via `su` is tricky.  The naive:
>
>   su - ${first_user} -c startx
>
> fails because `su - ...` clears a number of environment variables (so
> the prefixed `source /etc/profile` doesn't accomplish anything), but
> Bash isn't started with the `--login` option, so it doesn't source
> /etc/profile internally.  From bash(1):
>
>   A login shell is one whose first character of argument zero is a -,
>   or one started with the --login option.
>   ...
>   An interactive shell is one started without non-option arguments and
>   without the -c option whose standard input and error are both
>   connected to terminals (as determined by isatty(3)), or one started
>   with the -i option...
>   ...
>   When bash is invoked as an interactive login shell, or as a
>   non-interactive shell with the --login option, it first reads and
>   executes commands from the file /etc/profile, if that file exists.
>   After reading that file, it looks for ~/.bash_profile,
>   ~/.bash_login, and ~/.profile, in that order, and reads and executes
>   commands from the first one that exists and is readable.  The
>   --noprofile option may be used when the shell is started to inhibit
>   this behavior.
>
> In order to get the login-style profile loading with a non-interactive
> `su` invocation, you need to use something like:
>
>   echo "${command}" | su - "${user}"
>
> This starts a login shell and pipes the command in via stdin, which
> seems to fake Bash into thinking its running from an interactive
> terminal.  Not the most elegant, but the other implementations I can
> think of are even worse:
>
>   su - "${user}" -c "bash --login -c ${command}"
>   su - "${user}" -c 'source /etc/profile &&
>       (source .bash_profile || ...) && ${command}"
>
> The old expression was broken anyway due to unescaped ampersands in
> the sed expression.  From sed(1):
>
>   s/regexp/replacement/
>     Attempt to match regexp against the pattern space.  If successful,
>     replace that portion matched with replacement.  The replacement
>     may contain the special character & to refer to that portion of
>     the pattern space which matched, and the special escapes \1
>     through \9 to refer to the corresponding matching sub-expressions
>     in the regexp.
>
> This means that the old expression (with unescaped ampersands) lead
> to:
>
>   source /etc/profile ##STARTX##STARTX su - ${first_user} -c startx
>
> with ${first_user} expanded.  This commented out startx, so it was
> never run.
> ---
>  targets/support/livecdfs-update.sh | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/targets/support/livecdfs-update.sh b/targets/support/livecdfs-update.sh
> index 77d694e..0ac41dd 100644
> --- a/targets/support/livecdfs-update.sh
> +++ b/targets/support/livecdfs-update.sh
> @@ -388,9 +388,7 @@ esac
>  # We want the first user to be used when auto-starting X
>  if [ -e /etc/startx ]
>  then
> -       sed -i \
> -               "s:##STARTX:source /etc/profile && su - ${first_user} -c startx:" \
> -               /root/.bashrc
> +       sed -i "s:##STARTX:echo startx | su - '${first_user}':" /root/.bashrc
>  fi
>
>  if [ -e /lib/rcscripts/addons/udev-start.sh ]
> --
> 1.8.2.rc0.16.g20a599e
>
>

This doesn't apply after PATCH 1/2 in this series. Probably why the
first PATCH wasn't labeled as 1/2. Want to confirm what you want to do
here?

[gentoo-catalyst] Re: [PATCH v2 2/2] livecdfs-update.sh: Use `bash --login` to spawn startx

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On Fri, Mar 08, 2013 at 06:55:07PM -0800, Matt Turner wrote:
> This doesn't apply after PATCH 1/2 in this series. Probably why the
> first PATCH wasn't labeled as 1/2. Want to confirm what you want to do
> here?

With the "v2" series, I intended the initial v1 stuff to be thrown out
and v2 applied instead.  So I'd revert the \&\& fix and just go with
the v2 stuff.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] [PATCH] livecd-bashrc: Avoid a startx race by restricting to tty1

[Matt Turner]

On Sun, Mar 3, 2013 at 7:13 PM, W. Trevor King <wking@tremily.us> wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> Otherwise several virtual consoles may notice the existence of
> /etc/startx, and spawn simultaneous X servers.  This way we only spawn
> a single X server, regardless of timing.
>
> A better solution here is probably to add a "start" or "x-server"
> service to /etc/init.d/, but that's more work than I'm up to at the
> moment.
> ---
>  livecd/files/livecd-bashrc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/livecd/files/livecd-bashrc b/livecd/files/livecd-bashrc
> index 983e657..7abfbd4 100644
> --- a/livecd/files/livecd-bashrc
> +++ b/livecd/files/livecd-bashrc
> @@ -4,7 +4,7 @@ if [ ! "$(grep nox /proc/cmdline)" ]
>  then
>         if [ -x /usr/bin/X ]
>         then
> -               if [ -e /etc/startx ]
> +               if [ -e /etc/startx ] && [ "$(tty)" == /dev/tty1 ]
>                 then
>                         rm -f /etc/startx
>                         ##STARTX
> --
> 1.8.2.rc0.16.g20a599e
>
>

Seems reasonable, but I don't really know how any of this works. Have
you experienced the problem you describe? I.e., multiple X servers
starting.

[gentoo-catalyst] Re: [PATCH] livecd-bashrc: Avoid a startx race by restricting to tty1

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Fri, Mar 08, 2013 at 06:49:42PM -0800, Matt Turner wrote:
> > -               if [ -e /etc/startx ]
> > +               if [ -e /etc/startx ] && [ "$(tty)" == /dev/tty1 ]
> 
> Seems reasonable, but I don't really know how any of this works. Have
> you experienced the problem you describe? I.e., multiple X servers
> starting.

I had one X server starting, but also crashed X servers (do to
.Xauthority conflicts or something, I didn't write down the message)
in other ttys.  Also, if another tty starts the X server, it's not
clear to me that the user will actually end up in X.  The user may end
up on tty1 and have to Alt-F7 into X, which is not intuitive ;).

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] [PATCH] livecdfs-update.sh: Escape ampersands in STARTX sed expression

[Matt Turner]

On Sun, Mar 3, 2013 at 8:53 AM, W. Trevor King <wking@tremily.us> wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> From sed(1):
>
>   s/regexp/replacement/
>     Attempt to match regexp against the pattern space.  If successful,
>     replace that portion matched with replacement.  The replacement
>     may contain the special character & to refer to that portion of
>     the pattern space which matched, and the special escapes \1
>     through \9 to refer to the corresponding matching sub-expressions
>     in the regexp.
>
> This means that the old expression (with unescaped ampersands) lead
> to:
>
>   source /etc/profile ##STARTX##STARTX su - ${first_user} -c startx
>
> when we want:
>
>   source /etc/profile && su - ${first_user} -c startx
>
> with ${first_user} expanded in both cases.
> ---
>  targets/support/livecdfs-update.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/targets/support/livecdfs-update.sh b/targets/support/livecdfs-update.sh
> index 77d694e..fda3e36 100644
> --- a/targets/support/livecdfs-update.sh
> +++ b/targets/support/livecdfs-update.sh
> @@ -389,7 +389,7 @@ esac
>  if [ -e /etc/startx ]
>  then
>         sed -i \
> -               "s:##STARTX:source /etc/profile && su - ${first_user} -c startx:" \
> +               "s:##STARTX:source /etc/profile \&\& su - ${first_user} -c startx:" \
>                 /root/.bashrc
>  fi
>
> --
> 1.8.2.rc0.16.g20a599e
>
>

Nice. Bug since 2006. I've committed this. Thanks!

[gentoo-catalyst] Re: [PATCH] livecdfs-update.sh: Escape ampersands in STARTX sed expression

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Fri, Mar 08, 2013 at 06:47:37PM -0800, Matt Turner wrote:
> Nice. Bug since 2006. I've committed this. Thanks!

Oops, this patch was deprecated by v2 ;).

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]