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

Adapting cppExamples for Windows build #178

Merged
merged 8 commits into from
Nov 27, 2024
Merged

Adapting cppExamples for Windows build #178

merged 8 commits into from
Nov 27, 2024

Conversation

morteham
Copy link
Collaborator

  • Avoid conflict with definition of _CONCAT in xatomic.h
  • Set CXX flags for MSVC

@morteham morteham linked an issue Nov 15, 2024 that may be closed by this pull request
Copy link
Collaborator

@vegardjervell vegardjervell left a comment

Choose a reason for hiding this comment

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

Nice!

Could you add a note in the C++ "getting started" helper (docs/vCurrent/getting_started_cpp.md) mentioning anything people need to take into account when using the C++ wrapper?

It would probably also be nice to add a workflow step to the unittests workflow equivalent to Run cppExamples on Linux/macOS that just builds and runs the examples on windows so that we can be sure they stay fixed in the future :)

@morteham
Copy link
Collaborator Author

Updated docs and unittets

@vegardjervell
Copy link
Collaborator

vegardjervell commented Nov 27, 2024

It looks like the Ubuntu GitHub runner doesn't have ninja installed. I can see from the cibuildwheel workflow that CIBW fetches ninja as part of its setup process. I don't know why this is suddenly breaking the ubuntu build when it didn't before, but maybe there's some change in some package (or in scipy-build-core) that now means we need to install ninja to the ubuntu runner before building the wheel for testing.

I tried pushing a fix now, I'll merge if it works, otherwise I'll probably just need to tweak a bit for the ninja installation to work properly (ensure stuff is correctly added to PATH etc.)

Copy link
Collaborator

@vegardjervell vegardjervell left a comment

Choose a reason for hiding this comment

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

Nice stuff, we ballin'

@vegardjervell vegardjervell merged commit 34f4839 into main Nov 27, 2024
13 checks passed
@vegardjervell vegardjervell deleted the cpp_msvc branch November 27, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cmake for addon/cppExamples not working with MSVS
2 participants