From b64593afc07d3f62a13a3b6b634bcb80b7fe6180 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Mon, 21 Dec 2020 19:08:33 -0800 Subject: [PATCH] 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 --- lib/cgraph/scan.l | 17 ++++++++--------- rtest/test_regression.py | 1 - 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/cgraph/scan.l b/lib/cgraph/scan.l index 224eb93d8..6e06994a2 100644 --- 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 2ca488265..fbe96c697 100644 --- 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 -- 2.40.0