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

Changes to enable Windows build #2

Closed
wants to merge 1 commit into from

Conversation

cegbertOculus
Copy link
Contributor

changes to Python.h fixes:
magnum-bindings\src\Magnum/SceneGraph/Python.h(67): error C2614: 'Magnum::SceneGraph::PyObjectesp::scene::SceneNode': illegal member initialization: 'Object' is not a base or member

change to magnum.cpp fixes:
magnum-bindings\src\python\magnum\magnum.cpp(220): error C2375: 'PyInit__magnum': redefinition; different linkage

change to glfw.cpp fixes:
magnum-bindings\src\python\magnum\platform\glfw.cpp(44): error C3640: 'magnum::platform::glfw::PublicizedApplication::[thunk]: __thiscall void __cdecl magnum::platform::glfw(pybind11::module &)'::2'::PublicizedApplication::`vcall'{8,{flat}}' }'': a referenced or virtual member function of a local class must be defined

@mosra mosra added this to the 2019.0b milestone Aug 20, 2019
@mosra
Copy link
Owner

mosra commented Aug 20, 2019

Hi, thank you for the continued push on Windows support!

This is something I didn't even try for the bindings repo yet. I need to set up a Windows CI job to ensure it builds and continues to build there.

For the first thing, is it possible that Object is some silly macro defined in windows.h? :)

@cegbertOculus
Copy link
Contributor Author

Yes, I think that's the case, though I couldn't find the source of the problem. The errors I was getting referencing 'Object' weren't really making sense, so I just changed the name to Obj.

@mosra
Copy link
Owner

mosra commented Sep 4, 2019

Finally had the time to set up a Windows CI. Merged the second and third fix as e8198cb and 14fa224. The Object rename was not needed, but also Magnum is not including windows.h that much anymore, so maybe that fixed it.

Can you confirm current master works for you? Thanks a lot!

@mosra mosra mentioned this pull request Sep 13, 2019
60 tasks
@mosra
Copy link
Owner

mosra commented Sep 13, 2019

Based on private discussion, the C2614-related change was unfortunately really needed as well, comitted as d6f1c50.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants