-
Notifications
You must be signed in to change notification settings - Fork 67
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
Using cmake with an alternate toolchain #146
Comments
Line 353 in ce01373
It checks Actually correct path is: Line 330 in ce01373
I am almost one hundred percent sure that this should work with MinGW on Windows. |
This is also very strange. |
Also this is strange. Not sure it is correct version. Maybe something is not installed. |
Probably generator must be |
For what it's worth, this is my toolchain file for cross-compilation purposes:
It actually should, if I remember correctly [snipping other stuff]
I am mentioned, but I don't speak and not proficient in cmake, I don't I don't know whether he'd be interested in, but @madebr did quite a lot |
The error is about an invalid stray
This file is generated here: Lines 351 to 352 in ce01373
I ran these 2 lines locally, and it generated
which looks incorrect. The string should not escape the semicolons. |
Interestingly, the generated map file I get has no stray |
No idea why that's happening. set(CONFTTEST_CONTENTS "VERS_1 {\n global: sym\;\n\n};\n\nVERS_2 {\n global: sym;\n} VERS_1\;")
file(WRITE conftest.map "${CONFTTEST_CONTENTS}") And ran:
Both 3.9.6 and my system CMake 3.24.2 generated the same |
In any case, removal of escaping semicolons should not hurt. |
No, the escape is only needed when you need to embed a semicolon as member of a list. |
So, the following is a go? (I couldn't make it to emit the missing newline to end, though.) diff --git a/CMakeLists.txt b/CMakeLists.txt
index 597efac..5bd290f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -350,3 +350,3 @@ elseif(CMAKE_SYSTEM_NAME MATCHES "Darwin")
else()
- set(CONFTTEST_CONTENTS "VERS_1 {\n global: sym\;\n\n};\n\nVERS_2 {\n global: sym;\n} VERS_1\;")
+ set(CONFTTEST_CONTENTS "VERS_1 {\n global: sym;\n\n};\n\nVERS_2 {\n global: sym;\n} VERS_1;")
file(WRITE ${PROJECT_BINARY_DIR}/${CMAKE_FILES_DIRECTORY}/conftest.map "${CONFTTEST_CONTENTS}") |
Off-topic to this ticket: would the existing wavpack cmake'ry work when embedding in other projects? I.e.: I might cook wavpack support to our SDL_mixer (hopefully soon (TM)). |
I believe that embedding in other projects was part of the original intent. WavPack was recently added to Audacity, and I believe that's how they did it (but really have only a passing knowledge). On-topic, the fix above eliminates the warning message (which occurs with regular CC also). However, apparently it wasn't the cause of the error return. It still fails there, this time with no syntax warning message:
So @sezero , I assume this step passes in your setup? I thought this would be easy considering that you were doing basically the same thing, but it's certainly not critical to what I'm doing (I still use Autotools for day-to-day work). |
I'll fork and take a looksy. |
With #147, I can build wavpack targeting Linux and MinGW, on my Linux machine. |
Me too, including the winamp and cooledit plugins. Thanks so much! |
Well, I wasn't hitting any errors, because I was setting
However, #147 fixes that for me. One thing to note here: COMPILER_SUPPORTS_SYMBOL_MAPS needn't (shouldn't) be |
Unless the existing behavior is intentional (I can't imagine why that would be), diff --git a/CMakeLists.txt b/CMakeLists.txt
index b5f5cee..9d1d5f5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -328,7 +328,8 @@ set(WAVPACK_EXPORT_SYMBOLS
WavpackWriteTag
)
-if(BUILD_SHARED_LIBS AND (WIN32 OR CYGWIN))
+if(BUILD_SHARED_LIBS)
+ if(WIN32 OR CYGWIN)
set(FILE_CONTENTS "EXPORTS\n")
foreach(EXPORT_SYMBOL ${WAVPACK_EXPORT_SYMBOLS})
list(APPEND FILE_CONTENTS " ${EXPORT_SYMBOL}\n")
@@ -336,8 +337,7 @@ if(BUILD_SHARED_LIBS AND (WIN32 OR CYGWIN))
file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/libwavpack.def ${FILE_CONTENTS})
target_sources(wavpack PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/libwavpack.def)
-
-elseif(CMAKE_SYSTEM_NAME MATCHES "Darwin")
+ elseif(CMAKE_SYSTEM_NAME MATCHES "Darwin")
set(FILE_CONTENTS "")
foreach(EXPORT_SYMBOL ${WAVPACK_EXPORT_SYMBOLS})
list(APPEND FILE_CONTENTS "_${EXPORT_SYMBOL}\n")
@@ -348,7 +348,7 @@ elseif(CMAKE_SYSTEM_NAME MATCHES "Darwin")
else()
target_link_directories(wavpack PRIVATE "-Wl,-exported_symbols_list,${CMAKE_CURRENT_BINARY_DIR}/libwavpack.sym")
endif()
-else()
+ else()
set(CONFTTEST_CONTENTS "VERS_1 {\n global: sym;\n};\n\nVERS_2 {\n global: sym;\n} VERS_1;\n")
file(WRITE ${PROJECT_BINARY_DIR}/${CMAKE_FILES_DIRECTORY}/conftest.map "${CONFTTEST_CONTENTS}")
check_c_linker_flag("-Wl,--version-script=${PROJECT_BINARY_DIR}/${CMAKE_FILES_DIRECTORY}/conftest.map" COMPILER_SUPPORTS_SYMBOL_MAPS)
@@ -366,6 +366,7 @@ else()
target_link_options(wavpack PRIVATE "-Wl,--version-script=${CMAKE_CURRENT_BINARY_DIR}/libwavpack.map;-Wl,-no-undefined")
endif()
endif()
+ endif()
endif()
set_target_properties(wavpack PROPERTIES EXPORT_NAME WavPack) |
That sounds correct to me. |
@dbry: The patch is above, but if you want a PR I can create one. |
@sezero That's all right...I can push the change, unless you want the credit... 😺 The purpose of this exercise for me was to verify the operation of the plugins, and unfortunately I have found that neither of them work. I noticed when the cooledit filter is building I got a whole batch of these warnings which look to me like somehow the Pascal calling convention is not happening correctly:
When cooledit starts I get this message: I don't get any warnings during the build step, but the winamp plugin fails to load also: One difference on the winamp plugin is that the
whereas the one built now shows:
Also, I believe that the However, I mention this only for completeness, This is not at all important to me because both of these plugins are rather obsolete and I can still build them on MSVC if there's ever a need to do so again. I also don't have enough time or interest (or knowledge) to figure out what might be happening. I am actually more concerned about building non-functioning plugins that someone might try to use (although that's also unlikely). At a minimum I think I'll put a note somewhere that plugins built with cmake do not work. |
Add diff --git a/CMakeLists.txt b/CMakeLists.txt
index b5f5cee..62d27db 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -512,6 +512,9 @@ if(WAVPACK_BUILD_COOLEDIT_PLUGIN)
PREFIX ""
)
target_link_libraries(cool_wv4 PRIVATE wavpack)
+ if(MINGW)
+ target_link_libraries(cool_wv4 PRIVATE -mwindows)
+ endif()
endif()
@@ -530,6 +533,9 @@ if(WAVPACK_BUILD_WINAMP_PLUGIN)
PREFIX ""
)
target_link_libraries(in_wv PRIVATE wavpack)
+ if(MINGW)
+ target_link_libraries(in_wv PRIVATE -mwindows)
+ endif()
add_library(in_wv_lng MODULE
winamp/wavpack.rc
@@ -541,6 +547,9 @@ if(WAVPACK_BUILD_WINAMP_PLUGIN)
SUFFIX ".lng"
LINKER_LANGUAGE "C"
)
+ if(MINGW)
+ target_link_libraries(in_wv_lng PRIVATE -mwindows)
+ endif()
endif()
|
Nope, don't want or need it.
Does the following patch help at all? (Either that, or you can remove the the exported symbols list from cmake and msvc project and mark every exported symbol with diff --git a/CMakeLists.txt b/CMakeLists.txt
index b5f5cee..00ff7a7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -473,22 +473,22 @@ endif()
if(WAVPACK_BUILD_COOLEDIT_PLUGIN)
set(WAVPACK_COOLEDIT_PLUGIN_EXPORT_SYMBOLS
- QueryCoolFilter
- FilterUnderstandsFormat
- GetSuggestedSampleType
- OpenFilterInput
- FilterGetFileSize
- ReadFilterInput
- CloseFilterInput
- FilterOptions
- FilterOptionsString
- OpenFilterOutput
- CloseFilterOutput
- WriteFilterOutput
- FilterGetOptions
- FilterWriteSpecialData
- FilterGetFirstSpecialData
- FilterGetNextSpecialData
+ QueryCoolFilter@4
+ FilterUnderstandsFormat@4
+ GetSuggestedSampleType@12
+ OpenFilterInput@24
+ FilterGetFileSize@4
+ ReadFilterInput@12
+ CloseFilterInput@4
+ FilterOptions@4
+ FilterOptionsString@8
+ OpenFilterOutput@28
+ CloseFilterOutput@4
+ WriteFilterOutput@12
+ FilterGetOptions@24
+ FilterWriteSpecialData@20
+ FilterGetFirstSpecialData@8
+ FilterGetNextSpecialData@8
)
set(FILE_CONTENTS "EXPORTS\n")
That really shouldn't make a difference..
That in_wv.lng thing only builds in the resource, so I really don't understand its purpose. There is an MSVC project for it, and the MSVC project for the dll itself doesn't include the resource, which is weird. I don't know anything about winamp or its plugins so I can't be of help with it... |
I am also seeing these warnings. MinGW does What is the difference when running Instead of using a #if defined(AUDITON_EXPORTS)
#define AUDITION_EXTERN __declspec(dllexport)
#else
#define AUDITION_EXTERN
#endif and use it as: AUDITION_EXTERN short PASCAL QueryCoolFilter (COOLQUERY *lpcq); This would work for both MSVC and CMake. |
The attached patch removes exported symbol lists from cmake and msvc project file, and marks exported syms explicitly with EDIT: updated patch to use macros suggested by @madebr |
The lng file thing seems to be used this way: https://github.com/dbry/WavPack/blob/master/winamp/in_wv.c#L85-L104 |
First of all, many thanks to @madebr and @sezero for your help here! I have pushed the first two patches above and they all seem fine. I can now build working executables in Unfortunately neither of the patches to fix the exports generates workable plugin files. I checked the exports of the mingw-generated file with I also read here that MSVC can change the decoration depending on whether the symbols come from a .def file or the __declspec(dllexport) attribute (what?). So maybe that's why the MSVC project does it the way it does, and in fact when I applied the second patch and built in MSVC the symbol names got the trailing numeric and a preceding underscore added, and so again, that didn't work. At this point I'm not even curious as to why this is so messed up, and am just happy that I can still build working plugins in MSVC! 😃 |
Yes, it's starting to come back to be (that was a loooong time ago). I think the |
Actually you need define here and no ifdefs. You never build Cool Edit plugin as static library and nobody includes its header somewhere and link it using import library. |
I guess Cool Edit plugin uses Found this snippet: https://raw.githubusercontent.com/Microsoft/vcpkg/master/ports/libssh/0002-mingw_for_Android.patch if (MINGW AND NOT ANDROID)
target_link_libraries(ssh PRIVATE "-Wl,--enable-stdcall-fixup")
target_compile_definitions(ssh PRIVATE "_POSIX_SOURCE")
endif () @dbry , just use native platform tools, eg Visual Studio for Windows. And here: https://sourceware.org/binutils/docs-2.18/ld/Options.html
|
If you don't have a badly written configure.ac, all you have to do is using the |
|
I am getting the following errors from current git master:
If I apply the following patch fixes, cmake configuration succeeds: diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1f7eab..69916fa 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -513,6 +513,7 @@ if(WAVPACK_BUILD_COOLEDIT_PLUGIN)
DEFINE_SYMBOL AUDITION_EXPORTS
SUFFIX ".flt"
PREFIX ""
+ LINKER_LANGUAGE "C"
)
if(MINGW)
target_link_libraries(cool_wv4 PRIVATE -mwindows -static-libgcc "-Wl,--enable-stdcall-fixup")
@@ -536,6 +537,7 @@ if(WAVPACK_BUILD_WINAMP_PLUGIN)
set_target_properties(in_wv PROPERTIES
DEFINE_SYMBOL WINAMP_EXPORTS
PREFIX ""
+ LINKER_LANGUAGE "CXX"
)
if(MINGW)
target_link_libraries(in_wv PRIVATE -mwindows -static-libgcc -static-libstdc++) ... BUT none of the objects of either plugins are built, and it simply
|
In general, when you modify your cmake toolchain you need to remove This error looks like an error with detection of the C/CXX compiler. |
I build in a separate directory, and at every attempt, I just remove and recreate that directory. The main library dll and the exes are built OK, so I don't think there is some bad detection. |
Care to share your cmake toolchain + arguments of cmake? |
I found that 8ff1f86 (and 6b20158) are to blame, which add diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1f7eab..ded1a74 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -504,7 +504,6 @@ if(WAVPACK_BUILD_COOLEDIT_PLUGIN)
audition/filters.h
audition/resource.h
audition/wavpack.rc
- $<TARGET_OBJECTS:wavpack>
${CMAKE_CURRENT_BINARY_DIR}/audition/cool_wv4.def
)
target_include_directories(cool_wv4 PRIVATE include)
@@ -514,6 +513,7 @@ if(WAVPACK_BUILD_COOLEDIT_PLUGIN)
SUFFIX ".flt"
PREFIX ""
)
+ target_link_libraries(cool_wv4 PRIVATE wavpack)
if(MINGW)
target_link_libraries(cool_wv4 PRIVATE -mwindows -static-libgcc "-Wl,--enable-stdcall-fixup")
endif()
@@ -529,7 +529,6 @@ if(WAVPACK_BUILD_WINAMP_PLUGIN)
winamp/wavpack.rc
winamp/wasabi/wasabi.cpp
winamp/wasabi/wasabi.h
- $<TARGET_OBJECTS:wavpack>
)
target_include_directories(in_wv PRIVATE include)
target_compile_definitions(in_wv PRIVATE $<$<BOOL:${MSVC}>:_CRT_SECURE_NO_DEPRECATE>)
@@ -537,6 +536,7 @@ if(WAVPACK_BUILD_WINAMP_PLUGIN)
DEFINE_SYMBOL WINAMP_EXPORTS
PREFIX ""
)
+ target_link_libraries(in_wv PRIVATE wavpack)
if(MINGW)
target_link_libraries(in_wv PRIVATE -mwindows -static-libgcc -static-libstdc++)
endif() |
I see. |
@dbry: I suggest applying the above revert patch (or bumping the cmake requirement to something minimum that works with existing cmake'ry.) |
BTW: Have I ever mentioned that cmake is evil? |
The frontend is something to have mixed feelings about, but its cross platform backend is really lovable. |
FYI, we use 3.0 as minimum required CMake version for SDL2, 3.16 for SDL_mixer. I haven't seen any complaints about lack of support for older CMake versions in the past 6 months. |
I verified that this builds and the Cooledit and Winamp plugins run fine (and with the asm mods I made run just as fast as the MSVC versions). Applied. Thanks all! |
Great. And I think all issues in this ticket are resolved. |
Out of curiosity, does the attached plugin builds (using open watcom) work properly? |
On the Cooledit one the exported symbols have the The Winamp one does have the correct symbols but it causes an immediate crash when loaded. Which could mean that the functions have the correct name but wrong calling convention, or something else bad about the code. The dependencies on both look fine (just |
Can you test this new one then? cool_wv.zip
Hmph.. Or possibly some c++ thing. Will try understanding later. |
Yes, the Cooledit filter works now! And it's very fast...I assume it has the asm code? |
Cool
Yep. |
It's really possible that it's some incompatibility thing with c++ code (that |
That |
Does this winamp plugin build work? It's built after removing all wasabi stuff which includes the albumart procs, so it's C-only now: if it works then it's watcom + c++ fault. If not, well... |
That one works, but it doesn't show the album art. I don't understand how (or why) that wasabi stuff does what it does. But it is how album art is displayed, and is set up and working, and was supposed to handle setting and deleting the album art from files too (but that is disabled). It's funny because my friend who wrote Monkey's Audio recently (6 months ago) was asking me how to add album art to his plugin (because it's not documented anywhere). He ended up copying my wasabi code (which I got when I was working with winamp developers, and they said it was okay for me to redistribute it). They never said I could modify it, and until you came along I hadn't, but I'm sure it's okay now. Anyway, I looked at the exported symbols and there's nothing different, so I have no idea how winamp knows that the album art stuff is in there. I even created a stripped version of my working plugin so that no other symbols were visible, and it still would load the album art. Would be curious how that works. |
OK, so it's definitely some incompatibility watcom has with c++ stuff, so I won't bother with it any longer unless I have a clear understanding of where the problem is and how. |
One last time: This one is linked with dead code elimination disabled. I'm not that hopeful, but looking at the map file, maybe .. ??.. |
Okay, took a quick look and I see that the winamp plugin calls Wasabi_Init() when it's init() is called. And looking there I can see that the wasabi code is talking to winamp through some sort of Windows IPC (using But why? 😃 |
Instant crash on starting winamp, just like the first one. I see you must have known about Oh, and if you get really curious... 😄 |
Well, it's possibly addressing the wrong offsets for class members. Will try looking at it in future. I wish wasabi thing had a simple C-only interface, but whatever. |
Since there have been numerous changes to the cmake stuff here, including the ability to build the winamp plugin for Window using mingw, I thought I would experiment with this (if for no other reason to test the generated winamp plugin and see if it actually works). I have mingw installed and use it to generate Windows executables all the time (by itself, or with Autotools to build WavPack), so I thought this would be fairly straightforward.
Initially I had no idea how to start so I Googled and found this page. I tried that and got pretty far, but it fails with both mingw versions here:
Figuring Google would rescue me again I Googled
COMPILER_SUPPORTS_SYMBOL_MAPS
but found to my surprise that the only place on the whole Internet where that string exists is myCMakeLists.txt
, which of course means I'm stuck.Could anyone kindly put me on the right track? @evpobr @sezero
Also, is there an easy way to switch to clang?
Thanks!
The text was updated successfully, but these errors were encountered: