Skip to content

[xlqc][omp] fix: discover GSL paths via gsl-config - #305

Open
mhalk wants to merge 2 commits into
ORNL:masterfrom
mhalk:users/mhalk/fix/xlqc-missing-gsl-flags
Open

[xlqc][omp] fix: discover GSL paths via gsl-config#305
mhalk wants to merge 2 commits into
ORNL:masterfrom
mhalk:users/mhalk/fix/xlqc-missing-gsl-flags

Conversation

@mhalk

@mhalk mhalk commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

GSL_INC and GSL_LIB were hardcoded placeholders ("/path/to/gsl/...").
Use gsl-config to discover the installed GSL at build time.

Assisted-by: Claude Opus 4.6

@zjin-lcf

zjin-lcf commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

With no gsl-config on PATH, both variables expand to empty strings and
2>/dev/null hides the "command not found":

GSL_INC=[]
GSL_LIB=[]
LDFLAGS=[]

xlqc-omp/main.cpp includes <gsl/gsl_math.h> and friends, and the -lgsl -lgslcblas flags disappear along with the paths, so the user now sees a missing-header or undefined-reference error with no hint that GSL is the problem. The old placeholder at least printed /path/to/gsl/include in the error. A fallback would keep both properties, for example assigning with ?= and defaulting to -lgsl -lgslcblas when gsl-config isn't found, or dropping 2>/dev/null so the shell error surfaces.

The stated precedent doesn't hold up. The PR body says this matches "how e.g. mriQ-omp resolves external libraries," but mriQ-omp/Makefile has no external library at all: its LDFLAGS is empty and its only include is -I../mriQ-cuda. No Makefile in the repo currently uses gsl-config; the closest existing pattern is $(shell $(MPICC) --showme:compile) in lulesh-cuda/LULESH/cuda/build/Makefile. Worth correcting the description so reviewers aren't looking for a precedent that isn't there.

@zjin-lcf
zjin-lcf requested review from Geekdude and zjin-lcf August 5, 2026 02:27
mhalk added 2 commits August 5, 2026 05:52
GSL_INC and GSL_LIB were hardcoded placeholders ("/path/to/gsl/...").
Use gsl-config to discover the installed GSL at build time.

Assisted-by: Claude Opus 4.6
…ride

Address review: with 2>/dev/null the previous version expanded to empty
strings when gsl-config was absent, producing a confusing missing-header
error with no hint that GSL was the problem. Now:
- Remove 2>/dev/null so the shell error surfaces.
- Use ?= so callers can override GSL_INC/GSL_LIB directly.
- Error out explicitly when GSL_LIB is empty.

Assisted-by: Claude Opus 4.6
@mhalk
mhalk force-pushed the users/mhalk/fix/xlqc-missing-gsl-flags branch from 45f26a8 to c6643cf Compare August 5, 2026 12:23
@mhalk

mhalk commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback, the variables can now be overridden.
Please let me know if this is what you had in mind.

Sign up for free to 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.

2 participants