-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
CI Make no-OpenMP build fail with unprotected cimport openmp
#25391
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
Conversation
…nto ci-no-openmp-build
cimport openmp
This seems to work as intended, adding a unprotected From this build log
In the log you can also see the omp.h that are inside the conda env and deleted:
|
I merged |
I assume that this is more the spirit of what we tried to do in this specific CI. |
9281d0b
to
e9c23cc
Compare
cimport openmp
cimport openmp
Let me summarize because it took me a while to understand exactely what was not failing and why we want it to fail.
So I'm fine with just deleting the openmp header in the CI to make sure we don't have unprotected imports or calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this still works with the current main
, LGTM as well.
…t-learn#25391) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
…t-learn#25391) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
…t-learn#25391) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Reference Issues/PRs
Fix #24694
What does this implement/fix? Explain your changes.
This is an attempt so that unprotected
cimport openmp
make the no-OpenMP build fail.See #24682 (comment) for more details.