Bug 257886

Summary: ls: don't check color env variables if compiled with WITHOUT_LS_COLORS="YES"
Product: Base System Reporter: Marko Turk <mt-bugs>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Some People CC: emaste, pstef
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch
none
patch again none

Description Marko Turk 2021-08-16 15:14:30 UTC
Created attachment 227246 [details]
patch

Hi,

if I compile with WITHOUT_LS_COLORS="YES", and if I have COLORTERM variable set, ls will complain each time:
ls: color support not compiled in

There is no way to suppres the message, it is printed every time.

If ls is compiled with no color support, it shouldn't even check for color env variables. The attached patch fixes that.

Regards,
Marko
Comment 1 Marko Turk 2021-08-16 15:18:52 UTC
Created attachment 227247 [details]
patch again
Comment 2 Piotr Pawel Stefaniak freebsd_committer freebsd_triage 2021-08-18 18:46:53 UTC
Hi,

I agree with you, but I would fix this another way: since the warnx is generally useless, I would like to remove it. But Ed pointed out that we may want to have a way to determine whether a particular copy of ls has been compiled with color support or not. So I think it's best to move the warnx to getopt in a WITHOUT_LS_COLORS build to fire if user asks for color. Like this:

diff --git a/bin/ls/ls.c b/bin/ls/ls.c
index 338b3d1d2a2..92575711251 100644
--- a/bin/ls/ls.c
+++ b/bin/ls/ls.c
@@ -105,9 +105,7 @@ static void  traverse(int, char **, int);

 static const struct option long_opts[] =
 {
-#ifdef COLORLS
         {"color",       optional_argument,      NULL, COLOR_OPT},
-#endif
         {NULL,          no_argument,            NULL, 0}
 };

@@ -448,8 +446,8 @@ main(int argc, char *argv[])
                case 'y':
                        f_samesort = 1;
                        break;
-#ifdef COLORLS
                case COLOR_OPT:
+#ifdef COLORLS
                        if (optarg == NULL || do_color_always(optarg))
                                colorflag = COLORFLAG_ALWAYS;
                        else if (do_color_auto(optarg))
@@ -460,6 +458,8 @@ main(int argc, char *argv[])
                                errx(2, "unsupported --color value '%s' (must be always, auto, or never)",
                                    optarg);
                        break;
+#else
+                       warnx("color support not compiled in");
 #endif
                default:
                case '?':
@@ -503,8 +503,6 @@ main(int argc, char *argv[])
                        f_color = 1;
                        explicitansi = true;
                }
-#else
-               warnx("color support not compiled in");
 #endif /*COLORLS*/
        }
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-08-19 19:24:55 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=ced2dcadccfcff8f7991b3cb5f6f70d6710eadfb

commit ced2dcadccfcff8f7991b3cb5f6f70d6710eadfb
Author:     Piotr Pawel Stefaniak <pstef@FreeBSD.org>
AuthorDate: 2021-08-18 20:47:37 +0000
Commit:     Piotr Pawel Stefaniak <pstef@FreeBSD.org>
CommitDate: 2021-08-19 19:22:16 +0000

    ls: prevent no-color build from complaining when COLORTERM is non-empty

    As 257886 reports, if ls(1) is built with WITHOUT_LS_COLORS="YES", it
    issues a warning whenever COLORTERM is non-empty. The warning is not
    useful, so I thought to remove it, but as Ed pointed out, we may want
    to have a way to determine whether a particular copy of ls has been
    compiled with color support or not.

    Therefore move the warnx() call to the getopt loop in
    a WITHOUT_LS_COLORS build to fire when the user asks for colored output.

    PR:             257886
    Reported by:    Marko Turk
    Reviewed by:    kevans

 bin/ls/ls.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)