Skip to content

Build NetCDF without DAP and HDF4: halve the Linux wheel - #154

Merged
antonio-leblanc merged 1 commit into
forefireAPI:masterfrom
HugoFara:perf/slim-netcdf-deps
Aug 12, 2026
Merged

Build NetCDF without DAP and HDF4: halve the Linux wheel#154
antonio-leblanc merged 1 commit into
forefireAPI:masterfrom
HugoFara:perf/slim-netcdf-deps

Conversation

@HugoFara

@HugoFaraHugoFara commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Halves the Linux wheel by building NetCDF without features ForeFire does not use.

BeforeAfter
Compressed8.6 MB4.0 MB
Unpacked23.3 MB12.0 MB
Vendored libraries326

What was in there

EPEL's netcdf package enables DAP (reading datasets over HTTP) and HDF4. auditwheel correctly vendors their entire dependency closure, so a wheel whose own code is 2.6 MB was shipping:

libcurl, libcrypto, libssl, libkrb5, libgssapi_krb5, libk5crypto,
libkrb5support, libcom_err, libkeyutils, libldap, liblber, libsasl2,
libssh, libnghttp2, libbrotlicommon, libbrotlidec, libpsl, libidn2,
libunistring, libselinux, libpcre2, libcrypt <- 10.2 MB, DAP
libmfhdf, libdf, libjpeg, libtirpc <- 1.4 MB, HDF4

26 of 32 libraries. ForeFire opens local .nc files and Meso-NH PGD files, so neither feature is reachable from it.

The security side matters more than the size. That list includes OpenSSL 1.1.1k, which is past end of life, plus Kerberos, LDAP and libssh. Any scanner pointed at an installed forefire flags them, and we would be respinning wheels for CVEs in code the simulator never calls.

What changed

tools/devops/install-netcdf-manylinux.sh now builds netcdf-c 4.9.3 from source, checksum pinned, rather than installing netcdf-devel from EPEL:

--disable-dap --disable-byterange --disable-hdf4 --disable-libxml2

--disable-byterange is needed as well as --disable-dap, because byterange is a second remote-access path that links libcurl on its own. HDF5 still comes from EPEL. netcdf-cxx4 was already built from source and is unchanged, other than skipping its examples/ directory: they call nc_set_log_level, which netcdf-c only exports when built --enable-logging, and 4.3.1 has no --disable-examples.

The script now fails if libnetcdf regains a libcurl dependency, so a future change to the base image cannot quietly undo this.

Effect on platform consistency

Mixed, and worth a decision rather than a nod:

  • HDF4: this removes an inconsistency. Homebrew's NetCDF has never enabled it, so the macOS wheel already could not read HDF4 files while the Linux one could. They now agree.
  • DAP: this introduces one. macOS keeps DAP, because its libnetcdf resolves libcurl to /usr/lib/libcurl.4.dylib, a system library delocate leaves alone. It costs nothing there, so there was no reason to build NetCDF from source on macOS just to remove it. If you would rather the two match exactly, say so and I will do the same on macOS.

The remaining 6 libraries are libnetcdf, libnetcdf_c++4, libhdf5, libhdf5_hl, libsz and libaec, which is the same core set the macOS wheel carries.

Verification

Built in a clean manylinux_2_28_x86_64 container with podman, installed into a fresh venv on a Fedora 44 host that has no NetCDF of its own:

CheckResult
Smoke test tests/python/test_wheel.pypasses, 122 fire nodes
forefire -vv2.5.0
Repository example tests/runff/run.ffexit 0, GeoJSON 1625 bytes, byte-identical to the current wheel
libnetcdf links libcurlno, and the script now asserts it

Open question

Does anything in ForeFire or the Meso-NH coupling read datasets over OPeNDAP, or open HDF4 files? I could not find a path to either, but the coupling is the part I know least. If one of them is genuinely used, the right answer is to enable it deliberately on both platforms instead, and this should be closed.


This pull request, including its code changes and this description, was generated by Claude Opus 5.

EPEL's netcdf enables DAP and HDF4. auditwheel then has to vendor their
whole dependency closure: libcurl drags in OpenSSL 1.1.1k, Kerberos, LDAP,
SASL, libssh and a dozen more, and HDF4 adds libmfhdf, libdf, libjpeg and
libtirpc. That is 26 of the 32 libraries in the wheel, for features
ForeFire never calls: it reads local .nc and PGD files.
Building netcdf-c from source with --disable-dap, --disable-byterange and
--disable-hdf4 takes the wheel from 8.6 MB to 4.0 MB compressed, 23.3 MB to
12.0 MB unpacked, and 32 vendored libraries to 6. The remaining set matches
what the macOS wheel already ships, and dropping HDF4 removes a capability
difference between the two platforms.
The script now fails if libnetcdf regains a libcurl dependency, so this
cannot silently regress.
@filippi

filippi commented Aug 11, 2026 via email

Copy link
Copy Markdown
Contributor

@HugoFara

HugoFara commented Aug 12, 2026

Copy link
Copy Markdown
CollaboratorAuthor

@antonio-leblanc I'll need your help to accept this one. GitHub won't let me merge, maybe because I was added to the project after opening the PR.


EDIT

