On Tue 05 Apr 2005, Frank Küster wrote:
> David Schmitt <[EMAIL PROTECTED]> wrote:
> 
> I am looking into this, trying to provide a patch - and NMU if Paul
> doesn't show up, I really think wwwoffle should go back into sarge.

I'm a bit busy with all sorts of things (e.g. new upstream rsync this
weekend)... I hadn't really delved into this report yet because it
doesn't go wrong for me, actually...

> 
> > The "overwwrites local config" part is way harder. Looking at the postinst
> > gives me the creeps. Just a few quotes which I think would need fixing:

You have no idea how much legacy needs to be taken into account...
You need to read up on all archived bug reports for all sorts of things
that also need to be reckoned with.


> This will create a symlink when it is missing, but not remove it if it
> point to a nonexistent target - the target might be only temporarily

If it's temporarily missing, it will temporarily not work, and people
will file bug reports -- trust me.

> missing.  I would not take "preserve user modifications" too literally
> here; in fact I doubt that these files are really configuration files -
> who would want to configure a 1x1 pixel png file?

Many people.
In fact, I once replaced it with a debian banner, and it took a while
before people realized it was a replacement image, and not in fact a
proliferation of debian propaganda.


> > The following code is AFAICS not conditional upon first installation, thus
> > overriding a local admin who has intentionally removed the link.
> >
> > Line 257:
> > |    perl -i.bak -pe 's/^(\# WWWOFFLE - World Wide Web Offline Explorer - 
> > Version).*/$1'" $config_version/" $CONFIG
> > |    if cmp -s $CONFIG $CONFIG.bak; then mv -f $CONFIG.bak $CONFIG; fi
> >
> > If the perl doesn't modify the config, this will kill the symlink:

Will it? AFAIK perl -i.bak will MOVE the original file to .bak; moving
it back will hence preserve the symlink.
Replacing the wwwoffle.conf with a symlink would seem pretty silly
anyway, and I wonder as to what extent such actions need to be
supported. Would you expect things to remain working if you replaced
/etc/passwd with a symlink?

> 
> > Line 354:
> > | if [ ! -f $OPTIONS ]; then
> > |    cp -p /usr/share/wwwoffle/default/wwwoffle.options $OPTIONS
> > | fi
> >
> > This too is not special cased to the not-upgrading case.

So what?

> > Line 368 and 372:
> > |         perl -i -e 'while (<>) { next if /htdig/; print; }' $OPTIONS
> >
> > This skips _all_ lines containing 'htdig' including comments

Where does it say it's safe to use comments in that file? See man
wwwoffle.options

> So it should be /^\s*htdig/, right?  I'd prefer to comment it out, this

Leading spaces aren't supported. Would you expect leading spaces to work
in /etc/passwd ...

> > Line 402:
> > |    if [ "$FREQ" != off ]; then
> > |        # convert to digits only
> > |        FREQ=$( expr "$FREQ" : '\([0-9]*\)' )
> >
> > In the .config, there is already complex code to clean this value and 
> > (without obvious cause) to cap it to 30 minutes. Also this would silently 
> > eat input errors.

The fact that you don't see the cause for the capping to 30 minutes
means that you don't understand how it works. If you don't understand
how it works, pleasepleaseplease don't touch the code!

> > Line 397, 412, 423:
> > |CRONTAB=/etc/cron.d/wwwoffle
> > [..]
> > |        if [ -s $CRONTAB ]; then
> >
> > This too is not special cased to the not-upgrading case and fails if 
> > $CRONTAB is a symlink.

See above comment about symlinks.

> > Line 448:
> > | # now duplicate directory structure of /usr/share/wwwoffle/html to 
> > /etc/wwwoffle/html
> >
> > Followed by approximatly 20 lines of shell code which probably amount to a 
> > 'cp -s' which 
> > I _think_ could be replaced by a 'ln -s /usr/share/wwwoffle/html 
> > /etc/wwwoffle/html' 
> > iff the package is not upgrading and there is no /etc/wwwoffle/html.
> 
> I am not sure that I understand the purpose of the code, but it seems to
> me that 
> 
> - the directories in /etc/wwwoffle/html should be shipped in the deb
> 
> - all those symlinks are in fact not configuration "files".  Of course
>   it alters the behaviour of the package if RefreshFormError.html points
>   to something else than the file shipped in the deb, but this is not a
>   usual configuration task - it's rather something that one would
>   normally achieve by patching the package.

It was requested that it was done this way, so that individual files
could be replaced at will. It works, so why mess with it? What has this
to do with this bug report?!


> I do not think it is something for an NMU to clean this up, which would
> amount to a very big change.  And I think if the maintainer script do

Exactly

> not respect local changes for something that shouldn't be a
> configuration file at all, it is not release-critical.
> 
> > Line 516, ff:
> > |    rm -f nohup.out
> > |    nohup chown -R proxy:proxy /var/cache/wwwoffle/. /var/lib/wwwoffle/. 
> > >/dev/null 2>&1 &
> > |    sleep 1
> > |    rm -f nohup.out
> >
> > This deletes nohup.out in the current directory, which could be not from 
> > wwwoffle, which could also be created in $HOME, but is not created by nohup 
> > anyway because of the redirection.

You missed the "cd /var/cache/wwwoffle/temp" 1 line higher...

> I'll just remove the nohup.out removals.

Why? Again, what has this to do with this bug report?



Paul Slootman


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to