[speex] init port #958 #1357
[speex] init port #958 #1357
Conversation
|
@atkawa7, |
|
Thanks for the PR! I simplified the CMakeListst.txt as in the other PRs, however I noticed you haven't filled out the version and description. Could you add those? Otherwise, looks good to me! |
ports/speex/portfile.cmake
Outdated
| if (VCPKG_LIBRARY_LINKAGE STREQUAL dynamic) | ||
| message(STATUS "Speex doesn't support visibility on Win32 yet. Building static.") | ||
| message(STATUS "See https://github.com/xiph/speex/blob/master/win32/config.h for more") | ||
| set(VCPKG_LIBRARY_LINKAGE static) |
There was a problem hiding this comment.
Upstream PR supports dynamic build on Windows with the help of a .def file: https://github.com/xiph/speex/blob/master/win32/libspeex.def.
Maybe we could do the same? Alternatively (.def file wasn't touched in like 10 years, so it may not be up to date...) we could use CMake's autoexport: -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON
There was a problem hiding this comment.
The def file would be ideal.
There was a problem hiding this comment.
Thanks @codicodi I am new to this.
Upstream CMake PR supports dynamic build on Windows with the help of a .def file:
So your guidance on this one might be really appreciated
There was a problem hiding this comment.
Just add win32/libspeex.def to library sources. CMake will automatically pass it to the linker in shared build, so that the linker will know what symbols to export.
|
@ras0219-msft Thanks. I have added the descriptions |
|
Worked like charm
@codicodi @ras0219-msft Changes added |
ports/speex/CONTROL
Outdated
| @@ -0,0 +1,3 @@ | |||
| Source: speex | |||
| Version: 2017-06-28-cae5026cfd88782c7051af6e685059223578b7e9 | |||
There was a problem hiding this comment.
Why use specific commit instead of an official tag Speex-1.2.0? There doesn't seem to be any meaningful changes between them: xiph/speex@Speex-1.2.0...cae5026
There was a problem hiding this comment.
@codicodi Thanks for the suggestion. I agree that using official tag is better in this case. I hadn't taken notice since I was working in the master.
There was a problem hiding this comment.
@codicodi Thanks. I have added official tags
|
Thanks for the changes! While merging, I simplified the version to just 1.2.0 instead of speex-1.2.0. |
No description provided.