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

Handle empty OpenMP_CXX_FLAGS correctly #2884

Merged
merged 2 commits into from Mar 21, 2021
Merged

Conversation

rcurtin
Copy link
Member

@rcurtin rcurtin commented Mar 20, 2021

@zoq reported yesterday that a build with Python and -DUSE_OPENMP=OFF fails. As it turns out, the reason for this is that an empty argument gets appended to the list of compilation flags, and then when those are joined with a space, we end up with two spaces in the compilation command. For whatever reason, setuptools/cython does not like this, and it causes a compilation failure with a strange error message:

https://gist.github.com/zoq/8d65e3caf0a58ef9772e8466caeaf3cc

I fixed the issue by only adding ${OpenMP_CXX_FLAGS} to the list of compilation arguments if it isn't empty.

Copy link
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

this is also neater! 😄

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Thanks again for the fast patch!

@rcurtin rcurtin merged commit 4654b8f into mlpack:master Mar 21, 2021
@rcurtin rcurtin deleted the python-omp-fix branch March 21, 2021 01:28
This was referenced Oct 14, 2022
@rcurtin rcurtin mentioned this pull request Oct 23, 2022
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.

None yet

3 participants