INSTALL - FIX - Make bundled-dependency builds honour FC/CC and embed rpath by krystophny · Pull Request #279 · PrincetonUniversity/GPEC

krystophny · 2026-07-24T22:51:02Z

Bundled HDF5/netCDF builds can retain a previous compiler's configuration when switching FC/CC. Their configure probes and the GPEC executables can also fail to load the intended netCDF libraries at runtime.

Export FC, CC and F77 centrally from install/DEFAULTS.inc, so dependency configure scripts receive the selected toolchain.
Make the existing realclean target run distclean for HDF5, netCDF-C and netCDF-Fortran. It removes their configuration and compiler-dependent installed libraries/modules, including those under a custom DEPSINSTALLDIR. Run make realclean before switching compilers; toolchain changes are not detected automatically.
Embed the configured library directories in dependency configure probes and executable runtime search paths. These paths must remain available when running the binaries.

No Fortran source changes. Current develop is merged, including the fork-PR CI checkout fix.

Validation:

Compiler export checks for values supplied by a makefile, environment or command line; cleanup checks for configured/unconfigured dependencies and default/custom prefixes.
make checkdeps, workflow lint and git diff --check pass.
Bundled HDF5/netCDF plus dcon, gpec and pentrc build with GNU (gfortran/gcc 16.2.1, system LAPACK), followed by make realclean and a rebuild with Intel (ifx/icx 2026.0, MKL 2026.0).
netCDF read/write round trips and runtime library resolution verified for both toolchains.
All five GitHub CI checks pass, including GNU/Intel builds and the Soloviev regression examples.

Remaining limitation: ifx with LAPACKHOME=/usr can still select a system GNU netcdf.mod, because existing RULES.inc places the math include directory before NETCDFINC. This was reproduced locally; include-path precedence is unchanged by this PR. Intel validation above uses MKL.

… rpath Three related build failures when the bundled netcdf/hdf5 stack is used with a compiler other than the one that first configured it. 1. The netcdf-c, netcdf-fortran and hdf5 rules run ./configure only when no Makefile is present, and never pass FC or CC to it. A tree first built with gfortran therefore keeps its gfortran configuration after FC is changed, and the Fortran codes then fail with error #7013: This module file was not generated by any release of this compiler. [NETCDF] followed by a long cascade of error #6683: A kind type parameter must be a compile-time constant. [R8] The second message is misleading: r8 is a PARAMETER in equil/local.f and is perfectly valid. The kind errors are collateral damage from the failed module read, and disappear once the module matches the compiler. This cost real debugging time, so the configure calls now pass CC, FC and F77 explicitly. 2. netcdf-fortran's configure links and *runs* test programs against the freshly installed netcdf-c. Without the install lib directory on the runtime search path those tests build but abort, and configure stops with the unhelpful configure: error: cannot compute sizeof (off_t) The configure calls now add -Wl,-rpath alongside the existing -L. 3. The codes themselves linked with -L$(NETCDFDIR) but no rpath, so they resolved against whatever libnetcdff the loader found first. That produced symbol lookup error: undefined symbol: netcdf_mp_nf90_create_ when a system netcdf shadowed the bundled one. NETCDF_EXTRA_LIBS now carries -Wl,-rpath for the Fortran and C library directories. A depsclean target is added, since plain clean leaves the dependency configuration in place and there was no supported way to switch toolchains. Verified by building the full stack and dcon, gpec and pentrc from scratch with FC=ifx CC=icx on Linux, and separately with FC=gfortran CC=gcc.

logan-nc · 2026-10-03T10:37:59Z

@claude review this pr

claude · 2026-10-03T10:38:14Z

Claude finished @logan-nc's task in 35s —— View job

Review of PR #279: honour FC/CC and embed rpath for bundled deps

The three fixes match the failure modes in the description. I read the diff only and did not build or run anything. git diff origin/develop...HEAD failed here with "no merge base" (shallow checkout), so I used git show HEAD.

Things that look right

Passing CC, FC and F77 to the netcdf-c and netcdf-fortran ./configure calls is the correct fix for the stale-toolchain problem.
-Wl,-rpath next to -L in LDFLAGS should fix the cannot compute sizeof (off_t) failure, because configure's test programs can now find the freshly installed libnetcdf.
NETCDF_RPATH in DEFAULTS.inc only adds the second rpath when NETCDF_C_DIR differs from NETCDFDIR. It is appended to NETCDF_EXTRA_LIBS in all three branches, including the one that sets it to empty.

Issues and suggestions

