Skip to content

Conversation

jkawamoto
Copy link
Contributor

This change exports the directory where libopenblas is located.

Having this information available is especially useful when compiling Rust bindings for other C/C++ libraries that depend on OpenBLAS. In such cases, knowing the exact location of libopenblas provided by this crate helps ensure a smoother build process and reduces the need for additional configuration.

@Dirreke
Copy link
Contributor

Dirreke commented Sep 6, 2025

Thanks, it looks great! How about use INCLUDE=${INCLUDE_PATH}?

@jkawamoto
Copy link
Contributor Author

Do you mean exporting the include path instead of the root path? That makes more sense. I’ve updated this PR so that it now exports both the include path and the library path.

@Dirreke
Copy link
Contributor

Dirreke commented Sep 8, 2025

Thanks! By the way, when do we actually need to use library_path? From my side, exporting include_path seems sufficient. Also, I noticed the export for system OpenBLAS was removed — was there a specific reason? I feel keeping it might still be helpful.

@jkawamoto
Copy link
Contributor Author

Thank you for your review!

I’m working on a Rust binding for CTranslate2, and in their CMakeLists.txt they check whether OpenBLAS is available by looking for cblas.h and libopenblas.a like this:

find_path(OPENBLAS_INCLUDE_DIR NAMES cblas.h)
find_library(OPENBLAS_LIBRARY NAMES openblas)

Since libopenblas.a is generated in the target directory, we need to provide the actual path to CMake in order to compile CTranslate2.

As for the system OpenBLAS library paths, I believe package managers already add them to the appropriate environment variables, so we usually don’t need to explicitly pass those paths when building another library. That said, I’m open to exporting them if you think it would be helpful.

@Dirreke
Copy link
Contributor

Dirreke commented Sep 8, 2025

That makes sense. But I’d prefer to export them for both static and system to keep things consistent. Besides, could you also update the README?

@jkawamoto
Copy link
Contributor Author

I understand. I’ve added the exports for the system feature as well and updated the README accordingly.

@Dirreke Dirreke merged commit a7776a9 into blas-lapack-rs:master Sep 8, 2025
18 checks passed
@Dirreke
Copy link
Contributor

Dirreke commented Sep 9, 2025

v0.10.13 is released

@jkawamoto
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants