James Hunt has proposed merging lp:~jamesodhunt/upstart/remove-initctl-job+instance-options into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/remove-initctl-job+instance-options/+merge/146272 This branch removes the ability for: - a user to modify an already running job instances environment table. - a job to modify _another jobs_ running environment table. The ability to do that was originally added since it simply reflects the full available D-Bus interface (which we do not wish to change) (*). The rationale for removing these initctl options are: 1) There is no obvious use-case for such a feature. 2) Any use of this could lead to unexpected behaviour and should thus be avoided. (*) - Removing these initctl options does not remove the ability to use such a feature, but it does make it marginally more difficult, and thus should be perceived as a good thing (tm). -- https://code.launchpad.net/~jamesodhunt/upstart/remove-initctl-job+instance-options/+merge/146272 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/remove-initctl-job+instance-options into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2013-01-31 17:23:55 +0000 +++ ChangeLog 2013-02-02 16:21:20 +0000 @@ -1,3 +1,12 @@ +2013-02-02 James Hunt <[email protected]> + + * util/initctl.c: Remove ability to specify explicitly job and/or job + instance values to the job environment commands. + * util/man/initctl.8: Remove job and job instance value command-line + options. + * util/tests/test_initctl.c: test_global_and_local_job_env(): Remove + tests for --job/--instance. + 2013-01-31 James Hunt <[email protected]> * init/control.c: === modified file 'util/initctl.c' --- util/initctl.c 2013-01-31 16:51:49 +0000 +++ util/initctl.c 2013-02-02 16:21:20 +0000 @@ -238,20 +238,6 @@ int apply_globally = FALSE; /** - * job_name: - * - * Name of job to apply changes to. - **/ -const char *job_name = NULL; - -/** - * job_instance: - * - * Name of job instance to apply changes to. - **/ -const char *job_instance = NULL; - -/** * NihOption setter function to handle selection of appropriate D-Bus * bus. * @@ -2820,10 +2806,7 @@ if (! details) return NULL; - if (job_name) { - upstart_job = job_name; - upstart_instance = job_instance ? job_instance : ""; - } else if (apply_globally) { + if (apply_globally) { upstart_job = upstart_instance = NULL; } else if ((upstart_job = getenv ("UPSTART_JOB")) != NULL) { upstart_instance = getenv ("UPSTART_INSTANCE"); @@ -3006,10 +2989,6 @@ NihOption set_env_options[] = { { 'g', "global", N_("apply to global job environment table"), NULL, NULL, &apply_globally, NULL }, - { 'i', "instance", N_("job instance name"), - NULL, "INSTANCE", &job_instance, NULL }, - { 'j', "job", N_("job name"), - NULL, "NAME", &job_name, NULL }, { 'r', "retain", N_("do not replace the value of the variable if already set"), NULL, NULL, &retain_var, NULL }, NIH_OPTION_LAST @@ -3023,10 +3002,6 @@ NihOption get_env_options[] = { { 'g', "global", N_("apply to global job environment table"), NULL, NULL, &apply_globally, NULL }, - { 'i', "instance", N_("job instance name"), - NULL, "INSTANCE", &job_instance, NULL }, - { 'j', "job", N_("job name"), - NULL, "NAME", &job_name, NULL }, NIH_OPTION_LAST }; @@ -3038,10 +3013,6 @@ NihOption unset_env_options[] = { { 'g', "global", N_("apply to global job environment table"), NULL, NULL, &apply_globally, NULL }, - { 'i', "instance", N_("job instance name"), - NULL, "INSTANCE", &job_instance, NULL }, - { 'j', "job", N_("job name"), - NULL, "NAME", &job_name, NULL }, NIH_OPTION_LAST }; @@ -3053,10 +3024,6 @@ NihOption list_env_options[] = { { 'g', "global", N_("apply to global job environment table"), NULL, NULL, &apply_globally, NULL }, - { 'i', "instance", N_("job instance name"), - NULL, "INSTANCE", &job_instance, NULL }, - { 'j', "job", N_("job name"), - NULL, "NAME", &job_name, NULL }, NIH_OPTION_LAST }; @@ -3068,10 +3035,6 @@ NihOption reset_env_options[] = { { 'g', "global", N_("apply to global job environment table"), NULL, NULL, &apply_globally, NULL }, - { 'i', "instance", N_("job instance name"), - NULL, "INSTANCE", &job_instance, NULL }, - { 'j', "job", N_("job name"), - NULL, "NAME", &job_name, NULL }, NIH_OPTION_LAST }; === modified file 'util/man/initctl.8' --- util/man/initctl.8 2013-01-31 16:51:49 +0000 +++ util/man/initctl.8 2013-02-02 16:21:20 +0000 @@ -576,17 +576,6 @@ .B OPTIONS .RS -.IP "\fB\-j\fP, \fB\-\-job\fP" -Operate on the job environment table for the specified job. This option is -implied when run from within a job. -.RE -.RS -.IP "\fB\-i\fP, \fB\-\-instance\fP" -Operate on the job environment table for the specified job instance -(requires \fI\-\-job\fR). This option is implied when run from within a -job. -.RE -.RS .IP "\fB\-g\fP, \fB\-\-global\fP" Operate on the global job environment table. This option is implied when not run from within a job. @@ -604,17 +593,6 @@ .B OPTIONS .RS -.IP "\fB\-j\fP, \fB\-\-job\fP" -Operate on the job environment table for the specified job. This option is -implied when run from within a job. -.RE -.RS -.IP "\fB\-i\fP, \fB\-\-instance\fP" -Operate on the job environment table for the specified job instance -(requires \fI\-\-job\fR). This option is implied when run from within a -job. -.RE -.RS .IP "\fB\-g\fP, \fB\-\-global\fP" Operate on the global job environment table. This option is implied when not run from within a job. @@ -639,17 +617,6 @@ If the specified variable is already set, do not modify it. .RE .RS -.IP "\fB\-j\fP, \fB\-\-job\fP" -Operate on the job environment table for the specified job. This option is -implied when run from within a job. -.RE -.RS -.IP "\fB\-i\fP, \fB\-\-instance\fP" -Operate on the job environment table for the specified job instance -(requires \fI\-\-job\fR). This option is implied when run from within a -job. -.RE -.RS .IP "\fB\-g\fP, \fB\-\-global\fP" Operate on the global job environment table. This option is implied when not run from within a job. @@ -674,17 +641,6 @@ If the specified variable is already set, do not modify it. .RE .RS -.IP "\fB\-j\fP, \fB\-\-job\fP" -Operate on the job environment table for the specified job. This option is -implied when run from within a job. -.RE -.RS -.IP "\fB\-i\fP, \fB\-\-instance\fP" -Operate on the job environment table for the specified job instance -(requires \fI\-\-job\fR). This option is implied when run from within a -job. -.RE -.RS .IP "\fB\-g\fP, \fB\-\-global\fP" Operate on the global job environment table. This option is implied when not run from within a job. @@ -713,17 +669,6 @@ If the specified variable is already set, do not modify it. .RE .RS -.IP "\fB\-j\fP, \fB\-\-job\fP" -Operate on the job environment table for the specified job. This option is -implied when run from within a job. -.RE -.RS -.IP "\fB\-i\fP, \fB\-\-instance\fP" -Operate on the job environment table for the specified job instance -(requires \fI\-\-job\fR). This option is implied when run from within a -job. -.RE -.RS .IP "\fB\-g\fP, \fB\-\-global\fP" Operate on the global job environment table. This option is implied when not run from within a job. === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2013-01-31 17:23:55 +0000 +++ util/tests/test_initctl.c 2013-02-02 16:21:20 +0000 @@ -16364,7 +16364,6 @@ char **output; size_t line_count; FILE *fi; - char flagfile[PATH_MAX]; assert (confdir); assert (logdir); @@ -16405,100 +16404,6 @@ DELETE_FILE (confdir, "foo.conf"); /*******************************************************************/ - TEST_FEATURE ("ensure set-env with explicit job arg can inject variable into main process"); - - contents = nih_sprintf (NULL, - "pre-start exec %s set-env --job \"$UPSTART_JOB\" hello=world\n" - "exec %s list-env\n", - get_initctl (), get_initctl ()); - TEST_NE_P (contents, NULL); - - CREATE_FILE (confdir, "foo.conf", contents); - - cmd = nih_sprintf (NULL, "%s start foo 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); - - logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", - logdir, - "foo.log")); - - WAIT_FOR_FILE (logfile); - - fi = fopen (logfile, "r"); - TEST_FILE_CONTAINS (fi, "hello=world*"); - TEST_NE_P (fi, NULL); - - fclose (fi); - - TEST_EQ (unlink (logfile), 0); - DELETE_FILE (confdir, "foo.conf"); - - /*******************************************************************/ - TEST_FEATURE ("ensure set-env with explicit instance arg can inject variable into main process"); - - contents = nih_sprintf (NULL, - "pre-start exec %s set-env --instance \"$UPSTART_INSTANCE\" hello=world\n" - "exec %s list-env\n", - get_initctl (), get_initctl ()); - TEST_NE_P (contents, NULL); - - CREATE_FILE (confdir, "foo.conf", contents); - - cmd = nih_sprintf (NULL, "%s start foo 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); - - logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", - logdir, - "foo.log")); - - WAIT_FOR_FILE (logfile); - - fi = fopen (logfile, "r"); - TEST_FILE_CONTAINS (fi, "hello=world*"); - TEST_NE_P (fi, NULL); - - fclose (fi); - - TEST_EQ (unlink (logfile), 0); - DELETE_FILE (confdir, "foo.conf"); - - /*******************************************************************/ - TEST_FEATURE ("ensure set-env with explicit job+instance args can inject variable into main process"); - - contents = nih_sprintf (NULL, - "pre-start exec %s set-env --job \"$UPSTART_JOB\" " - "--instance \"$UPSTART_INSTANCE\" hello=world\n" - "exec %s list-env\n", - get_initctl (), get_initctl ()); - TEST_NE_P (contents, NULL); - - CREATE_FILE (confdir, "foo.conf", contents); - - cmd = nih_sprintf (NULL, "%s start foo 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); - - logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", - logdir, - "foo.log")); - - WAIT_FOR_FILE (logfile); - - fi = fopen (logfile, "r"); - TEST_FILE_CONTAINS (fi, "hello=world*"); - TEST_NE_P (fi, NULL); - - fclose (fi); - - TEST_EQ (unlink (logfile), 0); - DELETE_FILE (confdir, "foo.conf"); - - /*******************************************************************/ TEST_FEATURE ("ensure 'set-env --global' does not inject variable into main process"); cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ()); @@ -16596,86 +16501,9 @@ TEST_STR_ARRAY_NOT_CONTAINS (output, "hello=world"); nih_free (output); - TEST_EQ (unlink (logfile), 0); + assert0 (unlink (logfile)); DELETE_FILE (confdir, "bar.conf"); - /*******************************************************************/ - TEST_FEATURE ("ensure set-env outside job with job args catches invalid job"); - - cmd = nih_sprintf (NULL, "%s set-env -j foo hello=world 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - TEST_EQ (line_count, 1); - TEST_EQ_STR (output[0], "initctl: Unknown job: foo"); - nih_free (output); - - /*******************************************************************/ - TEST_FEATURE ("ensure set-env outside job with job args modifies job"); - - TEST_FILENAME (flagfile); - (void)unlink (flagfile); - - contents = nih_sprintf (NULL, - "pre-start script\n" - " i=0\n" - " # timeout after 5 seconds\n" - " while [ $i -lt 50 ]\n" - " do\n" - " [ -f \"%s\" ] && break\n" - " sleep 0.1\n" - " i=$((i+1))\n" - " done\n" - "end script\n" - "\n" - "exec env\n", flagfile); - TEST_NE_P (contents, NULL); - - CREATE_FILE (confdir, "foo.conf", contents); - - cmd = nih_sprintf (NULL, "%s start --no-wait foo 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - nih_free (output); - - /* set variable */ - cmd = nih_sprintf (NULL, "%s set-env -j foo hello=world 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - - /* check that it did NOT get set globally */ - cmd = nih_sprintf (NULL, "%s get-env --global hello 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - TEST_EQ (line_count, 1); - TEST_EQ_STR (output[0], "initctl: No such variable: hello"); - - /* check that it DID get set for the job in question */ - cmd = nih_sprintf (NULL, "%s get-env -j foo hello 2>&1", get_initctl ()); - TEST_NE_P (cmd, NULL); - RUN_COMMAND (NULL, cmd, &output, &line_count); - TEST_EQ (line_count, 1); - TEST_EQ_STR (output[0], "world"); - - /* Create flag file to allow job to continue so it can run its - * main stanza. - */ - assert0 (fclose (fopen (flagfile, "w"))); - - 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, "hello=world*"); - - fclose (fi); - - assert0 (unlink (flagfile)); - assert0 (unlink (logfile)); - DELETE_FILE (confdir, "foo.conf"); /*******************************************************************/ }
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
