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 MSVC FTBFS #490

Merged
merged 1 commit into from
Dec 3, 2015
Merged

Fix MSVC FTBFS #490

merged 1 commit into from
Dec 3, 2015

Conversation

xlz
Copy link
Member

@xlz xlz commented Dec 2, 2015

The workaround cdd4f06 (from #476) broke MSVC building (reported in #489). I didn't test MSVC in #476. MSVC refuses to resolve the symbol because the return type is different, which was the point of the workaround.

Alternative workarounds would make it more a mess. I have sent a patch to iai_kinect2 directly to use new API. code-iai/iai_kinect2#189

Build tested OK with MSVC and GCC after this PR.

The workaround broke MSVC building. MSVC refuses to resolve the
symbol because the return type is different, which was the
point of the workaround.

Alternative workarounds would make it more a mess. I have sent a
patch to iai_kinect2 directly to use new API.
@RyanGordon
Copy link
Contributor

This also fixes an issue with generating SWIG code on master in #427 👍

@floe
Copy link
Contributor

floe commented Dec 2, 2015

@RyanGordon great, so did you also test this on MSVC? Then I think I'll merge it right away, multiple inheritance is actually a big problem with SWIG IIRC, so good to get rid of it.

@floe floe added this to the 0.1 milestone Dec 2, 2015
@RyanGordon
Copy link
Contributor

@floe Unfortunately, I don't have MSVC setup to test this

@xlz
Copy link
Member Author

xlz commented Dec 3, 2015

#489 user reported this worked on Windows.

floe added a commit that referenced this pull request Dec 3, 2015
@floe floe merged commit 4162721 into OpenKinect:master Dec 3, 2015
@xlz xlz deleted the msvc-symbol-resolving branch December 4, 2015 02:52
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.

3 participants