James Hunt has proposed merging 
lp:~jamesodhunt/upstart/set-env--global-apply-to-running-jobs into lp:upstart.

Requested reviews:
  Upstart Reviewers (upstart-reviewers)

For more details, see:
https://code.launchpad.net/~jamesodhunt/upstart/set-env--global-apply-to-running-jobs/+merge/148754

This branch makes the '--global' option for the initctl commands 'set-env' and 
'unset-env' (but not 'reset-env') apply not just to all newly-started jobs (by 
modifying the global job environment table), but also to all *running* job 
objects environment tables. Note that this is not changing a running job 
processes *process environment*, it's changing a running job processes *Upstart 
environment table*.

This is a potentially confusing thing for a job to do, but is required for jobs 
which start early and wish to change the global environment from for example a 
pre-start stanza and have that change available to:

- all newly-started jobs.
- all subsequent job processes that the job which initiated the change runs.

As it could confuse, I've added a warning in the initctl man page on the use of 
--global for these commands.

Note that the branch somewhat arbitrarily does not apply such behaviour for the 
'reset-env' command as yanking a jobs carefully prepared environment between 
job processes is just asking for trouble. If really desired, a similar outcome 
can be effected using 'unset-env --global'.



-- 
https://code.launchpad.net/~jamesodhunt/upstart/set-env--global-apply-to-running-jobs/+merge/148754
Your team Upstart Reviewers is requested to review the proposed merge of 
lp:~jamesodhunt/upstart/set-env--global-apply-to-running-jobs into lp:upstart.
=== modified file 'ChangeLog'
--- ChangeLog	2013-02-08 16:55:25 +0000
+++ ChangeLog	2013-02-15 16:37:21 +0000
@@ -1,3 +1,14 @@
+2013-02-15  James Hunt  <[email protected]>
+
+	* init/event_operator.c: Typo.
+	* init/job_class.c:
+	  - job_class_environment_reset(): Comments.
+	  - job_class_environment_set(): Apply to all running job objects too.
+	  - job_class_environment_unset(): Apply to all running job objects too.
+	* util/man/initctl.8: Updated on environment command semantics.
+	* util/tests/test_initctl.c: test_global_and_local_job_env(): Modified
+	  test for new semantics.
+
 2013-02-08  James Hunt  <[email protected]>
 
 	* init/job_process.c: job_process_run(): Copy parent environment if

=== modified file 'init/event_operator.c'
--- init/event_operator.c	2012-09-20 13:07:53 +0000
+++ init/event_operator.c	2013-02-15 16:37:21 +0000
@@ -634,7 +634,7 @@
  * @parent: parent object for blocked structures,
  * @list: list to add events to.
  *
- * Collects events from the portion of the EventOperator tree rooted at @oper
+ * Collects events from the portion of the EventOperator tree rooted at @root
  * that are TRUE, ignoring the rest.
  *
  * Each event is blocked and a Blocked structure will be appended to @list

=== modified file 'init/job_class.c'
--- init/job_class.c	2013-01-31 17:23:55 +0000
+++ init/job_class.c	2013-02-15 16:37:21 +0000
@@ -125,6 +125,8 @@
  * job_class_environment_reset:
  *
  * Reset the environment back to defaults.
+ *
+ * Note: not applied to running job instances.
  **/
 void
 job_class_environment_reset (void)
@@ -157,6 +159,18 @@
 	if (! environ_add (&job_environ, NULL, NULL, replace, var))
 		return -1;
 
+	/* Update all running jobs */
+	NIH_HASH_FOREACH (job_classes, iter) {
+		JobClass *class = (JobClass *)iter;
+
+		NIH_HASH_FOREACH (class->instances, job_iter) {
+			Job *job = (Job *)job_iter;
+
+			if (! environ_add (&job->env, job, NULL, replace, var))
+				return -1;
+		}
+	}
+
 	return 0;
 }
 
@@ -178,6 +192,18 @@
 	if (! environ_remove (&job_environ, NULL, NULL, name))
 		return -1;
 
+	/* Update all running jobs */
+	NIH_HASH_FOREACH (job_classes, iter) {
+		JobClass *class = (JobClass *)iter;
+
+		NIH_HASH_FOREACH (class->instances, job_iter) {
+			Job *job = (Job *)job_iter;
+
+			if ( ! environ_remove (&job->env, job, NULL, name))
+				return -1;
+		}
+	}
+
 	return 0;
 }
 

=== modified file 'util/man/initctl.8'
--- util/man/initctl.8	2013-02-02 16:09:52 +0000
+++ util/man/initctl.8	2013-02-15 16:37:21 +0000
@@ -618,8 +618,16 @@
 .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.
+Operate on the global job environment table and all existing running job
+environment tables. This option is implied when not run from within a job.
+.sp
+This is an advanced option whose use is discouraged since it can change
+the environment of a job as it moves between different process stages
+(for example between
+.B pre\-start
+and the main process). See 
+.BR init (5)
+for further details.
 .RE
 .\"
 .TP
@@ -642,8 +650,16 @@
 .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.
+Operate on the global job environment table  and all existing running
+jobenvironment tables. This option is implied when not run from within a job.
+.sp
+This is an advanced option whose use is discouraged since it can change
+the environment of a job as it moves between different process stages
+(for example between
+.B pre\-start
+and the main process). See 
+.BR init (5)
+for further details.
 .RE
 .\"
 .TP
@@ -670,8 +686,11 @@
 .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.
+Operate on the global job environment table. This option is implied when
+not run from within a job.
+.sp
+Note that unlike \fBset\-env\fR and \fBunset\-env\fR, this option does
+not modify running job environment tables.
 .RE
 .\"
 .TP

=== modified file 'util/tests/test_initctl.c'
--- util/tests/test_initctl.c	2013-02-02 16:09:52 +0000
+++ util/tests/test_initctl.c	2013-02-15 16:37:21 +0000
@@ -16404,7 +16404,7 @@
 	DELETE_FILE (confdir, "foo.conf");
 
 	/*******************************************************************/
-	TEST_FEATURE ("ensure 'set-env --global' does not inject variable into main process");
+	TEST_FEATURE ("ensure 'set-env --global' can inject a variable into main process");
 
 	cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ());
 	TEST_NE_P (cmd, NULL);
@@ -16443,7 +16443,7 @@
 	/* we don't expect output from either set-env or get-env
 	 * (since 'hello' variable should not be set).
 	 */
-	TEST_FILE_MATCH (fi, "initctl: No such variable: hello*");
+	TEST_FILE_MATCH (fi, "world*\n");
 	TEST_FILE_END (fi);
 	fclose (fi);
 

-- 
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/upstart-devel

Reply via email to