Fix macOS build: qsort_r, OpenMP, linker flags#27
Merged
Conversation
Three pre-existing bugs that prevented building on macOS: 1. qsort_r has different signatures on BSD (macOS) vs GNU (Linux). The comparator and context arguments were swapped, causing a segfault in KDTree construction. Use #ifdef __APPLE__ to select the correct calling convention. 2. Apple Clang doesn't support -fopenmp. Auto-detect compiler support and skip the flag when unavailable. 3. --enable-new-dtags is Linux-only but was being added on Darwin due to an inverted ifeq/ifneq condition. Also adds a qsort_r regression test (10k points) and macOS to the CI workflow matrix.
Homebrew installs X11 libraries (libxaw, libxt, libx11, etc.) into per-package prefixes, so pkg-config can't find them without explicit PKG_CONFIG_PATH. Add a setup step that discovers and exports the paths. Also install additional X11 dependencies (libx11, libxmu, libxext, xorgproto) needed for the build.
GITHUB_ENV with PKG_CONFIG_PATH wasn't being picked up by Make's $(shell pkg-config ...) evaluation. Store the X11 paths in a custom env var (X11_PKG_CONFIG_PATH) and export PKG_CONFIG_PATH explicitly in the build and test steps.
Homebrew X11 packages spread headers across per-package prefixes, making pkg-config discovery unreliable in Make's $(shell ...) context. XQuartz installs everything to /opt/X11/ which the Makefile already handles as a built-in fallback path.
XQuartz installs to /opt/X11 but the Makefile's wildcard detection may not find it. Use the existing X11_PREFIX override to set the path directly. Added debug ls to verify XQuartz headers are present.
The pattern rule $(OBJDIR)/interface/%.o was shadowed by the more general $(OBJDIR)/%.o rule because Make's % matches 'interface/x_interface' in both patterns. Merged into a single rule using $(findstring) to select X11_FULL_CFLAGS for interface/ files and BASE_CFLAGS for others. Also restructured X11 detection so X11_PREFIX takes priority over pkg-config, preventing partial pkg-config results from masking the explicit path override.
view.c references zarr/grib functions when those features are enabled. The test Makefile now conditionally links file_zarr.c and file_grib.c when WITH_ZARR=1 or WITH_GRIB=1 is set.
The test expected variable name "t@surface=0" but the scanner uses just the shortName "t". Also fixed n_dims assertion: depth dimension is always added for GRIB variables (even single-level), so n_dims=2 not 1.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
qsort_rsegfault on macOS — BSD and GNU have different argument order for the comparator and context pointer. Uses#ifdef __APPLE__to select the correct calling convention-fopenmpwhen compiler doesn't support it (Apple Clang)--enable-new-dtags— was incorrectly added on Darwin (ifeq→ifneq), causing linker errormacos-latest) to CI workflow matrix with GRIB+Zarr build and testskdtree_qsort_r_large_treeregression test (10k scattered points, verifies sort correctness)Test plan
kdtree_qsort_r_large_treetest passes (16/16 kdtree tests)make WITH_GRIB=1 WITH_ZARR=1builds cleanly on macOS