Subject: Re: lib/9702: Makefile lossage for crypto-intl
To: Aidan Cully <aidan@kublai.com>
From: Simon Burge <simonb@netbsd.org>
List: tech-crypto
Date: 04/14/2000 13:51:12
Simon Burge wrote:

> Aidan Cully wrote:
> 
> > On Wed, Mar 29, 2000 at 09:25:29PM +1000, Simon Burge wrote:
> > > Lennart Augustsson wrote:
> > > 
> > > > >Number:         9702
> > > > >Category:       lib
> > > > >Synopsis:       Makefile lossage for crypto-intl
> > > > 
> > > > >Description:
> > > > 	There is a problem with the Makefiles for the crypto-intl stuff.
> > > > 	It remakes things too often.
> > > > >How-To-Repeat:
> > > > 		cd src
> > > > 		make build
> > > > 		make UPDATE=Y build
> > > > 	Watch how the second make recompiles a lot of the crypto libraries.
> > > > 	This can't be necessary and it's very annoying on slow machines.
> > > > >Fix:
> > > > 	Makefiles move mysteriously, meandering maliciously...
> > > 
> > > I _think_ from memory that this is because commands like compile_et and
> > > so on get rebuilt (because libc is updated) and they have dependancies
> > > on them so other files get rebuilt.  Do you get unnecessary recompiles
> > > if you "make UPDATE=y build" yet again?
> 
> I dug a bit deeper - it's asn1_compile that's the problem.  In my case
> after two complete builds on my Alpha:
> 
> 	cd /NetBSD/src/crypto-intl/lib/libasn1
> 	make
> 	touch /NetBSD/src/crypto-intl/lib/libasn1/asn1_compile/obj.alpha/asn1_compile
> 	make
> 
> nothing happens after the first make and the second make will rebuild
> libasn1 again.  Doing a diff on the old .x files and the new .x files
> reveals no differences.  It sounds like we should get asn1_compile to
> generate the new files into temporary files and only move them to the
> right name if the correct file is either different or missing, ala the
> cmphdr() function in usr.sbin/config/mkheaders.c.

The following works for me.  Before this goes in, I need to check if
any copyright notices would need to be added because of the use of
config(8)'s cmphdr() function, here called compare_and_move().  I guess
it could be rewritten from scratch to avoid any problems -

	system("cmp tmp real || mv tmp real");

is probably cheating :-)  Any calls on this, anyone?

Simon.
--
Index: gen.c
===================================================================
RCS file: /cvsroot/cryptosrc-intl/crypto-intl/dist/heimdal/lib/asn1/gen.c,v
retrieving revision 1.1.1.1
diff -p -u -r1.1.1.1 gen.c
--- gen.c	2000/01/16 01:52:56	1.1.1.1
+++ gen.c	2000/04/14 03:41:08
@@ -41,18 +41,24 @@ FILE *headerfile, *codefile, *logfile;
 
 static char *orig_filename;
 static char header[1024];
+static char theader[1024];
 static char headerbase[1024] = STEM;
 
+static void compare_and_move(const char *, const char *);
+
 void
 init_generate (char *filename, char *base)
 {
+    char *tfilename;
+
     orig_filename = filename;
     if(base)
 	strcpy(headerbase, base);
-    sprintf(header, "%s.h", headerbase);
-    headerfile = fopen (header, "w");
+    snprintf(header, sizeof(header), "%s.h", headerbase);
+    snprintf(theader, sizeof(theader), "tmp_%s.h", headerbase);
+    headerfile = fopen (theader, "w");
     if (headerfile == NULL)
-	err (1, "open %s", header);
+	err (1, "open %s", theader);
     fprintf (headerfile,
 	     "/* Generated from %s */\n"
 	     "/* Do not edit */\n\n",
@@ -85,9 +91,9 @@ init_generate (char *filename, char *bas
 #endif
 	     );
     fprintf (headerfile, "#endif\n\n");
-    logfile = fopen(STEM "_files", "w");
+    logfile = fopen("tmp_" STEM "_files", "w");
     if (logfile == NULL)
-	err (1, "open " STEM "_files");
+	err (1, "open tmp_" STEM "_files");
 }
 
 void
@@ -96,8 +102,10 @@ close_generate ()
     fprintf (headerfile, "#endif /* __%s_h__ */\n", headerbase);
 
     fclose (headerfile);
+    compare_and_move(theader, header);
     fprintf (logfile, "\n");
     fclose (logfile);
+    compare_and_move("tmp_" STEM "_files", STEM "_files");
 }
 
 void
@@ -318,14 +326,14 @@ generate_type_header (const Symbol *s)
 void
 generate_type (const Symbol *s)
 {
-    char *filename;
+    char *filename, *tfilename;
 
     asprintf (&filename, "%s_%s.x", STEM, s->gen_name);
-    codefile = fopen (filename, "w");
+    asprintf (&tfilename, "tmp_%s_%s.x", STEM, s->gen_name);
+    codefile = fopen (tfilename, "w");
     if (codefile == NULL)
-	err (1, "fopen %s", filename);
+	err (1, "fopen %s", tfilename);
     fprintf(logfile, "%s ", filename);
-    free(filename);
     fprintf (codefile, 
 	     "/* Generated from %s */\n"
 	     "/* Do not edit */\n\n"
@@ -348,4 +356,63 @@ generate_type (const Symbol *s)
     generate_glue (s);
     fprintf(headerfile, "\n\n");
     fclose(codefile);
+    compare_and_move(tfilename, filename);
+    free(filename);
+    free(tfilename);
+}
+
+/*
+ * This is lifted out of config(8)'s mkheaders.c.
+ */
+
+static void
+compare_and_move(const char *tfname, const char *nfname)
+{
+	char tbuf[BUFSIZ], nbuf[BUFSIZ];
+	FILE *tfp, *nfp;
+
+	if ((tfp = fopen(tfname, "r")) == NULL)
+		err(1, "open %s", tfname);
+
+	if ((nfp = fopen(nfname, "r")) == NULL)
+		goto moveit;
+
+	while (fgets(tbuf, sizeof(tbuf), tfp) != NULL) {
+		if (fgets(nbuf, sizeof(nbuf), nfp) == NULL) {
+			/*
+			 * Old file has fewer lines.
+			 */
+			goto moveit;
+		}
+		if (strcmp(tbuf, nbuf) != 0)
+			goto moveit;
+	}
+
+	/*
+	 * We've reached the end of the new file.  Check to see if new file
+	 * has fewer lines than old.
+	 */
+	if (fgets(nbuf, sizeof(nbuf), nfp) != NULL) {
+		/*
+		 * New file has fewer lines.
+		 */
+		goto moveit;
+	}
+
+	(void) fclose(nfp);
+	(void) fclose(tfp);
+	if (remove(tfname) == -1)
+		err(1, "remove %s", tfname);
+	return;
+
+ moveit:
+	/*
+	 * They're different, or the file doesn't exist.
+	 */
+	if (nfp)
+		(void) fclose(nfp);
+	if (tfp)
+		(void) fclose(tfp);
+	if (rename(tfname, nfname) == -1)
+		err(1, "rename %s %s", tfname, nfname);
 }