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.

Sign up for GitHub

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 symbols visibility on Windows #90

Merged
merged 3 commits into from
Oct 7, 2019
Merged

Fix symbols visibility on Windows #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7468a3d
add -no-undefined for compilation on Windows
vtorri Mar 5, 2019
02c400c
Merge remote-tracking branch 'upstream/master'
vtorri Jun 25, 2019
a12798e
fix symbols visibility on Windows
vtorri Jun 26, 2019
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
1 change: 1 addition & 0 deletions libtwolame/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ AM_CFLAGS = -I ../build/ $(WARNING_CFLAGS)
lib_LTLIBRARIES = libtwolame.la
include_HEADERS = twolame.h

libtwolame_la_CPPFLAGS = -DTWOLAME_BUILD
Copy link
Owner

Choose a reason for hiding this comment

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

Having to add this seems weird to me.
Probably not actually a problem but it also gets set for all OSes, not just Windows.

Is there a different way of doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you look carefully, TWOLAME_BUILD is used only in a #ifdef _WIN32 , so anyway, TWOLAME_BUILD will be pased to the preprocessor only if a Windows compiler is used.
I can add a test in Makefile.am, but the test is already done with the #ifdef _WIN32, so it would be useless.

Copy link
Owner

Choose a reason for hiding this comment

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

Yup, I can see that it is only used into the #ifdef _WIN32.

But is there no built-in way in automake of detecting what is being built, rather than having to define TWOLAME_BUILD ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.gnu.org/software/libtool/manual/libtool.html#Windows-DLLs :

"When the objects that form the library are built, there are generally two copies built for each object. One copy is used when linking the DLL and one copy is used for the static library. On Windows systems, a pair of defines are commonly used to discriminate how the interface symbols should be decorated. The first define is ‘-DDLL_EXPORT’, which is automatically provided by Libtool when libtool builds the copy of the object that is destined for the DLL. The second define is ‘-DLIBFOO_BUILD’ (or similar), which is often added by the package providing the library and is used when building the library, but not when consuming the library. "

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, thanks! It is really useful to know that is the official libtool way of doing it. I will try and get this merged tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

another good reading : https://gcc.gnu.org/wiki/Visibility section "How to use the new C++ visibility support"

libtwolame_la_LDFLAGS = -export-dynamic -version-info @TWOLAME_SO_VERSION@ -no-undefined
libtwolame_la_SOURCES = \
ath.c \
Expand Down
Loading