Re: [PATCH] ls - colorize files with capabilities

[Kamil Dudka]

I hope I didn't forget  anything this time. New version of patch is
in attachment.

Kamil


On Tuesday 22 July 2008 15:25:07 you wrote:
> 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

coreutils-libcap.patch
Description: Text Data