Re: etags test is broken on MS-Windows

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 21 May 2015 15:24:44 +0200
> From: Francesco Potortì <pot@gnu.org>
> Cc: emacs-devel@gnu.org, Paul Eggert <eggert@cs.ucla.edu>
> 
> >f-src/entry.for,172
> >      LOGICAL FUNCTION PRTPKG ^?3,75
> >       ENTRY  SETPRT ^?194,3866
> >       ENTRY  MSGSEL ^?395,8478
> >     & intensity1(^?577,12231
> >       character*(*) function foo(^?579,12307
> >^L
> >f-src/entry.strange_suffix,172
> >      LOGICAL FUNCTION PRTPKG ^?3,75
> >       ENTRY  SETPRT ^?194,3866
> >       ENTRY  MSGSEL ^?395,8478
> >     & intensity1(^?577,12231
> >       character*(*) function foo(^?579,12307
> >^L
> >f-src/entry.strange,171
> >      LOGICAL FUNCTION PRTPKG ^?2,2
> >       ENTRY  SETPRT ^?193,3793
> >       ENTRY  MSGSEL ^?394,8405
> >     & intensity1(^?576,12158
> >       character*(*) function foo(^?578,12234
> >
> >Now, these 3 files have exactly identical contents, and the _only_
> >difference between the first 2 and the 3rd is that the latter is
> >gzip-compressed.  So that should be the only reason why all its line
> >counts are off by 1, and its byte counts are all off by 73, which just
> >happens to be the length of the first line of the (uncompressed) file.
> 
> This is a bug.
> 
> >So could it be that rewinding a 'popen'-created stream doesn't work
> >correctly on GNU/Linux as well?  If so, we will have to make changes
> >in etags to not do that, I think, and instead reuse the already-read
> >stuff as needed.
> 
> It could well be.  It may have happened that, when I checked that the
> TAGS files were the same, I just looked at them without running diff and
> I missed this discrepancy.

After thinking a bit about the alternative solution, I concluded that
the simplest will be to decompress to a temporary file and read from
there.  Does the patch below look OK?  Or can someone think about a
more elegant way of solving this?


diff --git a/lib-src/etags.c b/lib-src/etags.c
index 0a308c1..28729da 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -116,6 +116,7 @@ CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 # undef HAVE_NTGUI
 # undef  DOS_NT
 # define DOS_NT
+# define O_CLOEXEC O_NOINHERIT
 #endif /* WINDOWSNT */
 
 #include <unistd.h>
@@ -125,6 +126,7 @@ CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 #include <sysstdio.h>
 #include <ctype.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <binary-io.h>
@@ -336,6 +338,7 @@ CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 static char *absolute_dirname (char *, char *);
 static bool filename_is_absolute (char *f);
 static void canonicalize_filename (char *);
+static char *etags_mktmp (void);
 static void linebuffer_init (linebuffer *);
 static void linebuffer_setlen (linebuffer *, int);
 static void *xmalloc (size_t);
@@ -1437,7 +1440,7 @@ C code are parsed as C code (use --help --lang=c --lang=yacc\n\
   fdesc *fdp;
   compressor *compr;
   char *compressed_name, *uncompressed_name;
-  char *ext, *real_name;
+  char *ext, *real_name, *tmp_name;
   int retval;
 
   canonicalize_filename (file);
@@ -1522,9 +1525,20 @@ C code are parsed as C code (use --help --lang=c --lang=yacc\n\
     }
   if (real_name == compressed_name)
     {
-      char *cmd = concat (compr->command, " ", real_name);
-      inf = popen (cmd, "r" FOPEN_BINARY);
-      free (cmd);
+      tmp_name = etags_mktmp ();
+      if (!tmp_name)
+	inf = NULL;
+      else
+	{
+	  char *cmd1 = concat (compr->command, " ", real_name);
+	  char *cmd = concat (cmd1, " > ", tmp_name);
+	  free (cmd1);
+	  if (system (cmd) == -1)
+	    inf = NULL;
+	  else
+	    inf = fopen (tmp_name, "r" FOPEN_BINARY);
+	  free (cmd);
+	}
     }
   else
     inf = fopen (real_name, "r" FOPEN_BINARY);
@@ -1536,10 +1550,12 @@ C code are parsed as C code (use --help --lang=c --lang=yacc\n\
 
   process_file (inf, uncompressed_name, lang);
 
+  retval = fclose (inf);
   if (real_name == compressed_name)
-    retval = pclose (inf);
-  else
-    retval = fclose (inf);
+    {
+      remove (tmp_name);
+      free (tmp_name);
+    }
   if (retval < 0)
     pfatal (file);
 
@@ -1707,9 +1723,6 @@ C code are parsed as C code (use --help --lang=c --lang=yacc\n\
 	}
     }
 
-  /* We rewind here, even if inf may be a pipe.  We fail if the
-     length of the first line is longer than the pipe block size,
-     which is unlikely. */
   rewind (inf);
 
   /* Else try to guess the language given the case insensitive file name. */
@@ -1734,8 +1747,6 @@ C code are parsed as C code (use --help --lang=c --lang=yacc\n\
       if (old_last_node == last_node)
 	/* No Fortran entries found.  Try C. */
 	{
-	  /* We do not tag if rewind fails.
-	     Only the file name will be recorded in the tags file. */
 	  rewind (inf);
 	  curfdp->lang = get_language_from_langname (cplusplus ? "c++" : "c");
 	  find_entries (inf);
@@ -5015,8 +5026,6 @@ enum,		0,			st_C_enum
       TEX_opgrp = '<';
       TEX_clgrp = '>';
     }
-  /* If the input file is compressed, inf is a pipe, and rewind may fail.
-     No attempt is made to correct the situation. */
   rewind (inf);
 }
 
@@ -6344,6 +6353,51 @@ enum,		0,			st_C_enum
   return path;
 }
 
+/* Return a newly allocated string containing a name of a temporary file.  */
+static char *
+etags_mktmp (void)
+{
+  const char *tmpdir = getenv ("TMPDIR");
+  const char *slash = "/";
+
+#if MSDOS || defined (DOS_NT)
+  if (!tmpdir)
+    tmpdir = getenv ("TEMP");
+  if (!tmpdir)
+    tmpdir = getenv ("TMP");
+  if (!tmpdir)
+    tmpdir = ".";
+  if (tmpdir[strlen (tmpdir) - 1] == '/'
+      || tmpdir[strlen (tmpdir) - 1] == '\\')
+    slash = "";
+#else
+  if (!tmpdir)
+    tmpdir = "/tmp";
+  if (tmpdir[strlen (tmpdir) - 1] == '/')
+    slash = "";
+#endif
+
+  char *templt = concat (tmpdir, slash, "etXXXXXX");
+  int fd = mkostemp (templt, O_CLOEXEC);
+  if (fd < 0)
+    {
+      free (templt);
+      templt = NULL;
+    }
+  else
+    close (fd);
+
+#if defined (DOS_NT)
+  /* The file name will be used in shell redirection, so it needs to have
+     DOS-style backslashes, or else the Windows shell will barf.  */
+  char *p;
+  for (p = templt; *p; p++)
+    if (*p == '/')
+      *p = '\\';
+#endif
+  return templt;
+}
+
 /* Return a newly allocated string containing the file name of FILE
    relative to the absolute directory DIR (which should end with a slash). */
 static char *