More macos fixes #6929
Open
More macos fixes #6929
+158
−43
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
problem: MacOS/BSD lack a linux-compatible proc (usually) so we can't read routes from proc solution: On linux we'll continue to use /proc/net/route, while on macos it turns out you can have a default interface *that does not have a netmask of zero*. To get it reliably, I've added a version of `getprimary_interface4` that uses the CoreFoundation and SystemConfiguration frameworks to request the global IPv4 default interface name from macos directly. This is more macos specific than using a PF_ROUTE socket, but much _much_ simpler. If we want this to work on other BSDs (aside from FreeBSD with linux compat turned on) we'll want to do that but NETLINK/PF_ROUTE are really arcane and rather high overhead for something this simple. Anyway, this is using the classic C interface to CoreFoundation rather than any of the add-ons so there's no need to compile code as Objective-C or anything. As to why I switched to this from the getifaddrs approach, that seemed to work fine on linux with a small tweak, but not on macos, and if it doesn't work reliably cross-platform it seems better to keep what we had battle tested and add the more specific mac version like this.
problem: MacOS provides NSIG which is used in one of the kill unit tests but not SIGRTMAX, which made the check fail because our test uses NSIG and our code assumes 64 if SIGRTMAX is not set solution: if SIGRTMAX is not set, but NSIG is available, then define SIGRTMAX=(NSIG-1)
problem: mkostemp is only defined to accept O_APPEND, O_SYNC, and O_CLOEXEC. Setting other flags is allowed to cause failures, and does on MacOS solution: remove the unnecessary flag
problem: the @ expansion for the anonymous domain socket namespace is only available on linux solution: use real paths in tmp
problem: all binding and cpuset related operations fail on macos solution: don't call them
problem: all attempts to interact with binding on macos fail solution: do not call them on macos
problem: our pty handling, despite using portable interfacs, didn't work
correctly on BSD-like platforms, specifically macos. It turns out the
posix_openpt method of creating a pty creates fragile results on macos,
but the classic BSD method of using `openpty`, which also sets the
initial size, works on both mac and linux just fine. Unfortunately it
comes from a header named `pty.h`, which conflicts with the initial name
of the libterminus pty handling header and caused include issues.MacOS
is much more likely to return ENOTTY when a terminal doesn't support
resizing (vim terminals apparently being one).
solution: use `openpty` on both platforms to get working ptys, to solve
getting the include I decided to rename pty.{c,h} rather than deal with
include_next or similar due to portability issues. This also includes
handling ENOTTY from terminal resizing and looping over write to handle
`EINTR`, `EAGAIN` and `EWOULDBLOCK` specifically for pty writes.
problem: we don't properly handle EINTR as a result from waitpid in libsubprocess, terminus, or even in the tap death tests harness. solution: in each case, loop over waitpid as long as the return is -1 and errno is EINTR to ensure we get to the end of the syscall at least once
problem: the plugins built specifically for the plugstack tests weren't getting the avoid-version flag to libtool, so they were generating symlinks to *.so.<ver> on linux and *.<ver>.so on macos. On macos this caused the plugstack tests to fail due to more libraries being found than should be. solution: add the avoid-version flag to the libraries in automake (thanks for this one @grondo, I was really lost)
problem: macos doesn't have /proc solution: macos, linux and I think the other BSDs all have /dev/fd with the same effect, so switch the path
problem: As @garlick noted we get EBADF when trying to run local processes through subprocess. I _think_ this is because the macos implementation reports errors when asked to close a file descriptor that is already closed. solution: Because of the structure of the spawn handler, and the likely low need for high-speed launch on macos, I decided to force macos to use the fork launcher for now. This should be revisited, as should allowing posix_spawn to chdir if we have support for that in the linux versions we target.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is stacked on top of #6926, and contains all changes necessary to get all tests in
srcto pass. I don't have the GHA updates in here yet, but that's coming next.