fix repeated agmemreads failing

This reverts commits 429718cb387092b5bf81850ac97d18ca34149055 but with a
slightly different variant of the macro hack that avoids subsequent sed
replacement steps. I had several false starts at this fix, so let's detail what
did not work:

  1. We cannot implement an always-no-answering isatty() locally or redirect
     isatty() to another function, because Flex calls isatty(fileno(f)) and our
     "FILE" from SFIO does not provide this. On operating systems where fileno()
     references an offset in the FILE struct (e.g. macOS) this causes a
     segfault. So we need to suppress the entire argument to isatty(), not just
     the call to it.

  2. Versions of Flex prior to 2.5.39 emit a spurious extern declaration [0]. If
     we reverted 429718cb387092b5bf81850ac97d18ca34149055 as-is, we would have
     also had to revert 3b00c1fc6b949cc9744d075e0d2b2ed4b1c46763. However, even
     this would not be enough because we now support a CMake build, so we would
     have to introduce a corresponding find-and-replace step in
     lib/cgraph/CMakeLists.txt. Rather than do this, I came up with a different
     macro replacement that still makes isatty() calls evaluate to 0 but does
     not require other find-and-replace steps.

  3. Although Flex 2.5.39 was released almost a decade ago, we cannot easily
     drop support for prior versions of Flex. The latest version of Apple XCode
     ships Flex 2.5.35 and is unlikely to upgrade any time soon. This means most
     macOS users have Flex 2.5.35 without realizing it. Compounding the problem,
     the files generated by Flex #include headers that ship with Flex, so even
     macOS users who have a newer version of Flex via Homebrew/Macports may
     accidentally have the Flex 2.5.35 headers in their include path. The path
     of least irritation to our downstream macOS users appear to be retaining
     Flex 2.5.35 support.

Fixes #1910. The underlying problem here still exists: with `never-interactive`
set in the lib/cgraph/scan.l lexer, we are somehow not resetting internal state
in between lexer invocations. I suspect this will return as an issue in future
and we will have to more thoroughly debug this.

  [0]: Fixed upstream in Flex, but only made it into release 2.5.39:
    https://github.com/westes/flex/commit/7ef69b02e6969d1a0372e1d4ee8cb3b51519dc62

diff --git a/lib/cgraph/scan.l b/lib/cgraph/scan.l
index 224eb93d8b895e4e613374eed98e398229541d94..6e06994a2b35b1fc1fa43f1ef729c299e4a4cde2 100644 (file)
--- a/lib/cgraph/scan.l
+++ b/lib/cgraph/scan.l
@@ -53,6 +53,14 @@ void agsetfile(char* f) { InputFile = f; line_num = 1; }
  */
 void aglexinit(Agdisc_t *disc, void *ifile) { Disc = disc; Ifile = ifile; graphType = 0;}
 
+/* By default, Flex calls isatty() to determine whether the input it is
+ * scanning is coming from the user typing or from a file. However, our input
+ * is being provided by Graphviz' I/O channel mechanism, which does not have a
+ * valid file descriptor that supports isatty().
+ */
+#define isatty(x) gv_isatty_suppression
+int gv_isatty_suppression;
+
 #ifndef YY_INPUT
 #define YY_INPUT(buf,result,max_size) \
        if ((result = Disc->io->afread(Ifile, buf, max_size)) < 0) \
@@ -182,15 +190,6 @@ static int chkNum(void) {
  * harm. (Presumably undefined characters will be ignored in display.) And,
  * it allows a greater wealth of names. */
 %}
-
-  /* By default, Flex calls isatty() to determine whether the input it is
-   * scanning is coming from the user typing or from a file. However, our input
-   * is being provided by Graphviz' I/O channel mechanism, which does not have a
-   * valid file descriptor that supports isatty(). To suppress Flex's behavior,
-   * we tell it that the input is unconditionally a file.
-   */
-%option never-interactive
-
 GRAPH_EOF_TOKEN                                [@]     
 LETTER [A-Za-z_\200-\377]
 DIGIT  [0-9]

diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index 2ca4882653bc46de7eea4100de75c982ed795bdc..fbe96c6973d72b090a68acc9c096f5a4265e18d9 100644 (file)
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -589,7 +589,6 @@ def test_1909():
                      ' b -> c;\n' \
                      '}\n'
 
-@pytest.mark.xfail(strict=True) # FIXME
 def test_1910():
     '''
     Repeatedly using agmemread() should have consistent results