(Please note that I didn't read mail while I wrote some to this bug, and now I'm starting to answer from the first mail I got. No offense intended, but I might not yet know some things you said in later mails).
Paul Slootman <[EMAIL PROTECTED]> wrote: > 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... It is easily reproducibly, and it is release critical. We should really try to fix this and get wwwoffle back into sarge (you didn't miss that it was removed, did you?). >> > 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. I see what tremendous amount of work, and of keeping track of old stuff is in there. Still there's also some issues that _need_ fixing.n >> 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. There is always a tight balance between allowing an admin to do unusual things, and preventing him from doing stupid things he doesn't understand. In the case of these images, I really don't care; as I said elsewhere, I don't think they are really configuration files. I'm happy with taking that hunk out. >> 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. Where do these images show up? If you think that many people do customize them, then this is an argument for the postinst script to really respect what they do. But if they didn't notice it, as you said, it rather seems to me as if they are not conffiles. I'm confused what you want to say, but let's just keep this as it is. >> > 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. If the files are equal (if is true), then the symlink, the bak file, will be moved back again. But if wwwoffle.conf has been changed, then it will be an ordinary file, and the symlink is the bak file which isn't used. > 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. I do it, and I do it with many files. Obviously others have noted this, too. > Would you expect things to remain working if you replaced > /etc/passwd with a symlink? Yes, as long as you don't show me anything in policy or in the relevant documentation that I shouldn'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? If the local admin has removed the file, the maintainer scripts have to respect this and keep it removed - just as dpkg does it. See the policy. >> > 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 The manpage only says that comments can be used to comment out ppp. Since it even doesn't say what is the comment sign, one should safely be able to assume that it has a standard behaviour. And this would include to be able to add textual comments. >> 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 ... I didn't know that. Well, then it's /^htdig/. By the way, /etc/passwd has a manpage that exactly describes its syntax, including separators etc. wwwoffle.options(5) is much less clear here. >> > 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! I don't think so. One line above this, FREQ is assigned from the debconf return value. And in wwwoffle.config this has been sanitized exactly the same way as is repeated here. Of course you are free to keep this in - this is not a bug, it's just superfluous AFAICT. >> > 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?! It will recreate the whole symlink farm if it has been deleted. Formally, this violates Policy 10.7.3. In my opinion, this is not release-critical, because - I don't really see how this should be needed to configure, and - I think maintainer scripts should not preserve local changes if they would just f*ck up the package. Therefore I think we should keep this as it is. >> > 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... On the one hand, this handling of nohup.out is completely useless, since this file will always be empty because of the redirections. On the other hand, if for some reason /var/cache/wwwoffle/temp is not writable, nohup will write to $HOME/nohup.out, which is not what you want. The most probably reason for not being able to write would be "no space left on device". Again, this is probably a bug, but by far not release-critical. If you insist on this behavior, well, please keep it. >> I'll just remove the nohup.out removals. > > Why? Again, what has this to do with this bug report? Nothing. But this bug has been open for 6 weeks, and there was no reaction at all from you, the maintainer. Therefore I assumed that I would have to do an NMU. Just today or yesterday it has been complained on -devel that when NMUs fix RC bugs, they often fail to otherwise care for the package. In a normal situation, I would have kept my proposals for an NMU as small as possible, and I assume David would have done the same. Here, however, it seemed reasonable to generally look over the maintainer scripts. By the way, it's fairly standard to talk about related issues in one mail to a bug report; if it turns out that there are separate bugs, the bug can be cloned and retitled for the other (in this case, less severe) issue. In the end let me say that I'm glad that you finally found time to look at this! Regards, Frank -- Frank Küster Inst. f. Biochemie der Univ. Zürich Debian Developer