Re: EINTR causing useless recompilation?

[Tom Rodriguez]

Hello again.  I just wanted to follow up about this bug as I've found
the same problem in the vpath code in 3.79.1, since the stat logic
isn't shared between these two pieces of code.  However in this case
it doesn't cause useless recompilation, it causes builds to fail since
it can't find files anymore, which is a much more serious problem. 
The fix is to use the name_mtime code from remake.c in vpath.c and
rewrite selective_vpath_search to use it instead of a raw stat.  I've
attached a unified diff (against 3.79.1) of those changes below and
confirmed that it does indeed solve the problem.  Obviously the code
should be shared between the two files but I didn't go that far
because I wasn't sure where to put it.

If anything I've said is unclear please contact me.  I'd love to see
these changes make it into the next update of 3.80, along with
restoring the EINTR fixes from 3.79.1.  Thank you.

tom

--- vpath.c~    2000-06-13 07:24:45.000000000 -0700
+++ vpath.c     2002-12-10 17:17:47.568864000 -0800
@@ -368,6 +368,26 @@
 }
 
 
+/* Return the mtime of the file or archive-member reference NAME.  */
+
+static FILE_TIMESTAMP
+name_mtime (name)
+     register char *name;
+{
+  struct stat st;
+
+ while (stat (name, &st) != 0)
+   if (errno != EINTR)
+     {
+       if (errno != ENOENT && errno != ENOTDIR)
+         perror_with_name ("stat:", name);
+       return NONEXISTENT_MTIME;
+     }
+
+  return FILE_TIMESTAMP_STAT_MODTIME (name, st);
+}
+
+
 /* Search the given VPATH list for a directory where the name pointed
    to by FILE exists.  If it is found, we set *FILE to the newly
malloc'd
    name of the existing file, *MTIME_PTR (if MTIME_PTR is not NULL)
to
@@ -507,7 +527,7 @@
             exists, but stat fails for it, confusion results in the
             higher levels.  */
 
-         struct stat st;
+          FILE_TIMESTAMP mtime;
 
 #ifndef VMS
          /* Put the slash back in NAME.  */
@@ -515,7 +535,7 @@
 #endif
 
          if (!exists_in_cache  /* Makefile-mentioned file need not exist. 
*/
-             || stat (name, &st) == 0) /* Does it really exist?  */
+             || (mtime = name_mtime(name)) != NONEXISTENT_MTIME) /* Does it
really exist?  */
            {
              /* We have found a file.
                 Store the name we found into *FILE for the caller.  */
@@ -527,7 +547,7 @@
                   If we have had no need to stat the file here,
                   we record UNKNOWN_MTIME to indicate this.  */
                *mtime_ptr = (exists_in_cache
-                             ? FILE_TIMESTAMP_STAT_MODTIME (name, st)
+                             ? mtime
                              : UNKNOWN_MTIME);
 
              free (name);


Tom Rodriguez wrote:
> 
> Sorry about that.  I was using 3.79.  3.79.1 appears to do the right
> thing and looking at the code it has a loop checking for EINTR.  3.80
> doesn't have the loop anymore but it does print a message saying that
> stat was interrupted but it still performs the useless recompilation.
> It looks like some other delta may have overridden the fix.  Here's the
> code in 3.80:
> 
> static FILE_TIMESTAMP
> name_mtime (name)
>       register char *name;
> {
>    struct stat st;
> 
>    if (stat (name, &st) != 0)
>      {
>        if (errno != ENOENT && errno != ENOTDIR)
>          perror_with_name ("stat:", name);
>        return NONEXISTENT_MTIME;
>      }
> 
>    return FILE_TIMESTAMP_STAT_MODTIME (name, st);
> }
> 
> Here's 3.79.1, which appears perfect to me:
> 
> /* Return the mtime of the file or archive-member reference NAME.  */
> 
> static FILE_TIMESTAMP
> name_mtime (name)
>       register char *name;
> {
>    struct stat st;
> 
>   while (stat (name, &st) != 0)
>     if (errno != EINTR)
>       {
>         if (errno != ENOENT && errno != ENOTDIR)
>           perror_with_name ("stat:", name);
>         return NONEXISTENT_MTIME;
>       }
> 
>    return FILE_TIMESTAMP_STAT_MODTIME (name, st);
> }
> 
> Searching in google, there was some talk about Hurd and it's behaviour
> in regards to EINTR makes me wonder if someone intentionally deleted the
> EINTR loop.
> 
> Thanks for getting back to me.  I'm happy since I can use 3.79.1 but
> hopefully 3.80.1 can have the loop again so this doesn't get lost.
> 
> tom
> 
> Paul D. Smith wrote:
> 
> > Please always include the version of GNU make you're using when
> > reporting problems.
> >
> > I believe this has been fixed in the latest version of GNU make, 3.80.
> >
> > Please try again with that version and let me know.
> >
> >