On Fri, Aug 24, 2012 at 06:16:43PM +0200, Tom Gundersen wrote:
On Fri, Aug 24, 2012 at 4:23 PM, Dave Reisner <d@falconindy.com> wrote:
This is going to read locale from root's home, if such a thing exists. An odd case perhaps, but do we really want this side effect?
Hm, $HOME is set here...? How absurd. Good catch. I suppose we just want to unset $HOME?
Ah, no nevermind... It's a non-interactive shell that was spawned by PID 1. HOME isn't going to be set here. A dump of the available environment just shows me: CONSOLE=/dev/console SHELL=/bin/sh TERM=linux INIT_VERSION=sysvinit-2.88 RD_TIMESTAMP= PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin RUNLEVEL=S PWD=/ LANG=C PREVLEVEL=N SYSTEMD_LOG_LEVEL=notice SHLVL=1 Still, I guess my point is that we should be testing for the presence of variables before trying to expand them. if [ -n "$HOME" ] && [ -s "$HOME/.config/locale.conf ]; then ...
No, we don't want that side effect. On the other hand, we do not want to duplicate the code, so better find some way to make it work :)
-if [ -s /etc/locale.conf ]; then +if [ -s $XDG_CONFIG_HOME/locale.conf ]; then + . $XDG_CONFIG_HOME/locale.conf
This is a problem. I'm going to say that most of the time, XDG_CONFIG_HOME won't be defined here. We read iterate over /etc/profile.d in glob order (defined by current LC_COLLATE). If XDG_CONFIG_HOME is set by /etc/profile.d/xorg.sh, we aren't ever going to see this var defined.
The way XDG_* is set is just messed up. One might hope that one day it will be done better (say by a pam module). Shouldn't need to be part of xorg, it is useful even with no graphical login.
I think other distros might order scripts by number. I've always thought that /etc/profile.d stuff is pretty ridiculous, but it's also roughly par for the course when you're talking about anything involving shell.
In addition, when XDG_CONFIG_HOME isn't defined, then under absurd circumstances, /locale.conf would be read. I think you meant to combine this case with the one below it for $HOME/config:
if [ -s "${XDG_CONFIG_HOME:-$HOME/.config}/locale.conf" ]; then . "${XDG_CONFIG_HOME:-$HOME/.config}/locale.conf" fi
But, again, with the current conditions, this just won't ever be defined. It's furthermore, impossible to set/override XDG_CONFIG_HOME in user config since it's all read post /etc/profile.
Maybe we just stick to $HOME/.config/locale.conf?
Might as well, we can add a comment about adding back XDG_CONFIG_HOME if it is ever likely to work. That said, I think your argument about reading /locale.conf would also apply to $HOME being unset (?), so we'd need to deal with that too.
Thanks for the review,
Tom