Bug 988152

Thanks for report, as /tmp is now tmpfs and files are not that big, it shouldn't be a big issue. Worth to fix anyway. Reassigning to Ondrej O., who is the author of the include patch.

/tmp is NOT on /tmpfs in any of my systems (let's not get into that argument here).  I also don't reap files on /tmp, thus they will stay.

I moved a few lines around on both scripts and that may fix the problem in almost all cases (may need to rm the file on a trap to do it better). I'll post my changes if it seems to work and I don't see a patch here before.

*** Bug 994722 has been marked as a duplicate of this bug. ***

Should be fixed in coreutils-8.21-12.fc19

I just noticed this bug because a user who never logs in had a zillion .colorls files in /tmp.  It turns out that user has some procmail rules setup to run every time an email comes in, and each email time coincided with the .colorls file.

(User's shell is tcsh and sources the .csh's.)

Anyhow, even if comment #4 has this bug fixed, should this (and other scripts) NOT be running on non-interactive sessions?  The script seems to look at $TERM but not in such a way that it instantly aborts when it detects a non-interactive shell.  Can't all these scripts abort when non-interactive to not waste time?  For my purposes, I think most of the scripts are useless and I'd like a way to rm or disable many of them (like colorls), but yum will put them back in everytime coreutils updates.

Well, Scott's problem in the duplicate bug was that he set $PS1 even for non-interactive shells, which bypassed the simple way these scripts check for them (return if $PS1 not set). Maybe there's a similar issue with the user you mentioned.

I got on #bash and asked for the best way to test for an interactive shell and the answer was:

if [[ -t 0 ]]

Not to check for $PS1. I modified my script to do that and I haven't had the problem since. Perhaps the lscolors things should test that way also? Here is the documentation for this feature:

http://www.tldp.org/LDP/abs/html/intandnonint.html

Comment #6: ah, yes I see the $PS1 check in the .sh, but the equivalent check is not in the .csh (for colorls, not sure about the other scripts).

The .csh's should have the equivalent check for non-int.  The checks I use personally are along the lines of:

if (! $?prompt) goto _end

I can't see any equivalent to -t in [t]csh as per comment #7.

Putting a similar check/goto in the .csh's should suffice.

If I modify these .csh files to effect my fix in comment #8 can I put the patches here to have them incorporated?  Does the $?prompt test sound like a good one to rely on?

I also had a different idea that might make maintenance easier to avoid this problem in the future: what if the csh scripts call bash to run the bash scripts and somehow capture the changes they make to bash, and convert that to take effect in csh/tcsh.  By that I mean do something like call a wrapper script that saves the state (vars/envvars) of a new bash process, run the .sh version file, look at the state (vars/envvars) after the run, and spit out csh commands that would effect the same thing in csh/tcsh.  Then we could eval that output in csh/tcsh.

Yes, that is terribly inefficient, but perhaps ultimately the better way as the scripts are tiny and few.  Maintenance of the "csh version" of these files would then be "for free" essentially.

Either way, something should be done because the .csh files are currently *majorly broken* in the sense that they do not do the same thing as their .sh counterparts.

[[ -t 0 ]] seems like a bashism - we use .sh scripts on ksh, which limits us in many ways. Still definitely worth to improve.

I agree with the statement .csh files are broken and they should do the same thing as the .sh counterpart - if you have any patch/proposal, feel free to post it here, we can incorporate it into rawhide (and later released fedoras) version of coreutils. Check from comment #8 might be ok - do you have some more ideas what to fix the "*majorly broken*" csh path? I don't think that the "bash solution" is the right way.

Very strange, the attachment part of bugzilla isn't working for me (I hit submit but nothing happens).  The patches are so short, I will put them here:

Add interactive shell test at top and completely skip (exit) out of the file if we're not interactive.  Don't even set aliases, as they are useless in a non-int shell.  Exit (as opposed to sh's return) works fine here, as exiting a "source" command just returns from the "source" and doesn't quit the original shell.

For people like me who don't want color stuff set at all, even on interactive, the current code allows one to set USER_LS_COLORS=1 in the calling file (usually /etc/csh.cshrc) beforehand.

patch for /etc/profile.d/colorls.csh to skip on non-int shells and fix this bug

--- coreutils-colorls.csh	2013-05-08 02:40:49.000000000 -0500
+++ coreutils-colorls.csh.new	2013-12-22 08:19:33.051771231 -0600
@@ -1,3 +1,6 @@
+# skip everything for non-interactive shells
+if (! $?prompt) exit
+
 # color-ls initialization
 if ( $?USER_LS_COLORS ) then
   if ( "$USER_LS_COLORS" != "" ) then



Also, I've tweaked the bash version as it's also a bit buggy/suboptimal:


--- coreutils-colorls.sh	2013-05-08 02:40:49.000000000 -0500
+++ coreutils-colorls.sh.new	2013-12-22 08:31:07.200811536 -0600
@@ -1,15 +1,14 @@
 # color-ls initialization
 
+# Skip all for noninteractive shells.
+[ -z "$PS1" ] && return
+
 #when USER_LS_COLORS defined do not override user LS_COLORS, but use them.
 if [ -z "$USER_LS_COLORS" ]; then
 
   alias ll='ls -l' 2>/dev/null
   alias l.='ls -d .*' 2>/dev/null
 
-
-  # Skip the rest for noninteractive shells.
-  [ -z "$PS1" ] && return
-
   INCLUDE=
   COLORS=
   TMP="`mktemp .colorlsXXX --tmpdir=/tmp`"


There is no point in running any of the file if it's non-interactive (in either shell) as non-ints don't need aliases and certainly don't need ls coloring.

Also, on my system there are many other .csh files that could use the same treatment.  Some do only setenv, which is legit for non-int shells, so those can be left alone.  I identified these as also setting aliases and other int-only things:

/etc/profile.d/vim.csh
/etc/profile.d/colorgrep.csh

from
vim-enhanced-7.4.027-2.fc19.i686
grep-2.14-4.fc19.i686

Obviously different people will have different rpm's installed that create different .csh files.  An exhaustive scan and fix-up of all of them would be tricky.

For sure, colorls is the most egregious one, creating temp files/dirs.

Sorry, I forgot to fix the actual bug in cases where people use the default settings!  Below will fix the leftover temp stuff as it rm's at every exit point now.

--- coreutils-colorls.csh	2013-05-08 02:40:49.000000000 -0500
+++ coreutils-colorls.csh.new	2013-12-22 08:59:34.622138773 -0600
@@ -1,3 +1,6 @@
+# skip everything for non-interactive shells
+if (! $?prompt) exit
+
 # color-ls initialization
 if ( $?USER_LS_COLORS ) then
   if ( "$USER_LS_COLORS" != "" ) then
@@ -30,18 +33,25 @@
 endif
 set INCLUDE="`cat "$COLORS" | grep '^INCLUDE' | cut -d ' ' -f2-`"
 
-if ( ! -e "$COLORS" ) exit
+if ( ! -e "$COLORS" ) then
+  rm -f $TMP
+  exit
+endif
 
 if ( "$INCLUDE" != '' ) cat "$INCLUDE" > $TMP
 cat "$COLORS" | grep -v '^INCLUDE' >> $TMP
 
 eval "`dircolors -c $TMP`"
 
-if ( "$LS_COLORS" == '' ) exit
+if ( "$LS_COLORS" == '' ) then
+  rm -f $TMP
+  exit
+endif
 set color_none=`sed -n '/^COLOR.*none/Ip' < $COLORS`
 if ( "$color_none" != '' ) then
-   unset color_none
-   exit
+  unset color_none
+  rm -f $TMP
+  exit
 endif
 unset color_none
 rm -f $TMP

Also, the proper fix (ignore the one in comment #11) for bash .sh that takes care of cleaning up temp files:

--- coreutils-colorls.sh	2013-05-08 02:40:49.000000000 -0500
+++ coreutils-colorls.sh.new	2013-12-22 09:13:46.964471296 -0600
@@ -1,15 +1,14 @@
 # color-ls initialization
 
+# Skip all for noninteractive shells.
+[ -z "$PS1" ] && return
+
 #when USER_LS_COLORS defined do not override user LS_COLORS, but use them.
 if [ -z "$USER_LS_COLORS" ]; then
 
   alias ll='ls -l' 2>/dev/null
   alias l.='ls -d .*' 2>/dev/null
 
-
-  # Skip the rest for noninteractive shells.
-  [ -z "$PS1" ] && return
-
   INCLUDE=
   COLORS=
   TMP="`mktemp .colorlsXXX --tmpdir=/tmp`"
@@ -32,14 +31,23 @@
   COLORS="/etc/DIR_COLORS"
 
   # Existence of $COLORS already checked above.
-  [ -n "$COLORS" ] || return
+  if [ ! -n "$COLORS" ]; then
+    rm -f $TMP
+    return
+  fi
 
   [ -e "$INCLUDE" ] && cat "$INCLUDE" > $TMP
   cat "$COLORS" | grep -v '^INCLUDE' >> $TMP
 
   eval "`dircolors --sh $TMP 2>/dev/null`"
-  [ -z "$LS_COLORS" ] && return
-  grep -qi "^COLOR.*none" $COLORS >/dev/null 2>/dev/null && return
+  if [ -z "$LS_COLORS" ]; then
+    rm -f $TMP
+    return
+  fi
+  if grep -qi "^COLOR.*none" $COLORS >/dev/null 2>/dev/null; then
+    rm -f $TMP
+    return
+  fi
   rm -f $TMP
 fi

(In reply to Ondrej Vasik from comment #10)
> have some more ideas what to fix the "*majorly broken*" csh path? I don't
> think that the "bash solution" is the right way.

As for broken csh path, I suppose I could look into it, but it should be a new bug, no?  Someone should make it and put a link here.

Personally, I clobber the system's csh path setting with my own sane setting, so I am not impacted by this bug.  Yes, it would be nice if the system "did the right thing" for all csh/tcsh users!

Thanks for the patches, you are right the scripts should do the cleanup even for these cases of exits. With the tmpfs on /tmp it is not so scary, but still having file for each session until reboot is not nice. In addition, I will use only the "no aliases for noninteractive" part of your patches - I believe that the other chunk was fixed by http://pkgs.fedoraproject.org/cgit/coreutils.git/commit/coreutils-colorls.csh?id=f0b6f85aff0496c51c8fcb58197a8a132949a4e5 and http://pkgs.fedoraproject.org/cgit/coreutils.git/commit/coreutils-colorls.sh?id=f0b6f85aff0496c51c8fcb58197a8a132949a4e5 ... they already do the cleanup before any of exits (I'd suggest to use the rawhide version as a base, as there were other minor changes meanwhile).

Feel free to file a new bugzilla for other ".csh" tunings/fixes, if you want to.

*** Bug 1055086 has been marked as a duplicate of this bug. ***

coreutils-8.21-13.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/coreutils-8.21-13.fc19

Package coreutils-8.21-13.fc19:
* should fix your issue,
* was pushed to the Fedora 19 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing coreutils-8.21-13.fc19'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-1208/coreutils-8.21-13.fc19
then log in and leave karma (feedback).

coreutils-8.21-13.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.