On 09/15/2012 11:40 AM, Pádraig Brady wrote:
reopen 12445
stop
On 09/15/2012 03:28 AM, Matei David wrote:
On Sat, 15 Sep 2012 00:55:12 +0100
Pádraig Brady<[email protected]> wrote:
tag 12445 + notabug
close 12445
stop
On 09/14/2012 05:54 PM, Matei David wrote:
When the ORPHAN property is disabled in dircolors, ls --color does not
stat() symlinks. However, in this case symlinks are not colored using the
LINK property.
How to reproduce:
$ eval $(dircolors -p | sed 's/^ORPHAN.*/ORPHAN 00/' | dircolors -)
$ mkdir /tmp/xxx
$ ln -s /tmp/nowhere /tmp/xxx/yyy
$ ls --color /tmp/xxx
Expected output:
yyy, colored cyan (default for LINK)
Actual output:
yyy, not colored
Fix attached. Basically, in gobble_file(), assume a symlink is ok if stat()
is not run on it.
It seems this was fixed over 5 years ago in version 6.9
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=0a744370
Are you using an older version than that?
I'm marking as done for now.
thanks,
Pádraig.
I'm using coreutils 8.19. There are 2 lines which ever modify f->linkok (both
set it to true): 2700 and 2720. Both are inside the if block starting at
line 2560. By design, the if block is _not_ executed under the conditions I laid
out above in order to avoid both fstat() and stat() calls on the symlink (which
is ok!) However, the coloring is missing. Did you try my code?
The bug you mention was about avoiding the stat() call, i.e. following the
link. There is also this one, related:
http://git.savannah.gnu.org/cgit/coreutils.git/commit/src/ls.c?id=a085b6fc6c8fa03dac20f01f2f2b64b50b8bdf66
The behaviour I observe is: no fstat(), no stat(), (great so far...) but no
color either- that's the bug.
Drats, I was too hasty sorry.
ls 8.10 doesn't have the issue, and when I tested the latest version
I only did so with -l which also doesn't show the issue.
So this regression was added since 8.10 and only when -l is not used.
I'll add tests etc. and apply a fix in your name.
So the regression was introduced in coreutils 8.14 in:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=84457c49
I dug a little deeper and was confused as to the need
for the simulated "linkok" at all. That was originally introduced with:
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/10608/focus=10927
I also noticed that is_colored() was not used with C_ORPHAN,
thus meaning any of or=00, or=0, or= in LS_COLORS was considered significant.
One could argue that for any attribute, 00 should mean color as 00,
rather than don't color, but for consistency with all other color checks
I changed that to use is_colored(). As that was the only part of the code
inspecting
the "linkok" variable, I removed the simulated linkok referenced
in the above link.
All tests pass with the new code.
cheers,
Pádraig.
From ebf697b623435d46dd48998408e1c7cf3eba1623 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Sun, 16 Sep 2012 04:10:50 +0100
Subject: [PATCH] ls: fix coloring of dangling symlinks in default listing
mode
When listing a directory containing dangling symlinks,
and not outputting a long format listing, and orphaned links
are set to no coloring in LS_COLORS, then the symlinks
would get no color rather than reverting to the standard
symlink color. The issue was introduced in v8.13-19-g84457c4
* src/ls.c (print_color_indicator): Use the standard method
to check if coloring is specified for orphaned symlinks.
The existing method would consider 'or=00' or 'or=0' as significant
in LS_COLORS. Even 'or=' was significant as in that case the
string='or=' and the length=0. Also apply the same change
for missing symlinks for consistency.
(gobble_file): Remove the simulation of linkok, which is only
tested in print_color_indicator() which now handles this directly
by keying on the LS_COLORS values correctly.
* tests/misc/ls-misc.pl: Add a test case.
* THANKS: Add the reporter.
* NEWS: Mention the fix.
Reported-by: David Matei
---
NEWS | 4 ++++
THANKS.in | 1 +
src/ls.c | 11 ++---------
tests/misc/ls-misc.pl | 15 +++++++++++++++
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 2e69391..ab205f5 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ GNU coreutils NEWS -*-
outline -*-
numbers, and up to 8 times slower for some worst-case individual numbers.
[bug introduced in coreutils-7.0, with GNU MP support]
+ ls now correctly colors dangling symlinks when listing their containing
+ directories, with orphaned symlink coloring disabled in LS_COLORS.
+ [bug introduced in coreutils-8.14]
+
rm -i -d now prompts the user then removes an empty directory, rather
than ignoring the -d option and failing with an 'Is a directory' error.
[bug introduced in coreutils-8.19, with the addition of --dir (-d)]
diff --git a/THANKS.in b/THANKS.in
index 2c3f83c..b0061b3 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -153,6 +153,7 @@ David Godfrey [email protected]
David Luyer [email protected]
David Madore [email protected]
David Malone [email protected]
+David Matei [email protected]
Davide Canova [email protected]
Dawson Engler [email protected]
Dean Gaudet [email protected]
diff --git a/src/ls.c b/src/ls.c
index 9494ae9..106d234 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3064,12 +3064,6 @@ gobble_file (char const *name, enum filetype type, ino_t
inode,
free (linkname);
}
- /* When not distinguishing types of symlinks, pretend we know that
- it is stat'able, so that it will be colored as a regular symlink,
- and not as an orphan. */
- if (S_ISLNK (f->stat.st_mode) && !check_symlink_color)
- f->linkok = true;
-
if (S_ISLNK (f->stat.st_mode))
f->filetype = symbolic_link;
else if (S_ISDIR (f->stat.st_mode))
@@ -4293,7 +4287,7 @@ print_color_indicator (const struct fileinfo *f, bool
symlink_target)
/* Is this a nonexistent file? If so, linkok == -1. */
- if (linkok == -1 && color_indicator[C_MISSING].string != NULL)
+ if (linkok == -1 && is_colored (C_MISSING))
type = C_MISSING;
else if (!f->stat_ok)
{
@@ -4368,8 +4362,7 @@ print_color_indicator (const struct fileinfo *f, bool
symlink_target)
/* Adjust the color for orphaned symlinks. */
if (type == C_LINK && !linkok)
{
- if (color_symlink_as_referent
- || color_indicator[C_ORPHAN].string)
+ if (color_symlink_as_referent || is_colored (C_ORPHAN))
type = C_ORPHAN;
}
diff --git a/tests/misc/ls-misc.pl b/tests/misc/ls-misc.pl
index 71647f9..04d8016 100755
--- a/tests/misc/ls-misc.pl
+++ b/tests/misc/ls-misc.pl
@@ -263,6 +263,21 @@ my @Tests =
{POST => sub {unlink 's' or die "s: $!\n";
restore_ls_colors; }},
],
+ # The code related to the above fixes introduced a regression,
+ # that was fixed after coreutils-8.19. This is the case when
+ # listing a dir containing dangling symlinks, but with orphans uncolored.
+ # I.E. the same as the previous test, but listing the directory
+ # rather than the symlink directly.
+ ['sl-dangle9', '--color=always d',
+ {OUT => "$e\e[1;36ms$e\n"},
+ {PRE => sub {mkdir 'd',0755 or die "d: $!\n";
+ symlink 'dangle', 'd/s' or die "d/s: $!\n";
+ push_ls_colors('ln=1;36:or=:')
+ }},
+ {POST => sub {unlink 'd/s' or die "d/s: $!\n";
+ rmdir 'd' or die "d: $!\n";
+ restore_ls_colors; }},
+ ],
# Test for a bug that was introduced in coreutils-4.5.4; fixed in 4.5.5.
# To demonstrate it, the file in question (with executable bit set)
--
1.7.6.4
