Re: pkg_add -U on NFS broken?

[Alistair Crooks <agc%pkgsrc.org@localhost>]

On Thu, Aug 04, 2011 at 01:30:36PM +0200, Anthony Mallet wrote:
Content-Description: message body text
> On Wednesday, at 23:52, Anthony Mallet wrote:
> | # rm -rf ~tmp && mkdir ~/tmp
> | # pkg_add -K ~/tmp/var/db/pkg -p ~/tmp 
> /usr/pkgsrc/packages/All/digest-20080510.tgz
> | # pkg_add -U -K ~/tmp/var/db/pkg -p ~/tmp 
> /usr/pkgsrc/packages/All/digest-20080510.tgz
> | pkg_delete: Couldn't remove package directory in 
> `/home/tho/tmp/var/db/pkg/digest-20080510'
> | Also, the dir var/db/pkg/digest-20080510.xxxxxxx is never cleaned.
> | 
> | The problem apparently is that pkg_delete finds some stalled NFS entries
> | (.nfs*) in the var/db/pkg/digest-20080510 dir, so it does not delete the
> | directory. Is this due to pkg_add not correctly closing file descriptors 
> before
> | exec'ing pkg_delete? For instance, I really don't understand the logic in
> | check_already_installed() (add/perform.c:375) regarding the open() of 
> +CONTENTS
> | which is almost never closed (and never used as well...). Shouldn't this be
> | closed before running pkg_delete?
> 
> ktrace shows that the +CONTENTS file is open() by pkg_add, I believe this is 
> in
> check_already_installed(), add/perform.c:381. Then pkg_delete is run and when
> it comes to deleting the pkgdir entry, it finds that .nfs file and aborts.
> 
> I tried the attached patch which fix everything.
> 

> --- add/perform.c~    2011-08-03 20:40:01.000000000 +0200
> +++ add/perform.c     2011-08-04 09:15:15.000000000 +0200
> @@ -382,6 +382,7 @@
>       free(filename);
>       if (fd == -1)
>               return 1;
> +     close(fd);
>  
>       if (ReplaceSame) {
>               struct stat sb;
> @@ -411,7 +412,6 @@
>               warnx("package `%s' already recorded as installed",
>                     pkg->pkgname);
>       }
> -     close(fd);
>       return 0;
>  
>  }

Thanks for the analysis and fix.

I've committed it now, and bumped the version number to 20110805.

Best,
Alistair