[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #13 from fxcoudert at gcc dot gnu dot org 2007-04-24 22:37 --- Subject: Bug 31587 Author: fxcoudert Date: Tue Apr 24 22:37:37 2007 New Revision: 124126 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=124126 Log: PR fortran/31587 * lib/gcc-dg.exp (scan-module): New function. * gfortran.dg/module_md5_1.f90: New test. * module.c (write_char): Add character to the MD5 buffer. (read_md5_from_module_file): New function. (gfc_dump_module): Compute MD5 for new module file. Call read_md5_from_module_file. Only overwrite old module file if the new MD5 is different. Added: trunk/gcc/testsuite/gfortran.dg/module_md5_1.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/module.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/lib/gcc-dg.exp -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #14 from fxcoudert at gcc dot gnu dot org 2007-04-24 22:38 --- Fixed on mainline. -- fxcoudert at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #12 from fxcoudert at gcc dot gnu dot org 2007-04-20 22:13
---
(In reply to comment #11)
Time to CC Janis?
No need. There's nothing a bit of trial-and-error can't help you write :)
The following adds the necessary dejagnu directive, and uses it in a new test.
I guess the MD5 sum should be the same on all platforms, so it's a good test,
although it means the test will need updating every time someone changes the
format of module files :)
Index: lib/gcc-dg.exp
===
--- lib/gcc-dg.exp (revision 123942)
+++ lib/gcc-dg.exp (working copy)
@@ -455,6 +455,24 @@
}
}
+# Scan Fortran modules for a given regexp.
+#
+# Argument 0 is the module name
+# Argument 1 is the regexp to match
+proc scan-module { args } {
+set modfilename [string tolower [lindex $args 0]].mod
+set fd [open $modfilename r]
+set text [read $fd]
+close $fd
+
+upvar 2 name testcase
+if [regexp -- [lindex $args 1] $text] {
+ pass $testcase scan-module [lindex $args 1]
+} else {
+ fail $testcase scan-module [lindex $args 1]
+}
+}
+
# Verify that the compiler output file exists, invoked via dg-final.
proc output-exists { args } {
# Process an optional target or xfail list.
Index: gfortran.dg/module_md5_1.f90
===
--- gfortran.dg/module_md5_1.f90(revision 0)
+++ gfortran.dg/module_md5_1.f90(revision 0)
@@ -0,0 +1,14 @@
+! Check that we can write a module file, that it has a correct MD5 sum,
+! and that we can read it back.
+!
+! { dg-do compile }
+module foo
+ integer(kind=4), parameter :: pi = 3_4
+end module foo
+
+program test
+ use foo
+ print *, pi
+end program test
+! { dg-final { scan-module foo MD5:18a257e13c90e3872b7b9400c2fc6e4b } }
+! { dg-final { cleanup-modules foo } }
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #7 from fxcoudert at gcc dot gnu dot org 2007-04-17 07:52 --- Created an attachment (id=13376) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13376action=view) Patch that allows for module to be overwritten only if they changed This is the complete patch. Have fun! -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
-- fxcoudert at gcc dot gnu dot org changed: What|Removed |Added Keywords||patch Target Milestone|--- |4.3.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #10 from fxcoudert at gcc dot gnu dot org 2007-04-17 13:33 --- (In reply to comment #8) The patch looks good, though it would probably be a better idea to use tmpnam() to get the name for the temporary file. Why not. But I like the idea that it is predictable :) A further thing one could do: instead of threatening If you edit this, you'll get what you deserve.\n\n, one could actually verify that the MD5 sum matches the files contents. Too expensive! I'm against this additional cost (plus the developer cost of writing and maintaining it). About testcases, you mean for the dejagnu testsuite? I have really no idea... -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #9 from tobi at gcc dot gnu dot org 2007-04-17 13:22 --- Oh, one more issue: do you have an idea how to write testcases for this? I'm a bit at a loss, though I've only thought about this for a few minutes. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #11 from tobi at gcc dot gnu dot org 2007-04-17 13:41 --- (In reply to comment #10) (In reply to comment #8) The patch looks good, though it would probably be a better idea to use tmpnam() to get the name for the temporary file. Why not. But I like the idea that it is predictable :) Given that it's going to be deleted, it's not that much of a gain :) People always do stuff one wouldn't expect, so having filenames that could not possibly collide with user files is a good thing. A further thing one could do: instead of threatening If you edit this, you'll get what you deserve.\n\n, one could actually verify that the MD5 sum matches the files contents. Too expensive! I'm against this additional cost (plus the developer cost of writing and maintaining it). I agree that this is probably not worth it. About testcases, you mean for the dejagnu testsuite? I have really no idea... Time to CC Janis? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #8 from tobi at gcc dot gnu dot org 2007-04-17 13:20 --- The patch looks good, though it would probably be a better idea to use tmpnam() to get the name for the temporary file. A further thing one could do: instead of threatening If you edit this, you'll get what you deserve.\n\n, one could actually verify that the MD5 sum matches the files contents. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
-- fxcoudert at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |fxcoudert at gcc dot gnu dot |dot org |org Status|UNCONFIRMED |ASSIGNED Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2007-04-16 10:21:35 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #1 from tobi at gcc dot gnu dot org 2007-04-16 13:28 --- An easy solution that I thought about implementing in the past would be to put a checksum into the file header. First the module file would be written to a temporary file. This file would be checksummed and only moved to the final location if the checksum is different from that of the extant file. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #2 from tobi at gcc dot gnu dot org 2007-04-16 13:31 --- Given your other PR, another solution that comes to mind is to include the filename and modification date of the source file in the module file, and to compare these before writing a module file. This would be more prone to inconsistencies than the approach from comment #1, as it wouldn't actually be verified that the .mod file remains unchanged. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #3 from fxcoudert at gcc dot gnu dot org 2007-04-16 13:33 --- Better than my current idea for that, which was to compare line after line the old and new files. What cheap hash function should we use? MD5? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #4 from fxcoudert at gcc dot gnu dot org 2007-04-16 13:34 --- (In reply to comment #2) include the filename and modification date of the source file in the module file, and to compare these before writing a module file I think that defeats the purpose: if the source file changed but the module file didn't change (ie the module interface is the same), we don't need to recompile the files that depend on the module, and the module file shouldn't be rewritten. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #5 from tobi at gcc dot gnu dot org 2007-04-16 13:35 --- (In reply to comment #3) Better than my current idea for that, which was to compare line after line the old and new files. What cheap hash function should we use? MD5? Probably, as it is included in libiberty, and is good enough for our purposes. And please disregard my second idea as it wouldn't help in any way :-) -- tobi at gcc dot gnu dot org changed: What|Removed |Added CC||tobi at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
[Bug fortran/31587] Module files shouldn't be updated if their content doesn't change
--- Comment #6 from fxcoudert at gcc dot gnu dot org 2007-04-16 23:15
---
OK, I've researched the libiberty md5.c interface a bit and here's the first
half of a patch :) It computes the MD5 sum of the module file (except the
first 3 lines) and it writes it at the top, like this:
GFORTRAN module created from test_mod.f90 on Tue Apr 17 00:12:12 2007
MD5:18a257e13c90e3872b7b9400c2fc6e4b -- If you edit this, you'll get what you
deserve.
I checked that it gives the same result as the md5sum command on my linux box
:)
Index: module.c
===
--- module.c(revision 123388)
+++ module.c(working copy)
@@ -72,6 +72,7 @@
#include arith.h
#include match.h
#include parse.h /* FIXME */
+#include md5.h
#define MODULE_EXTENSION .mod
@@ -170,6 +171,9 @@
/* The FILE for the module we're reading or writing. */
static FILE *module_fp;
+/* MD5 context structure. */
+static struct md5_ctx ctx;
+
/* The name of the module we're reading (USE'ing) or writing. */
static char module_name[GFC_MAX_SYMBOL_LEN + 1];
@@ -1275,6 +1279,9 @@
if (fputc (out, module_fp) == EOF)
gfc_fatal_error (Error writing modules file: %s, strerror (errno));
+ /* Add this to our MD5. */
+ md5_process_bytes (out, sizeof (out), ctx);
+
if (out != '\n')
module_column++;
else
@@ -3926,7 +3933,10 @@
int n;
char *filename, *p;
time_t now;
+ fpos_t md5_pos;
+ unsigned char md5_new[16]; /*, md5_old[16]; */
+
n = strlen (name) + strlen (MODULE_EXTENSION) + 1;
if (gfc_option.module_dir != NULL)
{
@@ -3959,8 +3969,14 @@
fprintf (module_fp, GFORTRAN module created from %s on %s\n,
gfc_source_file, p);
- fputs (If you edit this, you'll get what you deserve.\n\n, module_fp);
+ fputs (MD5:, module_fp);
+ fgetpos (module_fp, md5_pos);
+ fputs ( --
+If you edit this, you'll get what you deserve.\n\n, module_fp);
+ /* Initialize the MD5 context that will be used for output. */
+ md5_init_ctx (ctx);
+
iomode = IO_OUTPUT;
strcpy (module_name, name);
@@ -3973,6 +3989,11 @@
write_char ('\n');
+ md5_finish_ctx (ctx, md5_new);
+ fsetpos (module_fp, md5_pos);
+ for (n = 0; n 16; n++)
+fprintf (module_fp, %02x, md5_new[n]);
+
if (fclose (module_fp))
gfc_fatal_error (Error writing module file '%s' for writing: %s,
filename, strerror (errno));
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31587
