On Tue 05 Apr 2005, David Schmitt wrote: > On Tuesday 05 April 2005 19:21, Paul Slootman wrote: > > On Tue 05 Apr 2005, Frank Küster wrote: > > > David Schmitt <[EMAIL PROTECTED]> wrote: > > > > 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. > > The code doesn't move the symlink back if it has changed the file.
That wasn't the comment. The comment was: > > > > If the perl doesn't modify the config, this will kill the symlink: That was clearly wrong. That indicates to me a bad understanding of the workings of the code. I don't like people touching code they don't understand well. > > 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? > > Yes. Well, I wouldn't like to try it. I also think any sane admin wouldn't. > > > > 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? > > http://www.debian.org/doc/debian-policy/ch-files.html#s10.7.3 says: > > "local changes must be preserved during a package upgrade" > > I was under the impression, that this includes the removal of said > configuration files. See e.g. > http://lists.debian.org/debian-user/2003/08/msg01816.html > > "Are they configuration files (to a first approximation, "files in > /etc")? If so, dpkg considers deleting the file as a valid > configuration option, and will preserve this across upgrades." That discusses conffiles, as managed by dpkg, not files that happen to be in /etc; this is a case of a "maintainer scripts maintained" file. I can't find any clear text that also here removal should be preserved. I find this a case of " If the existence of a file is required for the package to be sensibly configured it is the responsibility of the package maintainer to provide maintainer scripts which correctly create, update and maintain the file and remove it on purge." as mentioned in http://www.debian.org/doc/debian-policy/ch-files.html#s10.7.3 > Thus I believe creating $OPTIONS on upgrade does not preserve local changes > and therefore is RC. I disagree. > > > > > 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 > > Then the shipped default is incorrect: It's OK to use comments, there's just no guarantee that they'll stay intact :-) > > > > 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! > > Neither have I claimed that understand the code nor have I touched it. So why bring it up? > > > > 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? > > At least it's undocumented leading to confusion (as proven by my review und > Franks work to remedy the situation). At worst it is (as detailed above) a RC > bug not preserving users changes (removing one or more of the symlinks). As I say, I dispute that it's acceptable to replace such files by symlinks. However, will in this case (the directories in /etc/wwwoffle/html) a symlink be changed? > > > > 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... > > info coreutils nohup tells me: > > "If standard output is a terminal, it is redirected so that it is > appended to the file `nohup.out'; if that cannot be written to, it is > appended to the file `$HOME/nohup.out'." > > Thus $HOME/nohup.out can be created iff /var/cache/wwwoffle/temp/nohup.out is > not writeable but will not be removed by your script. Huh? We're running as root here. > > > I'll just remove the nohup.out removals. > > > > Why? > > Looking at it more closely I see that nohups output is already redirected > to /dev/null. This causes nohup not to create nohup.out anywhere. Therefore > iff there is one it is surely not the one created by this nohup call, > removing the rm calls is therefore the correct thing to do. It's in /var/cache/. Nothing in /var/cache is guaranteed to remain. Packages are required to cope with a complete removal of all their data from /var/cache. If in that specific directory there happens to be a nohup.out from something else, then I suggest that the creation there is the bug. This is a silly point and in NO WAY relevant to this bug. Paul Slootman -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
