Hi,

On 2/8/2010 8:03 AM, Ola Lundqvist wrote:
>> First, postrm does not check for the possible location of the init.cfg file 
>> in /var/lib/ntop/ => fixed
> 
> True and intentional. With the debian package /etc/ntop/init.cfg is
> the place where it should be.

However, after a new installation there is no (default) one there.

>> Second, there is no ntop/createuser entry in debconf, as none is added in 
>> the config file => check removed
> 
> True. But it has been so in the past. It is still there to make
> transitions possible. However it has not been there for quite some
> time, so it could potentially be removed.
>
> But the way you propose would cause users to be removed always which
> is not good.
>>  if [ "$1" = "purge" ] ; then

The whole case /is/ about ntop being purged...

>>
>>    # source debconf library
>> -if [ -f  /usr/share/debconf/confmodule ] ; then
>> +  if [ -f /usr/share/debconf/confmodule ]; then
>>      . /usr/share/debconf/confmodule
>> -fi
>> +  fi
>>
>> -  INIT="/etc/ntop/init.cfg"
>> -  if [ -f $INIT ] ; then
>> +  for f in /var/lib/ntop/init.cfg /etc/ntop/init.cfg; do
>> +    if [ -f $f ]; then
>> +      INIT=$f
>> +    fi
>> +  done
>> +  if [ -n "$INIT" ]; then
>>      # parse config file for user
>> -if [ -f "$INIT" ] ; then
>>      . $INIT
>> -fi
> 
> What happens if there are no /var/lib/ntop/init.cfg /etc/ntop/init.cfg
> files? Then the installation would break. I do not like this one.

The user would not be removed in that case.
That is no different from what would have happened before when there was
no /etc/ntop/init.cfg (which there was not on a new install).

Also, as a sidenote, in the current postrm the check is done twice:

  INIT="/etc/ntop/init.cfg"
  if [ -f $INIT ] ; then
    # parse config file for user
if [ -f "$INIT" ] ; then
    . $INIT
fi

I fail to see why.

> The inclusion of /var/lib/ntop/init.cfg can be argued but the
> if [ -f "$INIT" ] ; then
> line should really be there!

But it is in the form of "if [ -f $f ]; then INIT=$f" combined with "if
[ -n "$INIT" ]; then" which is in the end the same as "if [ -f "$INIT"
]; then..."

>>      # remove user
>>      if grep -q ^$USER: /etc/passwd; then
>> -      db_get ntop/createuser
>> -      CREATEUSER=$RET
>> -      if [ "$CREATEUSER" = "true" ]; then
>> -        deluser $USER;
>> -      fi
>> +      deluser $USER
>>      fi
>>    fi
> 
> This is old code that should be removed by now. It is a transition
> for old systems where this was available. So if the user was created
> it should also be removed.

Shouldn't it be removed anyhow on purge? But considering your argument
we had maybe better say:

db_get ntop/createuser
CREATUSER=$RET
db_get ntop/user
DEBCONFUSER=$RET
if [ "$CREATEUSER" = "true" -o "$DEBCONFUSER" = "$USER" ]; then
 deluser $USER
...

> The space between the ] and the ; character is actually needed
> sometimes. Maybe not in Debian but I keep that rule so I know
> it always work.

Ok. Magic ;-)

JM



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to