Created attachment 169042 [details] Patch which fixes the issue On uninstall the nginx port removes the link %D/www/nginx if it exists. It expects that it created it but that might not be the case. The uninstall should check to see if it created the link i.e. it points where it would if it created it, and only then remove it. I'm thinking something like the attached patch.
Maintainer timeout, back to pool. Personally, I don't understand why is /var/www/nginx even used like that, if it symlinks to nginx-dist by default. The default config maps "localhost" docroot to /var/www/nginx, which is then a symlink to nginx-dist, which contains the warning not to touch or change anything. So what's the point? If you ask me, I'd use only nginx-dist, as a default "localhost" docroot with that warning kept, and with the "Welcome to nginx" default index.html. The users should be advised not to use that dir for any purpose, but to change the docroot to whatever else, unless they're okay with default index being present for default "localhost" docroot. They're free to copy over whatever default file from nginx-dist and manage their own docroots. Flagging with needs-patch too, as I believe above to be the best solution. Will also need one for www/nginx-devel.
@Steven, probably want someone with pkg-plist magic knowledge to review this simple change (portmgr? someone with pkgng chops? bapt? (cc'd) Once reviewed/approved, feel free to assign yourself to commit If the quarterly branch port is affected, this should also be MFH'd.
Comment on attachment 169042 [details] Patch which fixes the issue Explicitly record (implicit) maintainer approval (timeout)
Your patch is ok, but there are plenty of other issues in that plist :) for example: %%WWWDATA%%@exec mkdir -p -m 755 %D/www/nginx-dist should be @dir(,,555) www/nginx-dist %%WWWDATA%%@exec if [ ! -d %D/www/nginx/ ] ; then ln -fs %D/www/nginx-dist %D/www/nginx; fi What happens for for any reason a user using nginx decided that %D/www/nginx should be a file? it gets nuked :( during installation/upgrades So this test should imho test for absence of any file/directory and only in that case makes a symlink %%WWWDATA%%@exec chmod a-w %D/www/nginx-dist This is totally useless given the @dir is done (note the 555 and not the 755 on the @dir If I manage to find more time to properly test I will commit this Week End along with your patch
@Baptiste Thanks for the quick feedback & review
I meant LOCALBASE/www/nginx, sorry for the confusion, my mind was elsewhere...
@Vlad, as per our IRC conversation, did you want to attach a patch removing the unnecessary symlink, thereby precluding the need to run fancy comparison checks to determine whether it points to the original place or not?
Hi, I close this here since there is no feedback since the latest version. Please feel free to reopen if the problem still exist.