James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1159895 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) Related bugs: Bug #1159895 in upstart : "Allow for unsetting inherited variables" https://bugs.launchpad.net/upstart/+bug/1159895 For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1159895/+merge/166836 = Behaviour Change = Previously running 'initctl list-env' in a Session Init environment would only list those minimal variables added by init (PATH and TERM) along with any variables explicitly set via 'initctl set-env'. However, it now lists that set plus all variables set in inits environment at startup time. * init/job_class.c: job_class_environment_init(): Copy inits environment to the default job class environment for user mode where appropriate. * 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. -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1159895/+merge/166836 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1159895 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2013-05-28 15:12:19 +0000 +++ ChangeLog 2013-05-31 15:44:26 +0000 @@ -1,3 +1,16 @@ +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. + 2013-05-27 Marc Deslauriers <[email protected]> * init/job.c: Don't check for user mode when trying to load an === modified file 'init/job_class.c' --- init/job_class.c 2013-05-15 13:21:54 +0000 +++ init/job_class.c 2013-05-31 15:44:26 +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:44:26 +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:44:26 +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-05-31 15:44:26 +0000 @@ -16463,9 +16463,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 +16475,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 +16542,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 +16727,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 +16745,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 +16766,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 +16777,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 +16803,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 +16839,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 +16852,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 +16869,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 +16892,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 +16905,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 +16921,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 +16943,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 +16955,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 +16976,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 +16987,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 +17043,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 +17073,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 +17218,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 +17262,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",
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
