Re: Atomic install(1) by default (was: Re: groff breaks make -j N buildworld)
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.c2000/10/08 09:17:56 1.40
+++ xinstall.c2001/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.251Simferopol, 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
Re: Atomic install(1) by default (was: Re: groff breaks make -j N buildworld)
Hello Bruce! Forget all of this. I started incorporating OpenBSD fixes to install(1). They seem to cover all the cases you have mentioned. I will send a CFR when I finish. They used your revision 1.4 as the base, and implemented all of the todos, and even more. Thanks, -- Ruslan Ermilov Oracle Developer/DBA, [EMAIL PROTECTED] Sunbay Software AG, [EMAIL PROTECTED] FreeBSD committer, +380.652.512.251Simferopol, 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
Atomic install(1) by default (was: Re: groff breaks make -j N buildworld)
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:
[...]
IIRC, it is assumed that "make -jX install (where X 1)" _doesn't_ work.
I've heard why, but I've forgotten :-)
Right. One case where it doesn't work is installing /bin/sh with the
default install flags. /bin/sh gets clobbered, so anything that attempts
to use it concurrently doesn't work. In particular, a concurrent
sub-make may fail. This problem is avoided for some very important
install targets like ld.so by adding -C to INSTALLFLAGS to give an
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...
--
Ruslan Ermilov Oracle Developer/DBA,
[EMAIL PROTECTED] Sunbay Software AG,
[EMAIL PROTECTED] FreeBSD committer,
+380.652.512.251Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
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.
*/
#include sys/param.h
@@ -347,7 +342,7 @@ install(from_name, to_name, fset, flags)
devnull = 1;
}
- if (docompare) {
+ if (docopy) {
old_to_name = to_name;
/*
* Make a new temporary file in the same file system
@@ -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)) {
different:
if (debug != 0)
