Merge authors: James Hunt (jamesodhunt) Related merge proposals: https://code.launchpad.net/~jamesodhunt/upstart/bug-1159895/+merge/166836 proposed by: James Hunt (jamesodhunt) review: Approve - Stéphane Graber (stgraber) review: Resubmit - James Hunt (jamesodhunt) ------------------------------------------------------------ revno: 1480 [merge] fixes bug: https://launchpad.net/bugs/1159895 committer: James Hunt <[email protected]> branch nick: upstart timestamp: Thu 2013-06-20 10:50:07 +0100 message: * Merged lp:~jamesodhunt/upstart/bug-1159895. modified: ChangeLog init/job_class.c init/job_process.c util/man/initctl.8 util/tests/test_initctl.c
-- lp:upstart https://code.launchpad.net/~upstart-devel/upstart/trunk Your team Upstart Reviewers is subscribed to branch lp:upstart. To unsubscribe from this branch go to https://code.launchpad.net/~upstart-devel/upstart/trunk/+edit-subscription
=== modified file 'ChangeLog' --- ChangeLog 2013-06-05 16:18:42 +0000 +++ ChangeLog 2013-06-20 09:34:34 +0000 @@ -33,8 +33,29 @@ approach of detecting the format of the JSON is safer. * init/state.h: Remove STATE_VERSION. +2013-06-03 James Hunt <[email protected]> + + * util/tests/test_initctl.c: + - test_no_inherit_job_env(): New function to test --no-inherit-env. + - test_job_env(): + - Move code that sets HOME+PATH if not set from + test_default_job_env() so that test_no_inherit_job_env() can make + use of it. + - Session file cleanup tweaks to work with test_no_inherit_job_env(). + 2013-05-31 James Hunt <[email protected]> + * init/job_class.c: job_class_environment_init(): Copy inits environment + to the default job class environment for user mode where appropriate. + (LP: #1159895). + * init/job_process.c: job_process_run(): Don't copy inits environment + into the job instances environment table as those values are now + already in the table. + * util/tests/test_initctl.c: Updates for new behaviour (where + 'list-env' will now contain the entire environment of the init + process, not just those variables explicitly set via set-env). + * util/man/initctl.8: Update on behaviour. + [ Eric S. Raymond <[email protected]> ] * init/man/init.5: Fix unliftable markup (LP: #1185108). === modified file 'init/job_class.c' --- init/job_class.c 2013-05-29 10:36:36 +0000 +++ init/job_class.c 2013-06-20 09:34:34 +0000 @@ -62,6 +62,9 @@ #include <json.h> extern json_object *json_classes; +extern int user_mode; +extern int no_inherit_env; +extern char **environ; /* Prototypes for static functions */ static void job_class_add (JobClass *class); @@ -115,10 +118,14 @@ { char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL }; - if (! job_environ) { - job_environ = NIH_MUST (nih_str_array_new (NULL)); - NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ)); - } + if (job_environ) + return; + + job_environ = NIH_MUST (nih_str_array_new (NULL)); + NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ)); + + if (user_mode && ! no_inherit_env) + NIH_MUST(environ_append (&job_environ, NULL, 0, TRUE, environ)); } /** === modified file 'init/job_process.c' --- init/job_process.c 2013-05-24 17:21:47 +0000 +++ init/job_process.c 2013-05-31 15:34:00 +0000 @@ -283,9 +283,6 @@ envc = 0; env = NIH_MUST (nih_str_array_new (NULL)); - if (user_mode && ! no_inherit_env) - NIH_MUST(environ_append (&env, NULL, &envc, TRUE, environ)); - if (job->env) NIH_MUST(environ_append (&env, NULL, &envc, TRUE, job->env)); === modified file 'util/man/initctl.8' --- util/man/initctl.8 2013-02-15 14:07:05 +0000 +++ util/man/initctl.8 2013-05-31 15:41:20 +0000 @@ -565,10 +565,13 @@ job-specific environment table; otherwise the global environment table that is applied to all jobs when they first start is queried. -Note that the global job environment table only includes the minimal set -of standard system variables added by the -.BR init (8) -daemon along with any variables set using +Note that the global job environment table comprises those variables +already set in the +.BR init (8) +daemons environment at startup, the minimal set of standard system +variables added by the +.BR init (8) +daemon, and any variables set using .BR set\-env "." See .BR init (5) === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2013-02-27 11:46:04 +0000 +++ util/tests/test_initctl.c 2013-06-03 09:07:16 +0000 @@ -16436,21 +16436,6 @@ assert (upstart_pid); assert (dbus_pid); - /*******************************************************************/ - /* Ensure basic variables are set in the current environment */ - - if (! getenv ("TERM")) { - fprintf (stderr, "WARNING: setting TERM to '%s' as not set\n", - TEST_INITCTL_DEFAULT_TERM); - assert0 (setenv ("TERM", TEST_INITCTL_DEFAULT_TERM, 1)); - } - - if (! getenv ("PATH")) { - fprintf (stderr, "WARNING: setting PATH to '%s' as not set\n", - TEST_INITCTL_DEFAULT_PATH); - assert0 (setenv ("PATH", TEST_INITCTL_DEFAULT_PATH, 1)); - } - cmd = nih_sprintf (NULL, "%s reset-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); @@ -16463,9 +16448,9 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - TEST_EQ (line_count, 2); - TEST_STR_MATCH (output[0], "PATH=*"); - TEST_STR_MATCH (output[1], "TERM=*"); + TEST_GE (line_count, 2); + TEST_STR_ARRAY_CONTAINS (output, "PATH=*"); + TEST_STR_ARRAY_CONTAINS (output, "TERM=*"); nih_free (output); /*******************************************************************/ @@ -16475,9 +16460,9 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - TEST_EQ (line_count, 2); - TEST_STR_MATCH (output[0], "PATH=*"); - TEST_STR_MATCH (output[1], "TERM=*"); + TEST_GE (line_count, 2); + TEST_STR_ARRAY_CONTAINS (output, "PATH=*"); + TEST_STR_ARRAY_CONTAINS (output, "TERM=*"); nih_free (output); /*******************************************************************/ @@ -16542,17 +16527,15 @@ fi = fopen (logfile, "r"); TEST_NE_P (fi, NULL); - TEST_FILE_MATCH (fi, "PATH=*"); - TEST_FILE_MATCH (fi, "TERM=*"); + TEST_FILE_CONTAINS (fi, "PATH=*"); + TEST_FILE_CONTAINS (fi, "TERM=*"); /* asterisk required to match '\r\n' */ - TEST_FILE_MATCH (fi, "UPSTART_JOB=foo*"); - TEST_FILE_MATCH (fi, "UPSTART_INSTANCE=*"); - TEST_FILE_MATCH (fi, "UPSTART_SESSION=*"); - TEST_FILE_END (fi); + TEST_FILE_CONTAINS (fi, "UPSTART_JOB=foo*"); + TEST_FILE_CONTAINS (fi, "UPSTART_INSTANCE=*"); + TEST_FILE_CONTAINS (fi, "UPSTART_SESSION=*"); fclose (fi); - DELETE_FILE (confdir, "foo.conf"); TEST_EQ (unlink (logfile), 0); @@ -16729,6 +16712,7 @@ cmd = nih_sprintf (NULL, "%s reset-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 0); nih_free (output); /* Ensure nothing changed */ @@ -16746,7 +16730,7 @@ name, value); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), name); @@ -16767,6 +16751,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("set-env in 'name=' form"); @@ -16777,6 +16762,7 @@ name); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 0); nih_free (output); cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), @@ -16802,6 +16788,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("set-env in 'name' form"); @@ -16837,6 +16824,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("set-env for already set variable"); @@ -16849,7 +16837,7 @@ name, value); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); /* check it */ cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), @@ -16866,7 +16854,7 @@ name, value); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); /* check it again */ cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), @@ -16889,6 +16877,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("set-env --retain"); @@ -16901,7 +16890,7 @@ name, value); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); /* check it */ cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), @@ -16917,7 +16906,7 @@ get_initctl (), name, "HELLO"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); /* check that value did *NOT* change */ cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), @@ -16939,6 +16928,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("set-env with space within value and trailing tab"); @@ -16950,7 +16940,7 @@ name, value); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s get-env %s 2>&1", get_initctl (), name); @@ -16971,6 +16961,7 @@ RUN_COMMAND (NULL, cmd, &output, &line_count); TEST_EQ (line_count, 1); TEST_EQ_STR (output[0], "initctl: No such variable: foo"); + nih_free (output); /*******************************************************************/ TEST_FEATURE ("list-env output order"); @@ -16981,19 +16972,19 @@ "zygote", "cell"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s set-env %s='%s' 2>&1", get_initctl (), "median", "middle"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s set-env %s='%s' 2>&1", get_initctl (), "aardvark", "mammal"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); @@ -17037,13 +17028,13 @@ "aardvark", "mammal"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s set-env %s='%s' 2>&1", get_initctl (), "zygote", "cell"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); @@ -17067,25 +17058,26 @@ "aardvark", "mammal"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s set-env %s='%s' 2>&1", get_initctl (), "FOO", "BAR"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); cmd = nih_sprintf (NULL, "%s set-env %s='%s' 2>&1", get_initctl (), "_________", "_________"); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); + TEST_EQ (line_count, 0); CREATE_FILE (confdir, "modified-env.conf", "exec env"); cmd = nih_sprintf (NULL, "%s start modified-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 1); nih_free (output); logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", @@ -17211,6 +17203,7 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 1); nih_free (output); logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", @@ -17254,6 +17247,7 @@ cmd = nih_sprintf (NULL, "%s start foo 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 1); nih_free (output); logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", @@ -17333,6 +17327,73 @@ /*******************************************************************/ } +void +test_no_inherit_job_env (const char *runtimedir, const char *confdir, const char *logdir) +{ + nih_local char *cmd = NULL; + char **output; + size_t lines; + pid_t upstart_pid = 0; + char *extra[] = { "--no-inherit-env", NULL }; + nih_local char *logfile = NULL; + nih_local char *session_file = NULL; + FILE *fi; + + start_upstart_common (&upstart_pid, TRUE, confdir, logdir, extra); + + /*******************************************************************/ + TEST_FEATURE ("ensure list-env in '--user --no-inherit-env' environment gives expected output"); + + cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + + /* environment should comprise the default environment only */ + TEST_EQ (lines, 2); + TEST_STR_MATCH (output[0], "PATH=*"); + TEST_STR_MATCH (output[1], "TERM=*"); + nih_free (output); + + /*******************************************************************/ + TEST_FEATURE ("ensure '--user --no-inherit-env' provides expected job environment"); + + CREATE_FILE (confdir, "foo.conf", "exec env"); + + cmd = nih_sprintf (NULL, "%s start foo 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + nih_free (output); + + logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", + logdir, + "foo.log")); + + WAIT_FOR_FILE (logfile); + + fi = fopen (logfile, "r"); + TEST_NE_P (fi, NULL); + TEST_FILE_CONTAINS (fi, "PATH=*"); + TEST_FILE_CONTAINS (fi, "TERM=*"); + + /* asterisk required to match '\r\n' */ + TEST_FILE_CONTAINS (fi, "UPSTART_JOB=foo*"); + TEST_FILE_CONTAINS (fi, "UPSTART_INSTANCE=*"); + TEST_FILE_CONTAINS (fi, "UPSTART_SESSION=*"); + fclose (fi); + + DELETE_FILE (confdir, "foo.conf"); + TEST_EQ (unlink (logfile), 0); + + /*******************************************************************/ + + session_file = NIH_MUST (nih_sprintf (NULL, "%s/upstart/sessions/%d.session", + runtimedir, (int)upstart_pid)); + + STOP_UPSTART (upstart_pid); + + unlink (session_file); +} + /* * Test all the commands which affect the job environment table together * as they are so closely related. @@ -17376,6 +17437,21 @@ TEST_EQ (setenv ("XDG_RUNTIME_DIR", runtimedir, 1), 0); + /*******************************************************************/ + /* Ensure basic variables are set in the current environment */ + + if (! getenv ("TERM")) { + fprintf (stderr, "WARNING: setting TERM to '%s' as not set\n", + TEST_INITCTL_DEFAULT_TERM); + assert0 (setenv ("TERM", TEST_INITCTL_DEFAULT_TERM, 1)); + } + + if (! getenv ("PATH")) { + fprintf (stderr, "WARNING: setting PATH to '%s' as not set\n", + TEST_INITCTL_DEFAULT_PATH); + assert0 (setenv ("PATH", TEST_INITCTL_DEFAULT_PATH, 1)); + } + TEST_DBUS (dbus_pid); start_upstart_common (&upstart_pid, TRUE, confdir, logdir, NULL); @@ -17410,14 +17486,21 @@ /*******************************************************************/ STOP_UPSTART (upstart_pid); + session_file = NIH_MUST (nih_sprintf (NULL, "%s/upstart/sessions/%d.session", + runtimedir, (int)upstart_pid)); + unlink (session_file); + + /*******************************************************************/ + + test_no_inherit_job_env (runtimedir, confdir, logdir); + + /*******************************************************************/ + TEST_DBUS_END (dbus_pid); assert0 (unsetenv ("UPSTART_CONFDIR")); assert0 (unsetenv ("UPSTART_LOGDIR")); assert0 (unsetenv ("UPSTART_SESSION")); - session_file = NIH_MUST (nih_sprintf (NULL, "%s/upstart/sessions/%d.session", - runtimedir, (int)upstart_pid)); - unlink (session_file); session_file = NIH_MUST (nih_sprintf (NULL, "%s/upstart/sessions", runtimedir)); TEST_EQ (rmdir (session_file), 0); session_file = NIH_MUST (nih_sprintf (NULL, "%s/upstart", runtimedir));
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
