[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ls - colorize files with capabilities
From: |
Jim Meyering |
Subject: |
Re: [PATCH] ls - colorize files with capabilities |
Date: |
Tue, 22 Jul 2008 15:25:07 +0200 |
Kamil Dudka <address@hidden> wrote:
> From 4f14f5e39d7687ddb252f5a7560724179635c944 Mon Sep 17 00:00:00 2001
> From: Kamil Dudka <address@hidden>
> Date: Tue, 22 Jul 2008 12:53:00 +0200
> Subject: [PATCH] --color now highlights files with capabilities, too
Please make the first line of the log message start with
the name of the affected tool:
ls: --color now highlights files with capabilities, too
> * configure.ac: --disable-libcap configure parameter, check for libcap
> usability
> * src/Makefile.am: libcap library linking
> * src/ls.c (has_capability): new function for capability detection
> * src/dircolors.c: updated color lists
> * src/dircolors.hin: brief description of CAPABILITY color attribute
> (print_color_indicator): colorize file with capability
> * tests/ls/capability: test for ls - colorize file with capability (root
> only)
> * tests/Makefile.am (TESTS): Add ls/capability.
> * NEWS: mentioned the change
While you're editing the log, please remove the spaces before each "*"
and adjust the rest to look like this: note changes in tense, wording,
punctuation, capitalization.
* configure.ac: New option: --disable-libcap. Check for libcap usability.
* src/Makefile.am (dir_LDADD, ls_LDADD, ...): Append $(LIB_CAP).
* src/ls.c [HAVE_CAP] Include <sys/capability.h>.
(has_capability): New function for capability detection.
* src/dircolors.c: Update color lists.
* src/dircolors.hin: Mention new CAPABILITY color attribute.
(print_color_indicator): Colorize file with capability.
* tests/ls/capability: Test for ls - colorize file with capability.
* tests/Makefile.am (TESTS): Add ls/capability.
* NEWS: Mention the change.
> diff --git a/tests/ls/capability b/tests/ls/capability
...
> +. $srcdir/test-lib.sh
> +require_root_
> +
> +which setcap >/dev/null || skip_test_ "setcap utility not found"
It it not portable to use "which". Do this instead:
[currently, it doesn't recognize --help, but someday it may]
(setcap --help) 2>&1 |grep 'usage: setcap' > /dev/null \
|| skip_test_ "setcap utility not found"
> +fail=0
> +
> +# Don't let a different umask perturb the results.
> +umask 22
> +
> +touch test
> +setcap cap_net_bind_service=ep test || \
> + skip_test_ "can't set capability"
When you leave "||" at the end of the line,
the trailing "\" is not needed. However, I find it
slightly more readable to put the operator at the beginning
of the next line, so keep the "\":
(also, once we know we have setcap, any failure is
worth more than a mere "skip", so use framework_failure):
setcap cap_net_bind_service=ep test \
|| framework_failure
> +LS_COLORS='ca=30;41' \
> + ls --color=always test > out || fail=1
> +printf "\033[0m\033[30;41mtest\033[0m\n\033[m" > out_ok
It's slightly better to factor out the "30;41", e.g.,
(also note that we must detect if/when printf fails; otherwise,
the test could mistakenly pass if ls were somehow to output nothing
in spite of a 0 exit status, and the printf failed e.g., due to disk
full)
code='30;41'
LS_COLORS="ca=$code" \
ls --color=always test > out || fail=1
printf "\033[0m\033[${code}mtest\033[0m\n\033[m" > out_ok || fail=1
> +diff out out_ok || fail=1
Use the "compare" functions, not "diff".
compare out out_ok || fail=1
> +rm -f test
There is no need to remove temporary files, so please don't.
test-lib.sh arranges for each test to be run in its own directory,
and then removes that directory upon completion.
> +(exit $fail); exit $fail