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

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

CSHARP_LDFLAGS = $(MINGW_LDFLAGS) -Wl,--output-def=libs/libpdcsharp.def \
-Wl,--out-implib=libs/libpdcsharp.lib
CPP_LDFLAGS = $(LDFLAGS)
Expand Down
5 changes: 4 additions & 1 deletion cpp/PdBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
#include <iostream>

#ifdef LIBPD_USE_STD_MUTEX
#if __cplusplus <= 201103L // C++ 11 check
#ifdef _WIN32
#define _LOCK() mutex.lock()
#define _UNLOCK() mutex.unlock()
#elif __cplusplus <= 201103L // C++ 11 check
#define _LOCK() mutex.lock()
#define _UNLOCK() mutex.unlock()
#endif
Expand Down
5 changes: 4 additions & 1 deletion cpp/PdBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

#include <mutex>
#elif __cplusplus < 201103L
#warning std::mutex requires C++11
#else
#include <mutex>
Expand Down
1 change: 1 addition & 0 deletions samples/c/pdtest/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ else
ifeq ($(OS), Windows_NT) # Windows, use Mingw
SOLIB_EXT = dll
PDNATIVE_PLATFORM = windows
CC = gcc
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

else # Assume Linux
SOLIB_EXT = so
PDNATIVE_PLATFORM = linux
Expand Down
3 changes: 2 additions & 1 deletion samples/cpp/pdtest_rtaudio/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ else
SOLIB_PREFIX =
PDNATIVE_PLATFORM = windows
CXXFLAGS = -D__WINDOWS_DS__
AUDIO_API = -lole32 -loleaut32 -ldsound -lwinmm
else # Assume Linux
SOLIB_EXT = so
PDNATIVE_PLATFORM = linux
Expand All @@ -42,7 +43,7 @@ CXXFLAGS += -I$(LIBPD_DIR)/pure-data/src -I$(LIBPD_DIR)/libpd_wrapper -I$(LIBPD_
.PHONY: clean clobber

$(TARGET): ${SRC_FILES:.cpp=.o} $(LIBPD_CPP)
g++ -o $(TARGET) $^ $(LIBPD_CPP) $(AUDIO_API)
g++ -o $(TARGET) $^ $(AUDIO_API)
if [ $(PDNATIVE_PLATFORM) == "mac" ]; then mkdir -p ./libs && cp $(LIBPD_CPP) ./libs; fi

$(LIBPD_CPP):
Expand Down