On Fri, Apr 20, 2001 at 06:39:30AM +1000, Bruce Evans wrote:
> On Thu, 19 Apr 2001, Ruslan Ermilov wrote:
>
> > On Thu, Apr 19, 2001 at 05:53:53PM +0300, Ruslan Ermilov wrote:
> > > On Thu, Apr 19, 2001 at 11:12:24PM +1000, Bruce Evans wrote:
> > > > atomic installation. Atomic installation (but not -C) should be the
> > > > default.
> > > >
> > > This one seems like an easy task, and this is suspicious... How about
> > > the attached patch? I have tested it lightly, and haven't found any
> > > problems. Will the `make -j32 installworld' of -CURRENT be enough
> > > test to commit this and remove -B from Makefile.inc1?
> > >
> > Damn, forgot to attach the patch. Here it goes...
>
> This seems to be simple enough. A bit too simple :-). The old
> behaviour of deleting the target first is still needed at least
> optionally to handle cases where there is no space for a copy.
>
But then all atomicy goes awry. Should I introduce -r instead (borrowed
from NetBSD)?
: -r Install to a temporary file and then rename the file to its final
: destination name. This can be used for precious files, to avoid
: truncation of the original when error conditions (filesystem full
: etc.) occur.
> Cleaning up the temporary files after a signal is more necessary if -C
> is the default.
>
I didn't mean to volunteer to fix all of the BUGS section :-)
> Index: xinstall.c
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/xinstall/xinstall.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 xinstall.c
> --- xinstall.c 2000/10/08 09:17:56 1.40
> +++ xinstall.c 2001/04/19 14:38:41
> @@ -53,11 +53,6 @@ static const char rcsid[] =
> * attribute changes and don't clear the dump flag. (I think inode
> * ctimes are not updated for null attribute changes, but this is a
> * bug.)
> - * o independent of -C, if a copy must be made, then copy to a tmpfile,
> - * set all attributes except the immutable flags, then rename, then
> - * set the immutable flags. It's annoying that the immutable flags
> - * defeat the atomicicity of rename - it seems that there must be
> - * a window where the target is not immutable.
> */
>
> The comment still applies to the -C case. We now always make a copy, but
> for -C this is just a waste of time if the comparison succeeds.
>
I fail to understand how this still applies. Could you please exaplain
it in a bit more details?
Also, for -C case, copy is not exactly the waste of time, and this is you
who added the comment below in revision 1.4:
/*
* Unfortunately, because we strip the installed file and not the
* original one, it is impossible to do the comparison without
* first laboriously copying things over and then comparing.
* It may be possible to better optimize the !dostrip case, however.
* For further study.
*/
OTOH, I agree that it is probably makes sense to disable -C and -s
combo:
/*-
* Todo:
* o for -C, compare original files except in -s case.
And have always compare with original files. What do you think?
> ...
> @@ -409,7 +404,7 @@ install(from_name, to_name, fset, flags)
> * It may be possible to better optimize the !dostrip case, however.
> * For further study.
> */
> - if (docompare) {
> + if (docopy) {
> struct stat old_sb, new_sb, timestamp_sb;
> int old_fd;
> struct utimbuf utb;
> @@ -423,7 +418,7 @@ install(from_name, to_name, fset, flags)
> if (old_sb.st_flags & NOCHANGEBITS)
> (void)fchflags(old_fd, old_sb.st_flags & ~NOCHANGEBITS);
> fstat(to_fd, &new_sb);
> - if (compare(old_fd, old_to_name, to_fd, to_name, &old_sb,
> + if (!docompare || compare(old_fd, old_to_name, to_fd, to_name, &old_sb,
> &new_sb)) {
>
> Line too long.
>
Argh, I recently added allscreens_flags="VGA_90x25" to /etc/rc.conf :-)
Cheers,
--
Ruslan Ermilov Oracle Developer/DBA,
[EMAIL PROTECTED] Sunbay Software AG,
[EMAIL PROTECTED] FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
