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:
- It doesn’t find
gawk, but uses
- 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 20200816
I’m on macOS Catalina 10.15.7 and
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. 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.
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?
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 <firstname.lastname@example.org> 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 . 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. : 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.