@filippi I am asking NetCDF C++ if the project continues, they haven't released in 7 years and that is causing considerable drift with other tools. Otherwise, yes we align on Méso-NH. Currently NetCDF is making the code complex to serve only a fraction of users.

@pantera-botpantera-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

✅ Looks Good

  • Dropping DAP and HDF4 significantly reduces the manylinux wheel size and its attack surface (no more bundled OpenSSL, Kerberos, libssh).
  • The script is well-structured: pinning the source tarballs and checksums is a solid security practice.
  • Fallback checks are correctly implemented (failing if is dynamically linked).
  • Consistent behavior between Linux and macOS regarding HDF4 is a plus.

💡 Suggestions

  • The PR description states that the pkg-config file (netcdf-cxx4.pc) is still installed. However, because make install SUBDIRS=cxx4 only recurses into the cxx4 directory, the root-level pkgconfig_DATA rule is skipped and the .pc file is actually omitted. This is not a blocker for ForeFire, as its CMakeLists.txt uses find_path and find_library rather than pkg_check_modules, but it's worth noting.
  • Regarding the open question on DAP and HDF4: ForeFire's NetCDF interactions (including Meso-NH coupling) operate exclusively on local files (.nc, PGD), so OPENDAP and HDF4 features are indeed safe to remove.

Reviewed by Hermes Agent

@pantera-botpantera-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

✅ Looks Good

  • Dropping DAP and HDF4 significantly reduces the manylinux wheel size and its attack surface (no more bundled OpenSSL, Kerberos, libssh).
  • The install-netcdf-manylinux.sh script is well-structured: pinning the source tarballs and checksums is a solid security practice.
  • Fallback checks are correctly implemented (failing if libcurl is dynamically linked).
  • Consistent behavior between Linux and macOS regarding HDF4 is a plus.

💡 Suggestions

  • The PR description states that the pkg-config file (netcdf-cxx4.pc) is still installed. However, because make install SUBDIRS=cxx4 only recurses into the cxx4 directory, the root-level pkgconfig_DATA rule is skipped and the .pc file is actually omitted. This is not a blocker for ForeFire, as its CMakeLists.txt uses find_path and find_library rather than pkg_check_modules, but it's worth noting.
  • Regarding the open question on DAP and HDF4: ForeFire's NetCDF interactions (including Meso-NH coupling) operate exclusively on local files (.nc, PGD), so OPENDAP and HDF4 features are indeed safe to remove.

Reviewed by Hermes Agent

@antonio-leblanc
antonio-leblanc merged commit 2f2ae15 into forefireAPI:masterAug 12, 2026
8 checks passed
antonio-leblanc pushed a commit that referenced this pull request Aug 13, 2026
The repository has seven tags and six GitHub releases, and no file that
tells a user what changed between them. `pip install forefire` makes that
worse: someone pinning a version has the release notes on GitHub and
nothing in the tree.
CHANGELOG.md reconstructs every release from the release notes and the
commit history. Claims that could be checked were checked against the
tags: listenHTTP and RothermelAndrews2018 are absent at v1.2 and present
at v2.0, the geojson dump mode is absent at v1.1.10.
[Unreleased] lists only what is on this branch, #154 and #155. The dev
branch carries a longer list in its own copy, which supersedes this one
when dev is merged.
CONTRIBUTING.md now asks for an entry, so the file stays current instead
of being reconstructed again in a year. Its link to a testing page
pointed at a Read the Docs URL that has never existed; it points at
TESTING.md, which nothing in the repository linked to.
antonio-leblanc pushed a commit that referenced this pull request Aug 13, 2026
The repository has seven tags and six GitHub releases, and no file that
tells a user what changed between them. `pip install forefire` makes that
worse: someone pinning a version has the release notes on GitHub and
nothing in the tree.
CHANGELOG.md reconstructs every release from the release notes and the
commit history. Claims that could be checked were checked against the
tags: listenHTTP and RothermelAndrews2018 are absent at v1.2 and present
at v2.0, the geojson dump mode is absent at v1.1.10.
[Unreleased] lists only what is on this branch, #154 and #155. The dev
branch carries a longer list in its own copy, which supersedes this one
when dev is merged.
CONTRIBUTING.md now asks for an entry, so the file stays current instead
of being reconstructed again in a year. Its link to a testing page
pointed at a Read the Docs URL that has never existed; it points at
TESTING.md, which nothing in the repository linked to.
(cherry picked from commit cc7e1789c66432aec56673993c3b6e198e0525a0)
antonio-leblanc pushed a commit that referenced this pull request Aug 13, 2026
The CHANGELOG landed on master carries only #154 and #155 under
[Unreleased], because that is all master has. This adds the ten pull
requests merged into dev since: the unit suite, the moisture invariants,
the sanitizer job, the HTTP characterisation tests, the threading
reproduction, and the shared-state and double-free fixes.
Two things went stale on dev while that work landed, both of them ours.
The README's CMake option table stopped at six options; #156 and #180
added FOREFIRE_BUILD_TESTS, FOREFIRE_ENABLE_WARNINGS,
FOREFIRE_WARNINGS_AS_ERRORS and FOREFIRE_SANITIZE. And CONTRIBUTING told
contributors to run `cd tests && bash run.bash`, which does not reach the
C++ unit tests at all; ctest does.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@HugoFara@filippi@antonio-leblanc@pantera-bot