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

Add blas-devel to the packages required to build against blas #2171

Merged
merged 1 commit into from
May 11, 2024

Conversation

traversaro
Copy link
Contributor

PR Checklist:

  • note any issues closed by this PR with closing keywords
  • if you are adding a new page under docs/ or community/, you have added it to the sidebar in the corresponding _sidebar.json file
  • put any other relevant information below

If you install just libblas on Windows (that installs the mkl variant), then for example in CMake find_package(BLAS REQUIRED) fails, so it is also necessary to add blas-devel. On the other hand, it is required to build against libblas as libblas has the correct run_exports, while blas-devel does not have them.

@traversaro traversaro requested a review from a team as a code owner May 4, 2024 17:37
Copy link

netlify bot commented May 4, 2024

Deploy Preview for conda-forge-previews ready!

Name Link
🔨 Latest commit 2ed9892
🔍 Latest deploy log https://app.netlify.com/sites/conda-forge-previews/deploys/66367268f2c2d500080d145a
😎 Deploy Preview https://deploy-preview-2171--conda-forge-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@traversaro
Copy link
Contributor Author

Just to explain why this PR is useful, we were hit again by the missing blas-devel in conda-forge/suitesparse-feedstock#92 .

@xhochy
Copy link
Member

xhochy commented May 11, 2024

This sounds a bit like blas-devel also should have some run_exports. I'm not sure which ones though.

@xhochy xhochy merged commit a999bd8 into conda-forge:main May 11, 2024
6 checks passed
@isuruf
Copy link
Member

isuruf commented May 12, 2024

This is unnecessary. We've had this design working for years now. Please revert and open an issue in `blas-feedstock.

@traversaro
Copy link
Contributor Author

traversaro commented May 12, 2024

This is unnecessary. We've had this design working for years now.

Sorry, it is not clear to me why this is unnecessary, can you elaborate more? Thanks!

Please revert and open an issue in `blas-feedstock.

An issue about what? That find_package(BLAS) fails on Windows if you just install libblas ?

@isuruf
Copy link
Member

isuruf commented May 12, 2024

Sorry, it is not clear to me why this is unnecessary, can you elaborate more? Thanks!

blas-devel is a meta-package for the 4 packages libblas, libcblas, liblapack, liblapacke. It's better to use the 4 as needed in recipes.

An issue about what? That find_package(BLAS) fails on Windows if you just install libblas ?

Yes

@traversaro
Copy link
Contributor Author

Sorry, it is not clear to me why this is unnecessary, can you elaborate more? Thanks!

blas-devel is a meta-package for the 4 packages libblas, libcblas, liblapack, liblapacke. It's better to use the 4 as needed in recipes.

God point, from the name I thought it only dependended on libblas plus the required devel packages (similar to other *-devel packages), but this is not the case indeed (I just checked with https://conda-metadata-app.streamlit.app/?q=conda-forge%2Flinux-64%2Fblas-devel-3.9.0-21_linux64_mkl.conda).

Yes

Done: conda-forge/blas-feedstock#120 .

@isuruf
Copy link
Member

isuruf commented May 12, 2024

Could you send a PR reverting this?

@traversaro
Copy link
Contributor Author

Could you send a PR reverting this?

Done: #2179 .

@traversaro traversaro deleted the patch-3 branch May 12, 2024 20:20
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.

3 participants