Re: PATCH: Re: zrecompile

Re: PATCH: Re: zrecompile

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Apr 4,  4:07pm, Sven Wischnowsky wrote:
> } Subject: PATCH: Re: zrecompile
> }
> } Bart Schaefer wrote:
> } 
> } > } And another thing: the zwc files till use $ZSH_VERSION in the header
> } > } to test for compatibility -- somehow I didn't like to add an additional 
> } > } version number scheme for them, but it would be better, I think (the
> } > } format will certainly change less often than $ZSH_VERSION).
> } > 
> } > Yea, but it'll be much less recognizable in the -t output.  I think the
> } > $ZSH_VERSION test is fine.
> 
> Having thought about it a bit longer now, I have a suggestion: Put *both*
> a .zwc version number *and* $ZSH_VERSION into the header.  When the .zwc
> version doesn't match, display the $ZSH_VERSION string in the error output.
> That completely hides the .zwc version from anyone who would be confused
> by it, while avoiding the need to recompile when the shell is upgraded.

Wouldn't that be rather confusing? I mean: for some reason we have to
change the zwc-version, but the ZSH_VERSION is still the same. The shell
can't load an old zwc file and says: `wrong version (...)' where `...'
is the same as $ZSH_VERSION.

This would only affect people upgrading their shell between releases,
though... hm.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: zrecompile

[Sven Wischnowsky]

I wrote:

> Bart Schaefer wrote:
> 
> > ...
> >
> > Having thought about it a bit longer now, I have a suggestion: Put *both*
> > a .zwc version number *and* $ZSH_VERSION into the header.  When the .zwc
> > version doesn't match, display the $ZSH_VERSION string in the error output.
> > That completely hides the .zwc version from anyone who would be confused
> > by it, while avoiding the need to recompile when the shell is upgraded.
> 
> Wouldn't that be rather confusing? I mean: for some reason we have to
> change the zwc-version, but the ZSH_VERSION is still the same. The shell
> can't load an old zwc file and says: `wrong version (...)' where `...'
> is the same as $ZSH_VERSION.
> 
> This would only affect people upgrading their shell between releases,
> though... hm.

Because of that last one I could convince myself to make this
change. So, zwc files now use their magic number as the
zwc-file-version to compare. The zsh version is used to report version 
mismatches.

Bye
 Sven

Index: Src/parse.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
retrieving revision 1.3
diff -u -r1.3 parse.c
--- Src/parse.c	2000/04/04 14:08:56	1.3
+++ Src/parse.c	2000/04/05 09:05:03
@@ -2197,7 +2197,10 @@
  * the version string in a field of 40 characters and the descriptions
  * for the functions in the dump file.
  *
- * NOTE: this layout has to be kept; everything after it may be changed.
+ * NOTES:
+ *  - This layout has to be kept; everything after it may be changed.
+ *  - When incompatible changes are made, the FD_MAGIC and FD_OMAGIC
+ *    numbers have to be changed.
  *
  * Each description consists of a struct fdhead followed by the name,
  * aligned to sizeof(wordcode) (i.e. 4 bytes).
@@ -2358,11 +2361,17 @@
     }
     if (read(fd, buf, (FD_PRELEN + 1) * sizeof(wordcode)) !=
 	((FD_PRELEN + 1) * sizeof(wordcode)) ||
-	(fdmagic(buf) != FD_MAGIC && fdmagic(buf) != FD_OMAGIC) ||
-	(v = strcmp(ZSH_VERSION, fdversion(buf)))) {
-	if (err)
-	    zwarnnam(nam, (v ? "invalid zwc file, wrong version: %s" :
-			   "invalid zwc file: %s") , name, 0);
+	(v = (fdmagic(buf) != FD_MAGIC && fdmagic(buf) != FD_OMAGIC))) {
+	if (err) {
+	    if (v) {
+		char msg[80];
+
+		sprintf(msg, "zwc file has wrong version (zsh-%s): %%s",
+			fdversion(buf));
+		zwarnnam(nam, msg , name, 0);
+	    } else
+		zwarnnam(nam, "invalid zwc file: %s" , name, 0);
+	}
 	close(fd);
 	return NULL;
     } else {

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

PATCH: Re: zrecompile

[Sven Wischnowsky]

Bart Schaefer wrote:

> } And another thing: the zwc files till use $ZSH_VERSION in the header
> } to test for compatibility -- somehow I didn't like to add an additional 
> } version number scheme for them, but it would be better, I think (the
> } format will certainly change less often than $ZSH_VERSION).
> 
> Yea, but it'll be much less recognizable in the -t output.  I think the
> $ZSH_VERSION test is fine.
> 
> On the other hand, that should be put in some immutable part of the file
> header so that *any* version of zsh can be guaranteed to be able to read
> it back -- to rephrase, we set the requirement NOW that changes to the
> format of the header must only come *after* the version string (or some
> equivalent requirement), so that it's always possible to display the
> version mismatch (or at least know that was the reason for the error).

For the change that lead to all this I had to change the magic
numbers, because there was no version change at the time, btw...

But still, this patch makes the error messages a bit more informative
and adds a comment saying that the header should not be changed.

Bye
 Sven

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.3
diff -u -r1.3 builtin.c
--- Src/builtin.c	2000/04/04 12:02:04	1.3
+++ Src/builtin.c	2000/04/04 14:03:05
@@ -2118,12 +2118,9 @@
 
     /* Take the arguments literally -- do not glob */
     for (; *argv; argv++) {
-	if (ops['w']) {
-	    if (dump_autoload(*argv, on, ops, func)) {
-		zwarnnam(name, "invalid wordcode file: %s", *argv, 0);
-		returnval = 1;
-	    }
-	} else if ((shf = (Shfunc) shfunctab->getnode(shfunctab, *argv))) {
+	if (ops['w'])
+	    returnval = dump_autoload(name, *argv, on, ops, func);
+	else if ((shf = (Shfunc) shfunctab->getnode(shfunctab, *argv))) {
 	    /* if any flag was given */
 	    if (on|off) {
 		/* turn on/off the given flags */
Index: Src/parse.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
retrieving revision 1.2
diff -u -r1.2 parse.c
--- Src/parse.c	2000/04/01 20:49:48	1.2
+++ Src/parse.c	2000/04/04 14:03:06
@@ -2196,6 +2196,9 @@
  * file should be mapped or read and if this header is the `other' one),
  * the version string in a field of 40 characters and the descriptions
  * for the functions in the dump file.
+ *
+ * NOTE: this layout has to be kept; everything after it may be changed.
+ *
  * Each description consists of a struct fdhead followed by the name,
  * aligned to sizeof(wordcode) (i.e. 4 bytes).
  */
@@ -2302,11 +2305,10 @@
 	    zwarnnam(nam, "too few arguments", NULL, 0);
 	    return 1;
 	}
-	if (!(f = load_dump_header(*args)) &&
-	    !(f = load_dump_header(dyncat(*args, FD_EXT)))) {
-	    zwarnnam(nam, "invalid dump file: %s", *args, 0);
-	    return 1;
-	}
+	if (!(f = load_dump_header(nam, (strsfx(FD_EXT, *args) ? *args :
+					 dyncat(*args, FD_EXT)), 1)))
+		return 1;
+
 	if (args[1]) {
 	    for (args++; *args; args++)
 		if (!dump_find_func(f, *args))
@@ -2315,7 +2317,7 @@
 	} else {
 	    FDHead h, e = (FDHead) (f + fdheaderlen(f));
 
-	    printf("function dump file (%s) for zsh-%s\n",
+	    printf("zwc file (%s) for zsh-%s\n",
 		   ((fdflags(f) & FDF_MAP) ? "mapped" : "read"), fdversion(f));
 	    for (h = firstfdhead(f); h < e; h = nextfdhead(h))
 		printf("%s\n", fdname(h));
@@ -2344,18 +2346,23 @@
 
 /**/
 static Wordcode
-load_dump_header(char *name)
+load_dump_header(char *nam, char *name, int err)
 {
-    int fd;
+    int fd, v = 0;
     wordcode buf[FD_PRELEN + 1];
 
-    if ((fd = open(name, O_RDONLY)) < 0)
+    if ((fd = open(name, O_RDONLY)) < 0) {
+	if (err)
+	    zwarnnam(nam, "can't open zwc file: %s", name, 0);
 	return NULL;
-
+    }
     if (read(fd, buf, (FD_PRELEN + 1) * sizeof(wordcode)) !=
 	((FD_PRELEN + 1) * sizeof(wordcode)) ||
 	(fdmagic(buf) != FD_MAGIC && fdmagic(buf) != FD_OMAGIC) ||
-	strcmp(ZSH_VERSION, fdversion(buf))) {
+	(v = strcmp(ZSH_VERSION, fdversion(buf)))) {
+	if (err)
+	    zwarnnam(nam, (v ? "invalid zwc file, wrong version: %s" :
+			   "invalid zwc file: %s") , name, 0);
 	close(fd);
 	return NULL;
     } else {
@@ -2372,6 +2379,7 @@
 	    if (lseek(fd, o, 0) == -1 ||
 		read(fd, buf, (FD_PRELEN + 1) * sizeof(wordcode)) !=
 		((FD_PRELEN + 1) * sizeof(wordcode))) {
+		zwarnnam(nam, "invalid zwc file: %s" , name, 0);
 		close(fd);
 		return NULL;
 	    }
@@ -2384,6 +2392,7 @@
 		 len - ((FD_PRELEN + 1) * sizeof(wordcode))) !=
 	    len - ((FD_PRELEN + 1) * sizeof(wordcode))) {
 	    close(fd);
+	    zwarnnam(nam, "invalid zwc file: %s" , name, 0);
 	    return NULL;
 	}
 	close(fd);
@@ -2483,7 +2492,7 @@
 	dump = dyncat(dump, FD_EXT);
 
     if ((dfd = open(dump, O_WRONLY|O_CREAT, 0600)) < 0) {
-	zwarnnam(nam, "can't write dump file: %s", dump, 0);
+	zwarnnam(nam, "can't write zwc file: %s", dump, 0);
 	return 1;
     }
     progs = newlinklist();
@@ -2614,7 +2623,7 @@
 	dump = dyncat(dump, FD_EXT);
 
     if ((dfd = open(dump, O_WRONLY|O_CREAT, 0600)) < 0) {
-	zwarnnam(nam, "can't write dump file: %s", dump, 0);
+	zwarnnam(nam, "can't write zwc file: %s", dump, 0);
 	return 1;
     }
     progs = newlinklist();
@@ -2876,7 +2885,7 @@
 
 #endif
 
-    if (!f && (isrec || !(d = load_dump_header(file))))
+    if (!f && (isrec || !(d = load_dump_header(NULL, file, 0))))
 	return NULL;
 
     if ((h = dump_find_func(d, name))) {
@@ -3014,7 +3023,7 @@
 
 /**/
 int
-dump_autoload(char *file, int on, char *ops, int func)
+dump_autoload(char *nam, char *file, int on, char *ops, int func)
 {
     Wordcode h;
     FDHead n, e;
@@ -3024,7 +3033,7 @@
     if (!strsfx(FD_EXT, file))
 	file = dyncat(file, FD_EXT);
 
-    if (!(h = load_dump_header(file)))
+    if (!(h = load_dump_header(nam, file, 1)))
 	return 1;
 
     for (n = firstfdhead(h), e = (FDHead) (h + fdheaderlen(h)); n < e;

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: zrecompile

[Bart Schaefer]

On Apr 4,  4:07pm, Sven Wischnowsky wrote:
} Subject: PATCH: Re: zrecompile
}
} Bart Schaefer wrote:
} 
} > } And another thing: the zwc files till use $ZSH_VERSION in the header
} > } to test for compatibility -- somehow I didn't like to add an additional 
} > } version number scheme for them, but it would be better, I think (the
} > } format will certainly change less often than $ZSH_VERSION).
} > 
} > Yea, but it'll be much less recognizable in the -t output.  I think the
} > $ZSH_VERSION test is fine.

Having thought about it a bit longer now, I have a suggestion: Put *both*
a .zwc version number *and* $ZSH_VERSION into the header.  When the .zwc
version doesn't match, display the $ZSH_VERSION string in the error output.
That completely hides the .zwc version from anyone who would be confused
by it, while avoiding the need to recompile when the shell is upgraded.

I know, I know, I should have thought of this sooner ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com