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

Fixed some cpplib build issues under Windows/Mingw. #100

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@eigenbom

eigenbom commented Aug 10, 2015

Symbols weren't exporting for the cpplib under Windows/Mingw, so I patched the Makefile.

Also patched the makefiles for two samples to get them running under Windows/Mingw.

gcc version 5.2.0 (Rev3, Built by MSYS2 project)

@danomatika

This comment has been minimized.

Member

danomatika commented Aug 10, 2015

Thanks! I'll get to this soon.

@@ -24,7 +24,10 @@
// define this to use C++11 std::mutex for locking
#ifdef LIBPD_USE_STD_MUTEX
#if __cplusplus < 201103L
#ifdef _WIN32
// __cplusplus is badly behaved in windows. just assume we have mutex

This comment has been minimized.

@danomatika

danomatika Mar 27, 2016

Member

I don't like this. Why just assume? Maybe someone is using libpd with an older C++ library and can't compile it with C++11 support? That's the point of the __cplusplus version check.

This comment has been minimized.

@eigenbom

eigenbom Apr 2, 2016

The problem is that some versions of msvc implements mutex but have _cplusplus=199711L. I agree it is bad to assume. One way around this would be to use _MSC_VER or a user-configurable header.

This comment has been minimized.

@danomatika

danomatika Sep 24, 2016

Member

Ok, I follow you. In looking around stack overflow, it seems like doing an explicit check will not always work since compilers can lie and not everything might be supported. In that case, it's better to leave the responsibility on the user and honor the define. I've decided to drop the _cplusplus check.

@@ -24,7 +24,7 @@ else
-I"$(JAVA_HOME)/include" -I"$(JAVA_HOME)/include/win32"
MINGW_LDFLAGS = -shared -lws2_32 -lkernel32
LDFLAGS = $(MINGW_LDFLAGS) -Wl,--output-def=libs/libpd.def \
-Wl,--out-implib=libs/libpd.lib
-Wl,--out-implib=libs/libpd.lib -Wl,--export-all-symbols

This comment has been minimized.

@danomatika

danomatika Mar 27, 2016

Member

Shouldn't this be handled on the line above? --output-def=libs/libpd.def Aren't the symbols available in the def file? Just double-checking.

This comment has been minimized.

@eigenbom

eigenbom Apr 2, 2016

I can't remember why I had to do this, sorry.

@@ -9,6 +9,7 @@ else
ifeq ($(OS), Windows_NT) # Windows, use Mingw
SOLIB_EXT = dll
PDNATIVE_PLATFORM = windows
CC = gcc

This comment has been minimized.

@danomatika

danomatika Mar 27, 2016

Member

If you're running in msys, isn't this set by default?

This comment has been minimized.

@eigenbom

eigenbom Apr 2, 2016

Hmm not sure, maybe it was set to cc by default.

@danomatika

This comment has been minimized.

Member

danomatika commented Sep 24, 2016

At this point, I've brought in most of your proposed changes. Can you try the latest libpd master to see of things are working on your system?

@eigenbom

This comment has been minimized.

eigenbom commented Sep 28, 2016

Ok, cheers. I'm not sure if and when I'll be able to test this, probably not for quite some time. Glad you found my patch useful.

@danomatika

This comment has been minimized.

Member

danomatika commented Jul 18, 2017

Closing due to inactivity. Assuming the updates are working...

@danomatika danomatika closed this Jul 18, 2017

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