Skip to content

intel-oneapi-mkl: linking with the +cluster variant is broken for external mkl#39343

Merged
rscohn2 merged 11 commits intospack:developfrom
wrtobin:wrtobin/intel-oneapi-mkl/external-package-mpi
Aug 10, 2023
Merged

intel-oneapi-mkl: linking with the +cluster variant is broken for external mkl#39343
rscohn2 merged 11 commits intospack:developfrom
wrtobin:wrtobin/intel-oneapi-mkl/external-package-mpi

Conversation

@wrtobin
Copy link
Copy Markdown
Contributor

@wrtobin wrtobin commented Aug 8, 2023

fixes #38238.

Adding `mpi` variant to deal with external oneapi-mkl usage per spack#38238.
rscohn2
rscohn2 previously approved these changes Aug 8, 2023
Copy link
Copy Markdown
Member

@rscohn2 rscohn2 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this issue!

@wrtobin wrtobin marked this pull request as ready for review August 8, 2023 18:03
self.spec.satisfies(m)
for m in ["^intel-oneapi-mpi", "^intel-mpi", "^mpich", "^cray-mpich"]
):
if self.spec.satisfies("mpi=mpich"):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After some more thought:
this change will break the package for some existing users. Could you instead leave line 187 and add "mpi=mpich" to the list there? Same for "mpi=openmpi" below

rscohn2
rscohn2 previously approved these changes Aug 8, 2023
@rscohn2 rscohn2 enabled auto-merge (squash) August 8, 2023 18:25
auto-merge was automatically disabled August 8, 2023 19:49

Head branch was pushed to by a user without write access

rscohn2
rscohn2 previously approved these changes Aug 8, 2023
@rscohn2 rscohn2 enabled auto-merge (squash) August 8, 2023 19:58
@rscohn2 rscohn2 disabled auto-merge August 9, 2023 10:39
@rscohn2
Copy link
Copy Markdown
Member

rscohn2 commented Aug 9, 2023

I don't understand the gitlab failure. Maybe mpi is a reserved keyword? You could change the variant name to "mpi_family" or some other name.

@rscohn2
Copy link
Copy Markdown
Member

rscohn2 commented Aug 9, 2023

I verified locally that renaming the variant to something other than mpi makes the gitlab issue go away.

@wrtobin
Copy link
Copy Markdown
Contributor Author

wrtobin commented Aug 9, 2023

Thanks for verifying

rscohn2
rscohn2 previously approved these changes Aug 9, 2023
@rscohn2 rscohn2 enabled auto-merge (squash) August 9, 2023 18:45
@rscohn2 rscohn2 merged commit b8bfaf6 into spack:develop Aug 10, 2023
mpokorny pushed a commit to mpokorny/spack that referenced this pull request Sep 18, 2023
…ernal mkl (spack#39343)

* Update package.py

Adding `mpi` variant to deal with external oneapi-mkl usage per spack#38238.

* style conformance

* accept both mpi specification mechanisms

* style conformance

* Update package.py

* Update var/spack/repos/builtin/packages/intel-oneapi-mkl/package.py

Co-authored-by: Robert Cohn <[email protected]>

* rename mpi variant mpi_family

* style conformance

* update help message for mpi_family

---------

Co-authored-by: Robert Cohn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

intel-oneapi-mkl: linking with the +cluster variant is broken for external mkl

2 participants