[gentoo-catalyst] [PATCH 1/3] Revert "livecdfs-update.sh: Escape ampersands in STARTX sed expression"

[gentoo-catalyst] [PATCH 1/3] Revert "livecdfs-update.sh: Escape ampersands in STARTX sed expression"

[Matt Turner]

From: Matt Turner <mattst88@gmail.com>

This reverts commit ed4d1623c3dac22ed008f518b79deecde1f33c26.

The commit was a v1 commit that was supposed to be replaced.
---
 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 b34e917..f8eb425 100644
--- a/targets/support/livecdfs-update.sh
+++ b/targets/support/livecdfs-update.sh
@@ -378,7 +378,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.3.2

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

[Matt Turner]

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

Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=481236
Reviewed-by: Matt Turner <mattst88@gmail.com>
---
 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..18b8f1d 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 -a $(tty) = "/dev/tty1" ];
 		then
 			rm -f /etc/startx
 			##STARTX
-- 
1.8.3.2

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

[W. Trevor King]

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

On Fri, Oct 11, 2013 at 03:09:41PM -0700, Matt Turner wrote:
> From: "W. Trevor King" <wking@tremily.us>
> 
> Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=481236
> Reviewed-by: Matt Turner <mattst88@gmail.com>

Looks like you dropped my commit message [1].

[1]: http://git.tremily.us/?p=catalyst.git;a=commit;h=1fdbc0e92ed3309eeadb43c3768ec3d1e74fbf25

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

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

[Matt Turner]

Sorry, will replace the commit message.

Reviewed-by on the first patch?

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

[W. Trevor King]

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

On Fri, Oct 11, 2013 at 03:43:15PM -0700, Matt Turner wrote:
> Sorry, will replace the commit message.
> 
> Reviewed-by on the first patch?

Yup, ever since [1] ;).  In fact, I vaguely remember arguing for just
reverting that patch out of existence on #gentoo-releng…

Cheers,
Trevor

[1]: http://thread.gmane.org/gmane.linux.gentoo.catalyst/2187/focus=2204

-- 
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] Re: [PATCH 2/3] livecd-bashrc: Avoid a startx race by restricting to tty1

[W. Trevor King]

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

On Fri, Oct 11, 2013 at 03:47:17PM -0700, W. Trevor King wrote:
> Yup, ever since [1] ;).  In fact, I vaguely remember arguing for just
> reverting that patch out of existence on #gentoo-releng…

s/reverting/rebasing/

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

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

[Matt Turner]

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.

Reviewed-by: Matt Turner <mattst88@gmail.com>
---
 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 f8eb425..2b41f9d 100644
--- a/targets/support/livecdfs-update.sh
+++ b/targets/support/livecdfs-update.sh
@@ -377,9 +377,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.3.2