Red Hat Bugzilla – Full Text Bug Listing
|Summary:||shouldn't override user's LS_COLORS|
|Product:||[Fedora] Fedora||Reporter:||Mikel Ward <mikel>|
|Component:||coreutils||Assignee:||Ondrej Vasik <ovasik>|
|Status:||CLOSED CURRENTRELEASE||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Fixed In Version:||coreutils-6.9-16.fc8||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|Last Closed:||2008-03-17 08:23:59 EDT||Type:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
Description Mikel Ward 2008-01-29 18:37:31 EST
/etc/profile.d/colorls.sh overrides my $LS_COLORS environment variable. If I've already set up my own colors, this script shouldn't clobber them. The obvious change is to put [ -n "$LS_COLORS" ] && return at the top of the script.
Comment 1 Ondrej Vasik 2008-01-31 11:58:12 EST
Thanks for suggestion... Actually ... it could cause ls working without colors when no ls aliases would be defined. Therefore used if [ -n "$LS_COLORS" ]; then #do not override user LS_COLORS but use them alias ll='ls -l --color=tty' 2>/dev/null alias l.='ls -d .* --color=tty' 2>/dev/null alias ls='ls --color=tty' 2>/dev/null exit fi With following change built as coreutils-6.10-4.fc9 in rawhide. Will be in next update of F8.
Comment 2 Mikel Ward 2008-01-31 15:56:42 EST
That check doesn't do anything useful. The test needs to be around the dircolors stuff. If you want to keep the ls aliases, then do if [ -z "$LS_COLORS" ]; then [ -f ~/.dircolors.$TERM ] && eval `dircolors ~/.dircolors.$TERM ... fi
Comment 3 Ondrej Vasik 2008-01-31 16:31:29 EST
Why do you think that this check doesn't do anything useful? If not zero length of LS_COLORS then expect LS_COLORS correctly set and use colored aliases for ls ... otherwise make the rest of script as it was before... It works on my machine without problems - where you see the problem? Your LS_COLORS will be not overwritten by colorls.sh and you will have colored ls as you have specified in LS_COLORS...
Comment 4 Mikel Ward 2008-01-31 17:21:30 EST
The problem is these lines: COLORS=/etc/DIR_COLORS [ -e "/etc/DIR_COLORS.$TERM" ] && COLORS="/etc/DIR_COLORS.$TERM" [ -e "$HOME/.dircolors" ] && COLORS="$HOME/.dircolors" [ -e "$HOME/.dircolors.$TERM" ] && COLORS="$HOME/.dircolors.$TERM" [ -e "$HOME/.dir_colors" ] && COLORS="$HOME/.dir_colors" [ -e "$HOME/.dir_colors.$TERM" ] && COLORS="$HOME/.dir_colors.$TERM" [ -e "$COLORS" ] || return They override my LS_COLORS if I have already set it.
Comment 5 Mikel Ward 2008-01-31 18:03:35 EST
Actually it's this line: eval `dircolors --sh "$COLORS"` but I'd wrap all that in the LS_COLORS test.
Comment 6 Ondrej Vasik 2008-02-01 01:34:03 EST
Created attachment 293693 [details] colorls.sh which doesn't override user defined LS_COLORS I know what overrides LS_COLORS - and current version in RAWHIDE DOES NOT override LS_COLORS ... I think the best way is to attach the shell script... I think you don't understand where I added lines from comment #1. It is more clear now? I know that there is now doubled code, but anyway I have to rework colorls.sh (which is quiet old - as were DIR_COLORS shipped until coreutils 6.10) because of 256 color support, so I will try to improve it somehow.
Comment 7 Mikel Ward 2008-02-01 02:46:34 EST
You don't understand. Try running dircolors ~/.dircolor. You will see it prints commands to set LS_COLORS. Running eval `dircolors ~/.dircolors` runs the commands printed by dircolors ~/.dircolors. It is very clear that dircolors is setting LS_COLORS. My environment sets LS_COLORS before /etc/profile.d/colorls.sh is run, so my custom LS_COLORS setting is lost.
Comment 8 Ondrej Vasik 2008-02-02 04:50:36 EST
Ok, so maybe I don't understand... so please try to explain WHERE I'm using eval `dircolors ~/.dircolors` in RAWHIDE version of colorls.sh in the case that you have your LS_COLORS set. I'm sorry for typo in attachment(this version has -z instead of -n), but in comment #1 is correct sequence used in .sh script shipped in RAWHIDE branch(coreutils-6.10-4.fc9) and attachment was here only for clarification where the code from comment #1 is used. First lines of the RAWHIDE colorls.sh (with additional comments for COLOR_LS value set): # color-ls initialization if [ -n "$LS_COLORS" ]; then # if your LS_COLORS variable has nonzero length # do not override user defined colors and use them #set aliases for colored ls alias ll='ls -l --color=tty' 2>/dev/null alias l.='ls -d .* --color=tty' 2>/dev/null alias ls='ls --color=tty' 2>/dev/null #THIS WILL EXIT FROM THE SHELL SCRIPT return fi # you will never get there when you had LS_COLORS set from e.g. ~/.bashrc # (because of non zero length "if" with return) ... rest of the shell script which will get LS_COLORS from e.g. /etc/DIR_COLORS Could you please clarify where you see the line which will override your LS_COLORS?
Comment 9 Mikel Ward 2008-02-02 06:02:35 EST
Ah, sorry, I missed the exit in comment 1 (and the corresponding return in the new version). To me the following layout would make more sense: if [ -z "$LS_COLORS" ]; then COLORS=... eval `dircolors --sh "$COLORS"` fi if [ -n "$LS_COLORS" ]; then alias ll=... fi
Comment 10 Ondrej Vasik 2008-02-02 08:06:01 EST
As I said in comment #6 , colorls.sh(.csh) has to be rewritten because of 256 color support and because of broken design of DIR_COLORS.xterm usage ... this file was created for light background (default in XTERM), but TERM type XTERM is used by many others terminals(e.g. gnome terminal with default black background). I will see if your layout from comment #9 will be usable in reworked colorls.sh, but definitely is better than the old one. Thanks for suggestion...
Comment 11 Fedora Update System 2008-02-20 21:52:33 EST
coreutils-6.9-13.fc8 has been pushed to the Fedora 8 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update coreutils'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-1844
Comment 12 Fedora Update System 2008-02-28 16:46:18 EST
coreutils-6.9-13.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.
Comment 13 Ondrej Vasik 2008-03-11 09:29:22 EDT
Sorry, I have to reopen the bug because of the complaints about the current behaviour. The old one .sh/.csh script overrides user defined LS_COLORS, but makes LS_COLORS flexible - you may have basic LS_COLORS from /etc/DIR_COLORS and once you will start xterm, they will get overriden by /etc/DIR_COLORS.xterm . This is broken by the new version . So I have to remove those changes. What do you think about the possibility of env.variable like USER_LS_COLORS - which will block overwriting of LS_COLORS? This is more suitable solution. Let me know about your opinion - but I can't use current approach with [ -z LS_COLORS ];.
Comment 14 Mikel Ward 2008-03-11 17:03:29 EDT
So your use case is: 1. /etc/DIR_COLORS is sourced when you log in to X (where TERM is empty) 2. you start an xterm and want /etc/DIR_COLORS.xterm to be run One thought is to only run dircolors (and hence set LS_COLORS) if TERM is set. This could be done by wrapping the whole thing in an "if [ -n "$TERM" ]; then" or similar.
Comment 15 Ondrej Vasik 2008-03-12 04:21:15 EDT
No, other user case is: 1) runlevel 3 (TERM is linux(not empty), uses /etc/DIR_COLORS ) 2) startx (and then start xterm) - TERM is xterm, should use /etc/DIR_COLORS.xterm but the behaviour is basically same like you described... Previous solution with [ -n $LS_COLORS ] is wrong and I don't see any suitable solution - as you should be able to switch between /etc/DIR_COLORS.xterm, /etc/DIR_COLORS (and in devel branch additionally with /etc/DIR_COLORS.256colors if the term is able to support 256 colors). So rely only on the TERM variable is not usable idea - basically the same as the [ -n $LS_COLORS ] . Solution via additional env.variable will work in all cases - but if used, should be documented somehow ( I used that approach in latest builds, to make old behaviour running again ). At any case - you could always add export LS_COLORS=<your user settings> to the .bashrc , which is executed after shell scripts from /etc/profile.d/ (like colorls.sh) - and override settings from colorls shell script. I like the whole idea of keeping user defined LS_COLORS variable, but I don't see any easy solution without additional env.variable ("if [ -z USER_LS_COLORS ]; then <override LS_COLORS script> fi" sounds ok for me - it makes you able to keep LS_COLORS if you wish) .
Comment 16 Mikel Ward 2008-03-12 04:46:42 EDT
Testing whether $TERM is set would work in those situations: 1) TERM is set, LS_COLORS is unset => run dircolors to set LS_COLORS 2) I think startx starts bash as a login shell, which would erase the previous environment, and thus /etc/profile.d/colorls.sh would be run for the first time when the xterm is started, but I could be wrong (I don't have a Linux box in front of me to test) The only other case to consider is if you start a terminal from inside another one, e.g. run gnome-terminal inside an xterm. What I find most problematic is that it runs from /etc/profile as well as /etc/bashrc. I will get more evidence and then elaborate.
Comment 17 Mikel Ward 2008-03-17 08:10:14 EDT
OK. I was relying on .zshenv not getting clobbered by /etc/zprofile and /etc/zshrc. I've now got rid of .zshenv and made my zsh setup more like bash, and I can deal with the current setup. Thanks for your help.
Comment 18 Ondrej Vasik 2008-03-17 08:23:59 EDT
Ok, closing once more time CURRENTRELEASE as there is now a way to keep user defined LS_COLORS via env. variable - which should be harmless for the rest unless is set.