[Merge] lp:~stgraber/upstart/upstart-session-socket into lp:upstart

[Stéphane Graber]

> > * conf/rc-sysinit.conf: I've got some reservations about changing the
> >   default jobs we provide. That said, those building systems should
> >   really be creating their own jobs. I think I'd be happy if we added a
> >   note to the README stating that the files in 'conf/' are examples only
> >   and *will* need to be changed by those creating systems using Upstart.
> > * conf/rc.conf: As above.
> 
> Those two ended up in this branch by mistake, that should be fixed now.
> 
> > * init/control.c: control_init(): Spaces before '(' (4 occurences).

Fixed.

> > * init/job_process.c:
> > * init/main.c:
> > * util/initctl.c:
> >   - user_mode: Comment should be something like 'if TRUE, talk to
> >     Upstart over a private D-Bus socket'.

Changed.

> >   - upstart_open():
> >     - Space before '(' of getenv call (2 occurences).

Just one after the fix below, but fixed :)

> >     - I'd be tempted to save the value of the first call to getenv to
> >       avoid the 2nd call.

Fixed.

> >     - 'else' should be on same line as closing brace of 'if'.

Fixed. Working on 3 different C projects each with different coding guideline is a real source of headache ;)

> >     - Shouldn't the logic be inverted here to conform to?:
> >
> https://wiki.ubuntu.com/FoundationsTeam/Specs/RaringUpstartUserSessions#
> > Effect_of_UPSTART_SESSION
> >       So, by default, if UPSTART_SESSION is set, use the private D-Bus
> >       socket, unless the user specifies '--system'.

Yeah, that part is still a bit blurry in my head, I'll try to catch you tomorrow to discuss this some more.

Does that table mean we won't ever look for upstart on the session bus when passed --user?

Also, I'm at a bit of a loss as to how "initctl --user" as root without UPSTART_SESSION isn't failing according to that table?

> >   - options: I'd be tempted to change 'start' to 'run' as the former
> >     implies a long-running operation.

Done, was a copy/paste from main.c.

> > * init/man/init.5: 'Job environment' section needs to be updated for
> >   'UPSTART_SESSION'.

Done

> > * util/man/initctl.8: Need to document '--user'.

Done

> > * util/tests/test_initctl.c: We could do with a test for '--user'. This
> >   will necessitate something like:
> >   - Changing _START_UPSTART() into a function and allowing atleast 1 extra
> >     argument to be specified or specifying the type (meaning '--user' or
> >     '--session') would work, along with a test
> >   - Changing WAIT_FOR_UPSTART() into a function to allow the session
> >     bus or private socket to be connected to.
> >   - Then, a test to simply start upstart, wait for it, create a conf
> >     file, check that initctl list shows that job, then stop upstart
> >     (see test_list()).

Will look into this once we agree on what initctl should do :)
-- 
https://code.launchpad.net/~stgraber/upstart/upstart-session-socket/+merge/143344
Your team Upstart Reviewers is requested to review the proposed merge of lp:~stgraber/upstart/upstart-session-socket into lp:upstart.