Code review comment for lp:~xnox/upstart/user-log-dir

> OK, you've convinced me on paths.h :-)
>
> I've updated the spec such that we have a script which sets the appropriate
> XDG vars if not already set *and* creates them if they don't exist. It will

no, this is wrong again.
It's perfectly valid to not have XDG_HOME_* variables set, as by default the default fallbacks should be used.
It's only meant for a user to change them if they want to launch set of applications with "clean" config, cache.
The XDG_RUNTIME_DIR in ubuntu is already set by libpam-xdg-support
https://blueprints.launchpad.net/ubuntu/+spec/foundations-q-xdg-runtime-dir

So these variables should not be set, if they don't already exist.

Creating the directories if they don't yet exist is a good point. Especially brand new accounts, might not have those directories.
Oh and as per XDG spec we should create them with permissions 0700 if we need to access them.

The spec is unclear about XDG_RUNTIME_DIR, I'd like to assert if it doesn't exist. In ubuntu it exists in quantal and up. To be compliant with the spec we should gracefully fallback to some other place, but I don't know what that other place is a good to fallback to.

> then start the Session Init. However, I wonder if the Session Init should also
> attempt to create the directories returned by xdg_get_cache_home() and
> xdg_get_config_home() to handle the scenario that a user logs in multiple
> times and in the first session say deletes one or both of these directories.
>

I don't like the idea of creating empty directories, just to place inotify watch on it (for the $XDG_CONFIG_HOME/upstart).
We intend to generate logs everytime, so precreating $XDG_CACHE_HOME/upstart makes sense.

> I think that to be conformant to the XDG spec, the alluded-to script can as
> Dmitrijs says run 'initctl list-sessions' and if that returns nothing, remove
> XDG_RUNTIME_DIR before exiting.
>

I believe XDG_RUNTIME_DIR is cleaned up for us after the last logout. I'll check that. Or do you mean to clean up bits in the $XDG_RUNTIME_DIR/upstart/ that we created? E.g. remove pidfiles on exit?

(outside of this merge proposal as far as I can see)

> After scrutinizing the XDG spec again, we do need to make some minor changes:
>
> * init/xdg.c:
> - xdg_get_cache_home(): This must check that if XDG_CACHE_HOME is set that
> it also is an absolute path before we use it.
> - xdg_get_config_home(): Same logic as above for XDG_CONFIG_HOME.
> - get_user_upstart_dirs(): Same logic for XDG_CONFIG_DIRS.

All good points.