Merge lp:~stgraber/upstart/upstart-shell-paths into lp:upstart

[Stéphane Graber]

We recently noticed a bunch of upstart build failures caused by
test_job_process reporting that pWD=/ was added to the environment which
wasn't quite expected.

This was eventually tracked down to upstart calling the command through a
shell if any of the usual shell special characters are found (like ~ in this
case).

This branch does two things:
 - Changes job_process.c slightly to explicitly set ->script = TRUE when
   upstart decides that the command needs to be run under a shell.
 - Updates the tests to check for the post-run value of ->script and allow
   for PWD=/ in such case.

PWD will always be / if present as the shell itself is provided without any
environment and so can't inherit PWD.

[lp:~stgraber/upstart/upstart-shell-paths]

1409. By Stéphane Graber on 2012-12-17

Cleanup

[James Hunt (jamesodhunt)]

This is bug 1086474.

LGTM. Merged.

review: Approve

=== modified file 'init/job_process.c'
--- init/job_process.c	2012-12-03 20:27:09 +0000
+++ init/job_process.c	2012-12-17 11:39:20 +0000
@@ -178,11 +178,18 @@
 	nih_assert (proc != NULL);
 	nih_assert (proc->command != NULL);
 
+
+	/* If the command string contains any shell-like characters,
+	 * automatically set script = TRUE since that's the best way to deal
+	 * with things like variables.
+	 */
+	if (strpbrk (proc->command, SHELL_CHARS))
+		proc->script = TRUE;
+
 	/* We run the process using a shell if it says it wants to be run
-	 * as such, or if it contains any shell-like characters; since that's
-	 * the best way to deal with things like variables.
+	 * as such.
 	 */
-	if ((proc->script) || strpbrk (proc->command, SHELL_CHARS)) {
+	if (proc->script) {
 		char *nl, *p;
 
 		argc = 0;

=== modified file 'init/tests/test_job_process.c'
--- init/tests/test_job_process.c	2012-12-07 21:52:38 +0000
+++ init/tests/test_job_process.c	2012-12-17 11:39:20 +0000
@@ -680,6 +680,8 @@
 		output = fopen (filename, "r");
 		TEST_FILE_EQ (output, "BAR=BAZ\n");
 		TEST_FILE_EQ (output, "FOO=BAR\n");
+		if (job->class->process[PROCESS_MAIN]->script)
+			TEST_FILE_EQ (output, "PWD=/\n");
 		TEST_FILE_EQ (output, "UPSTART_INSTANCE=\n");
 		TEST_FILE_EQ (output, "UPSTART_JOB=test\n");
 		TEST_FILE_EQ (output, "UPSTART_NO_SESSIONS=1\n");
@@ -734,6 +736,8 @@
 		output = fopen (filename, "r");
 		TEST_FILE_EQ (output, "BAR=BAZ\n");
 		TEST_FILE_EQ (output, "FOO=BAR\n");
+		if (job->class->process[PROCESS_MAIN]->script)
+			TEST_FILE_EQ (output, "PWD=/\n");
 		TEST_FILE_EQ (output, "UPSTART_INSTANCE=foo\n");
 		TEST_FILE_EQ (output, "UPSTART_JOB=test\n");
 		TEST_FILE_EQ (output, "UPSTART_NO_SESSIONS=1\n");
@@ -790,6 +794,8 @@
 		TEST_FILE_EQ (output, "BAR=BAZ\n");
 		TEST_FILE_EQ (output, "CRACKLE=FIZZ\n");
 		TEST_FILE_EQ (output, "FOO=SMACK\n");
+		if (job->class->process[PROCESS_PRE_STOP]->script)
+			TEST_FILE_EQ (output, "PWD=/\n");
 		TEST_FILE_EQ (output, "UPSTART_INSTANCE=\n");
 		TEST_FILE_EQ (output, "UPSTART_JOB=test\n");
 		TEST_FILE_EQ (output, "UPSTART_NO_SESSIONS=1\n");
@@ -846,6 +852,8 @@
 		TEST_FILE_EQ (output, "BAR=BAZ\n");
 		TEST_FILE_EQ (output, "CRACKLE=FIZZ\n");
 		TEST_FILE_EQ (output, "FOO=SMACK\n");
+		if (job->class->process[PROCESS_POST_STOP]->script)
+			TEST_FILE_EQ (output, "PWD=/\n");
 		TEST_FILE_EQ (output, "UPSTART_INSTANCE=\n");
 		TEST_FILE_EQ (output, "UPSTART_JOB=test\n");
 		TEST_FILE_EQ (output, "UPSTART_NO_SESSIONS=1\n");