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

[lapack-reference] expose the lapacke option of lapack-reference #41911

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Nov 2, 2024

This adds the missing lapacke CMake option of lapack-reference so that vcpkg follows the upstream options.

Reference-LAPACK/lapack@1573c82/CMakeLists.txt#L344

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@aminya aminya mentioned this pull request Nov 2, 2024
11 tasks
@Neumann-A
Copy link
Contributor

LAPACKE needs to be its own port

@aminya
Copy link
Contributor Author

aminya commented Nov 2, 2024

LAPACKE needs to be its own port

@Neumann-A We can later add a generic LAPACKE port. This is simply exposing the option of lapack-reference, not touching the lapack port.

https://github.com/Reference-LAPACK/lapack/blob/1573c8275433f137107f02bb48017c2165e3ce96/CMakeLists.txt#L344

@aminya aminya changed the title [lapack-reference] support the lapacke feature of lapack-reference [lapack-reference] expose the lapacke option of lapack-reference Nov 2, 2024
@Neumann-A
Copy link
Contributor

This is simply exposing the option of lapack-reference, not touching the lapack port.

No LAPACKE depends on LAPACK and you can switch implementations lapack. as such LAPACKE is a port which depends on the lapack port!

@JonLiu1993 JonLiu1993 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Nov 4, 2024
@aminya
Copy link
Contributor Author

aminya commented Nov 4, 2024

This is simply exposing the option of lapack-reference, not touching the lapack port.

No LAPACKE depends on LAPACK and you can switch implementations lapack. as such LAPACKE is a port which depends on the lapack port!

Lapacke depends on lapack, but the lapacke code in lapack-reference works out of the box with lapck-reference.
What you're describing is adding patches to make that work with any lapack implementation, which is workable. However, multiple changes should be made to this code to make it usable with other lapack implementations (e.g MKL), which is not the goal of this pull request and is not a blocker.

https://github.com/Reference-LAPACK/lapack/blob/1573c8275433f137107f02bb48017c2165e3ce96/LAPACKE/CMakeLists.txt

@Neumann-A
Copy link
Contributor

You don't understand that the netlib lapack git consistent of different projects in the same repo. As such you are exposing a different project via a feature which is not the correct way to do this in vcpkg. Lapacke is logical different project and needs its own port.

@JonLiu1993
Copy link
Member

Tested feature lapacke successfully in the following triplet:

  • x86-windows
  • x64-windows
  • x64-windows-static

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Nov 12, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Nov 12, 2024

IIUC the community advice, based on blas/lapack experience, was to package lapacke(-reference) separate from lapack-reference, with a dependency on lapack.

@JonLiu1993 JonLiu1993 removed the info:reviewed Pull Request changes follow basic guidelines label Nov 13, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Nov 25, 2024

LAPACKE needs to be its own port

Draft: #42345

@dg0yt dg0yt mentioned this pull request Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants