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

Using cmake with an alternate toolchain #146

Open
dbry opened this issue Nov 17, 2022 · 84 comments
Open

Using cmake with an alternate toolchain #146

dbry opened this issue Nov 17, 2022 · 84 comments

Comments

@dbry
Copy link
Owner

dbry commented Nov 17, 2022

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:

Performing C SOURCE FILE Test COMPILER_SUPPORTS_SYMBOL_MAPS failed with the following compile output:
Change Dir: /home/david/WavPack/build/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/make cmTC_be63c/fast && /usr/bin/make -f CMakeFiles/cmTC_be63c.dir/build.make CMakeFiles/cmTC_be63c.dir/build
make[1]: Entering directory '/home/david/WavPack/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_be63c.dir/src.c.obj
/usr/bin/x86_64-w64-mingw32-gcc   -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/david/WavPack/build//CMakeFiles/conftest.map   -o CMakeFiles/cmTC_be63c.dir/src.c.obj   -c /home/david/WavPack/build/CMakeFiles/CMakeTmp/src.c
Linking C executable cmTC_be63c.exe
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_be63c.dir/link.txt --verbose=1
/usr/bin/cmake -E remove -f CMakeFiles/cmTC_be63c.dir/objects.a
/usr/bin/x86_64-w64-mingw32-ar cr CMakeFiles/cmTC_be63c.dir/objects.a @CMakeFiles/cmTC_be63c.dir/objects1.rsp
/usr/bin/x86_64-w64-mingw32-gcc  -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/david/WavPack/build//CMakeFiles/conftest.map    -Wl,--whole-archive CMakeFiles/cmTC_be63c.dir/objects.a -Wl,--no-whole-archive  -o cmTC_be63c.exe -Wl,--out-implib,libcmTC_be63c.dll.a -Wl,--major-image-version,0,--minor-image-version,0 @CMakeFiles/cmTC_be63c.dir/linklibs.rsp
/usr/bin/x86_64-w64-mingw32-ld:/home/david/WavPack/build//CMakeFiles/conftest.map:8: ignoring invalid character `\' in script
make[1]: Leaving directory '/home/david/WavPack/build/CMakeFiles/CMakeTmp'

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 my CMakeLists.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!

@evpobr
Copy link
Contributor

evpobr commented Nov 17, 2022

check_c_linker_flag("-Wl,--version-script=${PROJECT_BINARY_DIR}/${CMAKE_FILES_DIRECTORY}/conftest.map" COMPILER_SUPPORTS_SYMBOL_MAPS)

It checks -Wl,--version-script linker flag. I suspect that MinGW GCC does not support this.

Actually correct path is:

if(BUILD_SHARED_LIBS AND (WIN32 OR CYGWIN))

I am almost one hundred percent sure that this should work with MinGW on Windows.

@evpobr
Copy link
Contributor

evpobr commented Nov 17, 2022

/usr/bin/x86_64-w64-mingw32-ld:/home/david/WavPack/build//CMakeFiles/conftest.map:8: ignoring invalid character `' in script

This is also very strange.

@evpobr
Copy link
Contributor

evpobr commented Nov 17, 2022

Run Build Command(s):/usr/bin/make

Also this is strange. Not sure it is correct version. Maybe something is not installed.

@evpobr
Copy link
Contributor

evpobr commented Nov 17, 2022

Probably generator must be MinGW Makefiles.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

found this page.

For what it's worth, this is my toolchain file for cross-compilation purposes:

SET(CMAKE_SYSTEM_NAME Windows)

SET(CMAKE_C_COMPILER /opt/cross_win64/bin/x86_64-w64-mingw32-gcc)
SET(CMAKE_CXX_COMPILER /opt/cross_win64/bin/x86_64-w64-mingw32-g++)
SET(CMAKE_RC_COMPILER /opt/cross_win64/bin/x86_64-w64-mingw32-windres)

# where is the target environment
SET(CMAKE_FIND_ROOT_PATH /opt/cross_win64 /opt/cross_win64/x86_64-w64-mingw32)

# adjust the default behaviour of the FIND_XXX() commands:
# search headers and libraries in the target environment, search
# programs in the host environment
SET(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
SET(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
SET(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

check_c_linker_flag("-Wl,--version-script=${PROJECT_BINARY_DIR}/${CMAKE_FILES_DIRECTORY}/conftest.map" COMPILER_SUPPORTS_SYMBOL_MAPS)

It checks -Wl,--version-script linker flag. I suspect that MinGW GCC does not support this.

It actually should, if I remember correctly

[snipping other stuff]

Could anyone kindly put me on the right track?

I am mentioned, but I don't speak and not proficient in cmake, I don't
like cmake, I detest cmake, [append more here ...]

I don't know whether he'd be interested in, but @madebr did quite a lot
of cmake work in SDL projects, and he is proficient.

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

The error is about an invalid stray \ in conftest.map

/usr/bin/x86_64-w64-mingw32-ld:/home/david/WavPack/build//CMakeFiles/conftest.map:8: ignoring invalid character `\' in script

This file is generated here:

WavPack/CMakeLists.txt

Lines 351 to 352 in ce01373

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}")

I ran these 2 lines locally, and it generated

VERS_1 {
    global: sym\;

};

VERS_2 {
    global: sym;
} VERS_1\;

which looks incorrect.

The string should not escape the semicolons.
When fixing this, perhaps also add a final newline.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

Interestingly, the generated map file I get has no stray \ characters on my linux.
cmake version is 3.9.6, if that matters.

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

No idea why that's happening.
I just downloaded a CMake 3.9.6 binary,
created /tmp/a.cmake with:

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:

cmake -P /tmp/a.cmake

Both 3.9.6 and my system CMake 3.24.2 generated the same conftest.map.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

In any case, removal of escaping semicolons should not hurt.

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

No, the escape is only needed when you need to embed a semicolon as member of a list.
I verified without any escape and it worked on both 3.9.6 and 3.22.4.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

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}")

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

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)).

@dbry
Copy link
Owner Author

dbry commented Nov 17, 2022

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:

Performing C SOURCE FILE Test COMPILER_SUPPORTS_SYMBOL_MAPS failed with the following compile output:
Change Dir: /home/david/WavPack/build/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/make cmTC_48e24/fast && /usr/bin/make -f CMakeFiles/cmTC_48e24.dir/build.make CMakeFiles/cmTC_48e24.dir/build
make[1]: Entering directory '/home/david/WavPack/build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_48e24.dir/src.c.obj
/usr/bin/i686-w64-mingw32-gcc   -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/david/WavPack/build//CMakeFiles/conftest.map   -o CMakeFiles/cmTC_48e24.dir/src.c.obj   -c /home/david/WavPack/build/CMakeFiles/CMakeTmp/src.c
Linking C executable cmTC_48e24.exe
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_48e24.dir/link.txt --verbose=1
/usr/bin/cmake -E remove -f CMakeFiles/cmTC_48e24.dir/objects.a
/usr/bin/i686-w64-mingw32-ar cr CMakeFiles/cmTC_48e24.dir/objects.a @CMakeFiles/cmTC_48e24.dir/objects1.rsp
/usr/bin/i686-w64-mingw32-gcc  -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/david/WavPack/build//CMakeFiles/conftest.map    -Wl,--whole-archive CMakeFiles/cmTC_48e24.dir/objects.a -Wl,--no-whole-archive  -o cmTC_48e24.exe -Wl,--out-implib,libcmTC_48e24.dll.a -Wl,--major-image-version,0,--minor-image-version,0 @CMakeFiles/cmTC_48e24.dir/linklibs.rsp
make[1]: Leaving directory '/home/david/WavPack/build/CMakeFiles/CMakeTmp'

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).

Thanks @madebr and @evpobr !

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

FILE_CONTENTS is also handled weird. It's a string, which is appended as a cmake list.
CMake has string(APPEND), but not in cmake 3.2.

I'll fork and take a looksy.

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

With #147, I can build wavpack targeting Linux and MinGW, on my Linux machine.

@dbry
Copy link
Owner Author

dbry commented Nov 17, 2022

Me too, including the winamp and cooledit plugins. Thanks so much!

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

So @sezero , I assume this step passes in your setup?

Well, I wasn't hitting any errors, because I was setting BUILD_SHARED_LIBS=ON
Without that, I get the following error without #147:

-- Performing Test COMPILER_SUPPORTS_SYMBOL_MAPS
CMake Error: TRY_RUN() invoked in cross-compiling mode, please set the following cache variables appropriately:
   COMPILER_SUPPORTS_SYMBOL_MAPS_EXITCODE (advanced)
For details see /home/ozzie/5/b/TryRunResults.cmake
-- Performing Test COMPILER_SUPPORTS_SYMBOL_MAPS - Failed

CMakeError.log contains this: As you can see, without #147, I do hit that
stray \ warning:

Run Build Command:"/usr/bin/gmake" "cmTC_fb768/fast"
/usr/bin/gmake -f CMakeFiles/cmTC_fb768.dir/build.make CMakeFiles/cmTC_fb768.dir/build
gmake[1]: Entering directory `/home/ozzie/5/b/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_fb768.dir/src.c.obj
/opt/cross_win64/bin/x86_64-w64-mingw32-gcc -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGE_FILES  -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/ozzie/5/b//CMakeFiles/conftest.map   -o CMakeFiles/cmTC_fb768.dir/src.c.ob
Linking C executable cmTC_fb768.exe
/home/ozzie/opt/cmake3.9/bin/cmake -E cmake_link_script CMakeFiles/cmTC_fb768.dir/link.txt --verbose=1
/home/ozzie/opt/cmake3.9/bin/cmake -E remove -f CMakeFiles/cmTC_fb768.dir/objects.a
/opt/cross_win64/bin/x86_64-w64-mingw32-ar cr CMakeFiles/cmTC_fb768.dir/objects.a @CMakeFiles/cmTC_fb768.dir/objects1.rsp
/opt/cross_win64/bin/x86_64-w64-mingw32-gcc  -DCOMPILER_SUPPORTS_SYMBOL_MAPS -Wl,--version-script=/home/ozzie/5/b//CMakeFiles/conftest.map    -Wl,--whole-archive CMakeFiles/cmTC_fb768.dir/objects.a -Wl,--no-whole-archive  -o cmTC_fb768.
/opt/cross_win64/lib/gcc/x86_64-w64-mingw32/4.5.4/../../../../x86_64-w64-mingw32/bin/ld:/home/ozzie/5/b//CMakeFiles/conftest.map:8: ignoring invalid character `\' in script
gmake[1]: Leaving directory `/home/ozzie/5/b/CMakeFiles/CMakeTmp'

Return value: PLEASE_FILL_OUT-FAILED_TO_RUN
Source file was:
int main() { return 0;}

However, #147 fixes that for me.

One thing to note here: COMPILER_SUPPORTS_SYMBOL_MAPS needn't (shouldn't) be
checked at all if we are not building a shared lib. I believe that should be
rectified.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

One thing to note here: COMPILER_SUPPORTS_SYMBOL_MAPS needn't (shouldn't) be checked at all if we are not building a shared lib. I believe that should be rectified.

Unless the existing behavior is intentional (I can't imagine why that would be),
is the following patch draft good?

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)

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

That sounds correct to me.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

That sounds correct to me.

@dbry: The patch is above, but if you want a PR I can create one.

@dbry
Copy link
Owner Author

dbry commented Nov 17, 2022

@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:

/usr/bin/i686-w64-mingw32-ld: warning: resolving _CloseFilterInput by linking to _CloseFilterInput@4
Use --enable-stdcall-fixup to disable these warnings
Use --disable-stdcall-fixup to disable these fixups
/usr/bin/i686-w64-mingw32-ld: warning: resolving _CloseFilterOutput by linking to _CloseFilterOutput@4
/usr/bin/i686-w64-mingw32-ld: warning: resolving _FilterGetFileSize by linking to _FilterGetFileSize@4
...

When cooledit starts I get this message:
CooleditError-126

I don't get any warnings during the build step, but the winamp plugin fails to load also:
WinampError

One difference on the winamp plugin is that the file report is slightly different. The working plugin displays

in_wv.dll: PE32 executable (DLL) (GUI) Intel 80386, for MS Windows

whereas the one built now shows:

in_wv.dll: PE32 executable (DLL) (console) Intel 80386, for MS Windows

Also, I believe that the in_wv.lng gets included in the dll because the .lng file is never referenced in the NSI installation script.

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.

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

Add -mwindows to the link options to change (console) into (GUI):

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()
 

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

unless you want the credit...

Nope, don't want or need it.

Pascal calling convention is not happening correctly:

/usr/bin/i686-w64-mingw32-ld: warning: resolving _CloseFilterInput by linking to _CloseFilterInput@4
Use --enable-stdcall-fixup to disable these warnings
Use --disable-stdcall-fixup to disable these fixups
/usr/bin/i686-w64-mingw32-ld: warning: resolving _CloseFilterOutput by linking to _CloseFilterOutput@4
/usr/bin/i686-w64-mingw32-ld: warning: resolving _FilterGetFileSize by linking to _FilterGetFileSize@4
...

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 __declspec(dllexport))

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")

One difference on the winamp plugin is that the file report is slightly different. The working plugin displays

in_wv.dll: PE32 executable (DLL) (GUI) Intel 80386, for MS Windows

whereas the one built now shows:

in_wv.dll: PE32 executable (DLL) (console) Intel 80386, for MS Windows

That really shouldn't make a difference..

Also, I believe that the in_wv.lng gets included in the dll because the .lng file is never referenced in the NSI installation script.

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...

@madebr
Copy link
Contributor

madebr commented Nov 17, 2022

I am also seeing these warnings.
These are about the name of the symbol names created in the .c files not matching up with the exported names in the .def file.

MinGW does #define PASCAL __stdcall, so these symbols are actually built with stdcall convention.

What is the difference when running dumpbin /exports on the 2 dll's?

Instead of using a .def file and/or adding all these /export:symbol1 /export:symbol2 in the .vcxproj,
I think it is easier to add the following to e.g. audition/filters.h

#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.
And simplifies both build systems imho.

@sezero
Copy link
Contributor

sezero commented Nov 17, 2022

The attached patch removes exported symbol lists from cmake and msvc project file, and marks exported syms explicitly with __declspec(dllexport) (Hopefully I didn't screw up the msvc project file..)

EDIT: updated patch to use macros suggested by @madebr

patch_exports2.txt

@sezero
Copy link
Contributor

sezero commented Nov 18, 2022

Also, I believe that the in_wv.lng gets included in the dll because the .lng file is never referenced in the NSI installation script.

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...

The lng file thing seems to be used this way: https://github.com/dbry/WavPack/blob/master/winamp/in_wv.c#L85-L104

@dbry
Copy link
Owner Author

dbry commented Nov 18, 2022

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 mingw using cmake.

Unfortunately neither of the patches to fix the exports generates workable plugin files. I checked the exports of the mingw-generated file with dumpbin and now the symbols have the numeric decoration at the end (which the originals do not) so I knew they wouldn't work (but did try).

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! 😃

@dbry
Copy link
Owner Author

dbry commented Nov 18, 2022

Also, I believe that the in_wv.lng gets included in the dll because the .lng file is never referenced in the NSI installation script.

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...

The lng file thing seems to be used this way: https://github.com/dbry/WavPack/blob/master/winamp/in_wv.c#L85-L104

Yes, it's starting to come back to be (that was a loooong time ago). I think the in_wv.lng info is in the resource file and so it's in the DLL and that gets used to provide the text for the dialogs. But if there's a in_wv.lng file then there's an attempt to load alternate language strings for the dialogs from there (which is the code you see) that overrides the strings based on the language setting. The file generated can be used to test that facility, but normally it's not there. I don't know if that dialog was ever Internationalized, but if it was then that file would be present. Maybe. But anyway, it doesn't get included with the plugin distribution.

@evpobr
Copy link
Contributor

evpobr commented Nov 18, 2022

#if defined(AUDITON_EXPORTS)
#define AUDITION_EXTERN __declspec(dllexport)
#else
#define AUDITION_EXTERN
#endif

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.

@evpobr
Copy link
Contributor

evpobr commented Nov 18, 2022

I guess Cool Edit plugin uses PASCAL (stdcall) calling convension, but symbols must be undecorated. MS linker resolves this without problems, but not ld.

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

--enable-stdcall-fixup
--disable-stdcall-fixup
If the link finds a symbol that it cannot resolve, it will attempt to do “fuzzy linking” by looking for another defined symbol that differs only in the format of the symbol name (cdecl vs stdcall) and will resolve that symbol by linking to the match. For example, the undefined symbol _foo might be linked to the function _foo@12, or the undefined symbol _bar@16 might be linked to the function _bar. When the linker does this, it prints a warning, since it normally should have failed to link, but sometimes import libraries generated from third-party dlls may need this feature to be usable. If you specify --enable-stdcall-fixup, this feature is fully enabled and warnings are not printed. If you specify --disable-stdcall-fixup, this feature is disabled and such mismatches are considered to be errors. [This option is specific to the i386 PE targeted port of the linker]

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

Isn't this the same behavior as autotools? There, you need to define CC, CXX and AS. If you forget to set one of these

If you don't have a badly written configure.ac, all you have to do is using the --host=CROSS_PREFIX switch on your configure command line, e.g. ./configure --host=i686-w64-mingw32 ...., and the rest is automatically handled. You can set CC, CXX, AS, etc variables only if you want to override the CROSS_PREFIX'ed ones.

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

CMAKE_ASM_COMPILER is detected correctly as the cross compiler. I suppose the dialects are causing issues.

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

I am getting the following errors from current git master:

CMake Error: CMake can not determine linker language for target: cool_wv4
CMake Error: Cannot determine link language for target "cool_wv4".
CMake Error: CMake can not determine linker language for target: in_wv
CMake Error: Cannot determine link language for target "in_wv".

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
outputs empty dlls: WTH???

Scanning dependencies of target cool_wv4
[ 64%] Linking C shared module cool_wv4.flt
[ 64%] Built target cool_wv4
......
Scanning dependencies of target in_wv
[ 89%] Linking CXX shared module in_wv.dll
[ 89%] Built target in_wv

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

In general, when you modify your cmake toolchain you need to remove CMakeCache.txt and reconfigure from scratch.
CMake caches the initial values, and won't use new ones.
This is probably done to avoid mixing cached values of multiple toolchains.

This error looks like an error with detection of the C/CXX compiler.

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

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.

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

Care to share your cmake toolchain + arguments of cmake?

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

Care to share your cmake toolchain + arguments of cmake?

cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=ON -DCMAKE_TOOLCHAIN_FILE=../Toolchain-MinGW32.cmake ../WavPack2

I found that 8ff1f86 (and 6b20158) are to blame, which add $<TARGET_OBJECTS:wavpack> to those plugins' source list and remove explicit linkage to libwavpack. Reverting that (like patch below), I don't need to specify LINKER_LANGUAGE and things build properly.

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()

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

I see.
Up until CMake 3.14, the target specified in $<TARGET_OBJECTS:xxx> needed to be an object library.
So you can revert that commit, knowing that the plug-ins preferably link to a static wavpack library.

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

I see. Up until CMake 3.14, the target specified in $<TARGET_OBJECTS:xxx> needed to be an object library. So you can revert that commit, knowing that the plug-ins preferably link to a static wavpack library.

@dbry: I suggest applying the above revert patch (or bumping the cmake requirement to something minimum that works with existing cmake'ry.)

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

I see. Up until CMake 3.14, the target specified in $<TARGET_OBJECTS:xxx> needed to be an object library.

BTW: Have I ever mentioned that cmake is evil?

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

The frontend is something to have mixed feelings about, but its cross platform backend is really lovable.

@madebr
Copy link
Contributor

madebr commented Nov 20, 2022

@dbry: I suggest applying the above revert patch (or bumping the cmake requirement to something minimum that works with existing cmake'ry.)

FYI, we use 3.0 as minimum required CMake version for SDL2, 3.16 for SDL_mixer.
SDL3 might also make the jump to 3.16.

I haven't seen any complaints about lack of support for older CMake versions in the past 6 months.

@dbry
Copy link
Owner Author

dbry commented Nov 20, 2022

I see. Up until CMake 3.14, the target specified in $<TARGET_OBJECTS:xxx> needed to be an object library. So you can revert that commit, knowing that the plug-ins preferably link to a static wavpack library.

@dbry: I suggest applying the above revert patch (or bumping the cmake requirement to something minimum that works with existing cmake'ry.)

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!

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

Great. And I think all issues in this ticket are resolved.

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

Out of curiosity, does the attached plugin builds (using open watcom) work properly?
If they do I can send a patch.

plugin.zip

@dbry
Copy link
Owner Author

dbry commented Nov 20, 2022

Out of curiosity, does the attached plugin builds (using open watcom) work properly? If they do I can send a patch.

plugin.zip

On the Cooledit one the exported symbols have the @nn decoration, so I got the same error dialog message as above. If you fix that it may work.

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 USER32 and KERNEL32).

@sezero
Copy link
Contributor

sezero commented Nov 20, 2022

On the Cooledit one the exported symbols have the @nn decoration, so I got the same error dialog message as above. If you fix that it may work.

Can you test this new one then? cool_wv.zip

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.

Hmph.. Or possibly some c++ thing. Will try understanding later.

@dbry
Copy link
Owner Author

dbry commented Nov 20, 2022

Yes, the Cooledit filter works now! And it's very fast...I assume it has the asm code?

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

Yes, the Cooledit filter works now!

Cool

And it's very fast...I assume it has the asm code?

Yep.

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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.

Hmph.. Or possibly some c++ thing. Will try understanding later.

It's really possible that it's some incompatibility thing with c++ code (that wasabi thing -- whatever that it's used for, I don't know..)

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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.

Hmph.. Or possibly some c++ thing. Will try understanding later.

It's really possible that it's some incompatibility thing with c++ code (that wasabi thing -- whatever that it's used for, I don't know..)

That wasabi thing seems to be only used by the AlbumArt functions, and unless I'm mistaken those AlbumArt functions doesn't seem referenced anywhere.

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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...
in_wv.zip

@dbry
Copy link
Owner Author

dbry commented Nov 21, 2022

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... in_wv.zip

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.

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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.

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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.
in_wv_2.zip

I'm not that hopeful, but looking at the map file, maybe .. ??..

@dbry
Copy link
Owner Author

dbry commented Nov 21, 2022

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 sendMessage()). So that means that the C++ code does not need to be interoperable because it's just using messages to talk to winamp proper, so it might be possible to debug.

But why? 😃

@dbry
Copy link
Owner Author

dbry commented Nov 21, 2022

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. in_wv_2.zip

I'm not that hopeful, but looking at the map file, maybe .. ??..

Instant crash on starting winamp, just like the first one. I see you must have known about Wasabi_Init() because you would have had to delete those calls to link when yoiu took out the wasabi part.

Oh, and if you get really curious... 😄

Winamp Download Page

@sezero
Copy link
Contributor

sezero commented Nov 21, 2022

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.

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

No branches or pull requests

4 participants