On Sat, Feb 09, 2019 at 05:23:09PM -0500, Ted Unangst wrote:
> Marc Espie wrote:
> > hey, your commit to install(1) broke something.
> > 
> > Specifically lang/go-boostrap now produces a broken package which can't
> > be used to build go.
> > 
> > All the go/bootstrap/pkg/tool/openbsd_amd64/*
> > have lost their x bit
> > 
> > Relevant fake install information, it definitely looks like the last line
> > is now a no-op.
> 
> > # These get installed via `find' however we need them to be executable
> > /pobj/go-bootstrap-1.4.20171003/bin/install -d -m 755 
> > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64
> > /pobj/go-bootstrap-1.4.20171003/bin/install -c  -m 755 -p 
> > /pobj/go-bootstrap-1.4.20171003/go/pkg/tool//openbsd_amd64/* 
> > /pobj/go-bootstrap-1.4.20171003/fake-amd64/usr/local/go/bootstrap/pkg/tool//openbsd_amd64
> 
> Yes. This is a weird way to invoke chmod, but that's what it wants.
> 
> I think this was a long standing bug in install? If the files match, we need
> to continue on with to_fd == existing file so that metadata updates work.
> 
> 
> Index: xinstall.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 xinstall.c
> --- xinstall.c        8 Feb 2019 12:53:44 -0000       1.68
> +++ xinstall.c        9 Feb 2019 22:21:03 -0000
> @@ -313,8 +313,12 @@ install(char *from_name, char *to_name, 
>                               (void)unlink(tempfile);
>                       }
>               }
> -             (void)close(to_fd);
> -             to_fd = temp_fd;
> +             if (!files_match) {
> +                     (void)close(to_fd);
> +                     to_fd = temp_fd;
> +             } else {
> +                     close(temp_fd);
> +             }
>       }
>  
>       /*
I'm afraid this needs to be slightly more complex, probably to put the
remaining code in its own function, or something.

Specifically, the error paths for the fchown and fchmod  will be all bogus
in that case, referring to a tempfile  that's already been removed, and
is not even the target...

Reply via email to