depsclean ignores DEPSINSTALLDIR (install/makefile). The build rules honour DEPSINSTALLDIR ?= ../deps, but depsclean hardcodes ../deps/....
- With a custom DEPSINSTALLDIR, it leaves the real install tree untouched, so the stale-module problem persists.
- The rm -rf ../deps/lib/* ../deps/include/* ../deps/bin/* line is broad. It also removes anything else installed there, such as OpenBLAS, which is built into the same prefix (TARGETS.inc:86).
- Consider using $(DEPSINSTALLDIR) consistently. You could also remove only the netcdf/hdf5 artifacts, or at least document that OpenBLAS is rebuilt as well.
- Because DEPSINSTALLDIR is defined in TARGETS.inc, check that it is visible from the makefile at that point.
Comment placement (install/makefile). The new depsclean block sits between PYTHON ?= python3 and .PHONY: deps checkdeps. The comment above PYTHON says it is about regenerating DEPENDENCIES.inc, so it now ends up above depsclean and no longer describes deps. Move depsclean above that comment block.
A toolchain change is still not detected automatically. The [ ! -f Makefile ] guard remains. The fix works only if users know to run depsclean. A cheap safeguard would be to record FC/CC in a stamp file, or grep config.log for the configured compiler, and then fail with a message pointing to make depsclean. This is optional, but it would address the debugging cost you describe.
Check that FC/CC have sensible values when unset.
- Make's built-in default is FC=f77, and DEFAULTS.inc may or may not override it. Confirm that configure gets the intended compiler when the user passes nothing.
- Passing FC to hdf5 without --enable-fortran is harmless but has no effect.
- hdf5 does not get F77. This is also harmless.
rpath scope. The rpath is an absolute build-tree path. That is fine for in-tree use, but binaries will not be relocatable. A one-line note in the docs or the comment would help. A system netcdf in NETCDF_C_DIR is also hardwired this way.
Minor. The LDFLAGS line is duplicated in two rules. A shared variable such as DEPS_RPATH would reduce the duplication.

No changes to .f files, so the 72-column rule doesn't apply. Item 1 is the one I'd fix before merging. The rest are optional.

logan-nc · 2026-10-03T10:39:43Z

@krystophny please assign a reviewer to get merged more promptly interested future 🙏

logan-nc · 2026-10-03T12:46:12Z

I approve if the Claude review point 1 is fixed and @matt-pharr approves (I 100% deferred to @matt-pharr 's superior knowledge of these installation details)

krystophny · 2026-10-08T07:16:16Z

done

matt-pharr

See comments. Very useful contributions but should be cleaned up.

matt-pharr · 2026-10-08T14:23:12Z

 	if [ -f ../deps/src/netcdf-fortran/Makefile ]; then cd ../deps/src/netcdf-fortran && make clean; fi

+# Remove the autotools configuration of the bundled dependencies.  The netcdf
+# and hdf5 rules only run ./configure when no Makefile is present, so a plain


Comments like this should be in the PR, not the code. A terse two-liner would do fine here.

matt-pharr · 2026-10-08T14:34:18Z

 	cd ../; git submodule update --init deps/src/netcdf-c;
 	if [ ! -f ../deps/src/netcdf-c/Makefile ]; then \
 		cd ../deps/src/netcdf-c; \
+			CC="$(CC)" FC="$(FC)" F77="$(FC)" \


CC="$(CC)" FC="$(FC)" can go, these are already set in DEFAULTS.inc. You could consider adding export F77="$(FC)" to DEFAULTS.inc since that is where we set all these sorts of variables. This applies to the lines below as well.

matt-pharr · 2026-10-08T14:37:10Z

+# headers and tools are replaced on installation.  Other installed libraries,
+# including OpenBLAS, are retained.
+.PHONY: depsclean
+depsclean:


we don't really need this to be its own target, I would just change make clean to make distclean for the libraries in the realclean target.

matt-pharr · 2026-10-08T14:38:52Z

@krystophny please let me know if you disagree with any of this

…changes

krystophny · 2026-10-09T12:01:27Z

@matt-pharr Updated as suggested: shorter comments, FC/CC/F77 exported in DEFAULTS.inc, and dependency distclean folded into realclean (custom-prefix cleanup retained). Merged current develop, including the fork CI fix.

All five CI checks pass. Local GNU and ifx/MKL builds and netCDF round trips pass, including a realclean compiler switch.

One existing limitation remains: ifx + LAPACKHOME=/usr can pick up the system GNU netcdf.mod because math includes precede NETCDFINC; documented in the PR.

logan-nc assigned matt-pharr Aug 1, 2026

logan-nc added the Installation label Aug 1, 2026

logan-nc requested a review from matt-pharr October 3, 2026 10:39

INSTALL: scope dependency cleanup to the configured prefix

24b84b9

matt-pharr requested changes Oct 8, 2026

View reviewed changes

krystophny added 3 commits October 9, 2026 13:51

INSTALL: centralize compiler exports and use realclean for toolchain …

a1f2596

…changes

CI: check out fork branches from their source repository

94f1b04

Merge develop and retain upstream fork CI fixes

3cc7639

krystophny requested a review from matt-pharr October 9, 2026 12:12

matt-pharr approved these changes Oct 9, 2026

View reviewed changes

logan-nc approved these changes Oct 9, 2026

View reviewed changes

logan-nc merged commit e3b6ec6 into PrincetonUniversity:develop Oct 9, 2026
7 checks passed

Navigation Menu

Repository navigation

INSTALL - FIX - Make bundled-dependency builds honour FC/CC and embed rpath - #279

INSTALL - FIX - Make bundled-dependency builds honour FC/CC and embed rpath#279
logan-nc merged 5 commits into
PrincetonUniversity:developfrom
krystophny:agent/ifx-build-fixes

krystophny commented Jul 24, 2026 •

edited

Loading

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

claude Bot commented Oct 3, 2026 •

edited

Loading

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

krystophny commented Oct 8, 2026

Uh oh!

matt-pharr left a comment

Uh oh!

matt-pharr Oct 8, 2026

Uh oh!

Uh oh!

matt-pharr Oct 8, 2026

Uh oh!

matt-pharr Oct 8, 2026

Uh oh!

matt-pharr commented Oct 8, 2026

Uh oh!

krystophny commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

krystophny commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

claude Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review of PR #279: honour FC/CC and embed rpath for bundled deps

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

logan-nc commented Oct 3, 2026

Uh oh!

krystophny commented Oct 8, 2026

Uh oh!

matt-pharr left a comment

Choose a reason for hiding this comment

Uh oh!

matt-pharr Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matt-pharr Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

matt-pharr Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

matt-pharr commented Oct 8, 2026

Uh oh!

krystophny commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krystophny commented Jul 24, 2026 •

edited

Loading

claude Bot commented Oct 3, 2026 •

edited

Loading