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

Fix incorrect string manipulation #43739

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

wrwilliams
Copy link
Contributor

rstrip was of course intended, not sure how remove_suffix worked under any conditions.

Fixes #43716.

Copy link
Contributor

@wspear wspear left a comment

Choose a reason for hiding this comment

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

This still doesn't work for me. I get: Error: AttributeError: type object 'super' has no attribute 'with_or_without'

Copy link
Contributor

@wspear wspear left a comment

Choose a reason for hiding this comment

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

This fails like:
==> Error: AttributeError: 'list' object has no attribute 'replace'

Because with_or_without returns a list of arguments, not a single string. Are you able to test these changes against spack develop before committing? Do you have a test case where the "=yes" string is present? It doesn't show up when I print the results of with_or_without from my default scorep install. (The only argument returned for me is "--without-shmem")

@wrwilliams
Copy link
Contributor Author

This fails like: ==> Error: AttributeError: 'list' object has no attribute 'replace'

Because with_or_without returns a list of arguments, not a single string. Are you able to test these changes against spack develop before committing? Do you have a test case where the "=yes" string is present? It doesn't show up when I print the results of with_or_without from my default scorep install. (The only argument returned for me is "--without-shmem")

I do not have a working develop environment at present, and do not have many cycles free to create one. A working test case for the =yes string should be generated via any MPI or SHMEM variant that is not explicitly included in the subsequent mapping (e.g. OneAPI MPI, see #43700).

--with-mpi should work on ~99% of systems as long as a) there is only one MPI present at build time, which Spack should ensure is the case, and b) the MPI in question doesn't do anything weird that keeps us from autodetecting it as OMPI/MPICH/Intel. --with-mpi=yes (ditto --with-shmem=yes) will simply not work.

If --with-or-without cannot handle not having a with-value, it is the wrong tool and the package should instead map any dependency on the virtual MPI target to --with-mpi manually.

@wspear
Copy link
Contributor

wspear commented Apr 23, 2024

I wasn't sure if this should go to the issue or this PR... but for reference: #43700 (comment)

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.

Score-P fails to build
2 participants