Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/gmath: properly guard BLAS and LAPACK dependent code #4346

Merged
merged 5 commits into from
Sep 20, 2024

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Sep 19, 2024

Properly guard BLAS and LAPACK dependent code.

@nilason nilason added this to the 8.5.0 milestone Sep 19, 2024
@nilason nilason self-assigned this Sep 19, 2024
@github-actions github-actions bot added CI Continuous integration C Related code is in C libraries labels Sep 19, 2024
@nilason nilason marked this pull request as ready for review September 19, 2024 14:10
@nilason
Copy link
Contributor Author

nilason commented Sep 19, 2024

I used Mac runner to test without --with-blas and --with-lapack and it succeeded. Opening for review.

@neteler
Copy link
Member

neteler commented Sep 19, 2024

Side-question concerning addons:

In my fork I get
https://github.com/neteler/grass-addons/actions/runs/10937266666/job/30362726188

checking for lapacke.h... no configure: error: *** Unable to find "lapacke.h" with "-I/usr/include/x86_64-linux-gnu/openblas-pthread/ -g -O2 ". Error: Process completed with exit code 1.

Would that be addressed here as well?

@nilason
Copy link
Contributor Author

nilason commented Sep 19, 2024

OSGeo/grass-addons#1206 adds liblapacke-dev which is enough.

echoix
echoix previously approved these changes Sep 20, 2024
@nilason nilason changed the title lib/gis: properly guard BLAS and LAPACK dependent code lib/gmath: properly guard BLAS and LAPACK dependent code Sep 20, 2024
@nilason
Copy link
Contributor Author

nilason commented Sep 20, 2024

I have now updated this to work as it did before d5bb442. Namely, to only compile lib/gmath/la.c (and being able to include la.h) IFF compiled with BLAS and LAPACK.

The first solution (02b0d96) made available all parts which weren't directly using BLAS or LAPACK (only a couple of functions out of 30+) even without compiling with BLAS and LAPACK.

I'd say we follow the original logic and go for the present state of this PR. If need arises in the future, we may revise this.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Sounds like a straightforward solution.

@nilason nilason merged commit c4424ab into OSGeo:main Sep 20, 2024
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C CI Continuous integration libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants