Broken build due to commit reversion related to awk

The change to Makefile.am in plugins/core creating ps.h using RS="\r*\n" breaks the build on machines using standard awk. I reverted the change but this seemed to break various Windows builds. I not sure why this happened. I hope someone more familiar with the Windows build can suggest an alternative that works for both environments.

I made that change, but later on I realized that it wasn’t strictly necessary since I found a way to avoid crlf altogether. I kept it because I thought it was a good thing anyway, but didn’t realize that it could cause problems in other environments. I’m sorry about that.

There are a couple of more places were RS="\r*\n" has been introduced that also might cause problems.

What OS are such machines running? I would like to add a CI job testing that. Perhaps also make RS="\r*\n" conditioned on something so it does not occur on those OSes.

In future, it would be better that you push changes to a branch in your own fork of the repository and open a merge request so we can discuss before we merge to main.

The reason the Windows jobs fail is orthogonal to your change. See fix: upgrade Pytest 6.2.2 → 6.2.4 (!2202) · Merge requests · graphviz / graphviz · GitLab.

Here’s some more background to the original change:

I apologize for not following the new protocol. In future, I will try to follow this, though for other reasons, I may pass on any changes to someone else and ask him to submit it.

The problem was with awk. Linux probably comes with gawk for awk, so it does accept regular expressions for RS. The other versions of Unix, e.g., OSX, use the standard awk, which only allows a single character for RS. I take it no Apple machine is used during release validation.

Requiring gawk, or testing for it, doesn’t seem worth the effort.

There’s absolutely no need to apologize. We haven’t communicated anything about this. It’s just the way most projects work nowadays.

I’m not very familiar with the Apple world, but if OSX is the same thing as macOS, then we do build on it using the autotools build system in CI. See e.g. this job where you can see that:

  1. It doesn’t find gawk, but uses awk instead
  2. It does use RS="\r*\n" without any noticeable issues.

What version of OSX are you using?

One data point: I’m on macOS 11.6 Big Sur and

awk —version
awk version 20200816

1 Like

I’m on macOS Catalina 10.15.7 and

/usr/bin/awk --version
awk version 20070501

This version of awk definitely only allows one character for RS. I assume the Big Sur version now allows regular expressions.

14 years old. :scream: Is that really the version from the system?

Apparently so. I do nothing special when I move to a new release and that’s what I get in /usr/bin.

So the classic tool version problem: use the most vanilla form; tell the user to upgrade; or tune the build to check for differences. Though, in this case, it seems largely moot, as magjac seemed to suggest the \r\n problem can be avoided and awk isn’t heavily used.

1 Like

This is somewhat confusing as we run pretty vanilla macOS in CI and it appears to be using the system awk:

checking for gawk... no
checking for mawk... no
checking for nawk... no
checking for awk... awk

Furthermore, for once I happen to have a Mac on hand and it reports different results to Emden:

$ which awk
/usr/bin/awk
$ awk --version
awk version 20200816
$ uname -rms
Darwin 20.5.0 x86_64

I don’t understand macOS versioning, but this machine is not on the latest macOS either. It claims to be “macOS Big Sur 11.4”. Emden is a macOS upgrade available from your machine?

uname -rms
Darwin 19.6.0 x86_64

I’m not aware of any OS updates besides moving to Big Sur.

I investigated what this Catalina creature is and it appears to be an older release of macOS. Do we support older macOS releases? Obviously unnecessarily breaking them is not ideal, but AFAIK we have no ability to CI test anything except the latest macOS release, so these accidental breakages will keep happening.

Some datapoints: It’s successor, macOS Big Sur, was released on November 12, 2020, so Catalina is almost a year out of date at this stage.

macOS upgrades are pretty well supported even on somewhat older hardware (e.g. my 5 year old macbook pro is running Big Sur).

It’s a real shame that macOS shipped with such an ancient build.

I think it’d be reasonable to support, say, the N-1 macOS for 6 months after the new macOS version releases.

To add another little anecdote, we have made compromises to cope with the decrepit state of tooling on macOS in the past. I’m specifically thinking of:

commit b64593afc07d3f62a13a3b6b634bcb80b7fe6180
Author: Matthew Fernandez <matthew.fernandez@gmail.com>
Date:   Mon Dec 21 19:08:33 2020 -0800

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

I’ve submitted an MR with a better solution to the original problem. I’d appreciate if @erg could try that it doesn’t cause any problems on macOS Catalina.

The changes and build work fine on my mac.

1 Like