Bug#332264: tex-common: permission-handling of ls-R files is one-way

[Frank Küster <frank@debian.org>]

Norbert Preining <preining@logic.at> wrote:

> config.in implementation proposal, please comment:

This looks good to me.  Some comments:

> #
> # first we collect those ls-R files which are group writeable in truegwrite
> # and those which are not group writeable in falsegwrite
> # furthermore we save the respective group and write permission in ${i}group
> truegwrite=""
> falsegwrite=""

I don't like these names - truegwrite evokes "Trug" in my head.  Why not
gwritetrue and gwritefalse (if only it sounds more like
\newif\if@gwrite\gwritetrue...) 

> for i in var cache main ; do
>     ${i}group=""

Sometimes I'm dreaming about using Perl for maintainer scripts.  This is
one occation (my %group \dots).  Furthermore:

frank@alhambra:~$ i=var
frank@alhambra:~$ ${i}group=""
bash: vargroup=: command not found
frank@alhambra:~$ 

I think you wanted to use array, but they must be indexed with integers,
and that's ugly (and hell is loose when one changes the order of the
directory names...) and the reason why I want hashes.  But you can
simulate hashes like this:

i=var
eval group_$i=users
eval echo \$group_$i

>     lsr=`select_lsrfile $i`
>     if [ -r $lsr ] ; then
>         ${i}group=`echo $lsr | awk '{print$4}'`
>         if ls -l $lsr | grep -q ^.....w ; then

I don't understand the syntax in these lines (the purpose seems clear).
>From the lines with "if" it seems that lsr contains a filename, but from
the assignment to ${i}group it seems it is the output of "ls -l $file".

> db_get tex-common/managedlsr && SELECTED_LSR="$RET"

  db_get tex-common/managedlsr && SELECTED_LSR="$RET"  || true

> #
> # we do not care for user permissions in this setting!
> #
>
> Please note that we do not care for user permissions ATM, this way.

Hm, you mean permissions for "others", right?  The owner will be root.
In the default new install the files will end up readable, but not
writeable by others, that's what we want.  If someone previously had set
write permissions for "others" on ls-R files, this will not be changed
upon an upgrade.  I think we can keep it like this, but should mention
it in NEWS.Debian or README.Debian.

> postinst.in implementation proposal, please comment:
>
>     db_get tex-common/managedlsr || true
>     if [ -n "$RET" ] ; then
>       falsegwritefiles=""
>       MANAGEDLSR="$RET"
>       for i in var cache main ; do
>           if echo $MANAGEDLSR | grep -q $i ; then
>               :
>           else
>               $falsegwritefiles="$falsegwritefiles $i"
>           fi

There's a perl variable in it, and why not revert it?

if ! echo $MANAGEDLSR | grep -q $i ; then
    falsegwritefiles="$falsegwritefiles $i"
fi

Or just 

echo $MANAGEDLSR | grep -q $i || falsegwritefiles="$falsegwritefiles $i"

>     else
>       echo "Fixing permissions of ls-R files ..."
>       chmod -v 644 $LSRS 2>/dev/null | fgrep changed || true
>     fi

why the fgrep?

> Here it is not clear what we should do at the end, when NO group file is
> selected, ie the last else clause. Probably we shouldn't touch anything
> at all!

Yes, I think we should change nothing (just as your code does, doesn't it?)

Regards, Frank

-- 
Frank Küster
Inst. f. Biochemie der Univ. Zürich
Debian Developer