dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
346 stars 65 forks source link

distribution source tarball related updates #169

Closed sezero closed 8 months ago

sezero commented 8 months ago

CC: @madebr, as he may have better solution for cmake.

madebr commented 8 months ago

I've not yet seen a good source tarball generation script using CMake. CMake can do it with ninja package_source, but I explicitly disabled it for SDL3 because it doesn't allow sufficient control.

dbry commented 8 months ago

Wouldn't is be easier to just make the default OFF by replacing the ${WAVPACK_ROOTPROJECT} with OFF. I tried this and it seemed to do what makes sense. Building those plugins is so weird that someone should have to do it intentionally, IMHO.

Also, checking for the source files seemed to generate this warning. Is this what you were talking about?

CMake Warning (dev) at /usr/share/cmake-3.22/Modules/CMakeDependentOption.cmake:84 (message):
  Policy CMP0127 is not set: cmake_dependent_option() supports full Condition
  Syntax.  Run "cmake --help-policy CMP0127" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.
Call Stack (most recent call first):
  CMakeLists.txt:154 (cmake_dependent_option)
This warning is for project developers.  Use -Wno-dev to suppress it.
sezero commented 8 months ago

Wouldn't is be easier to just make the default OFF by replacing the ${WAVPACK_ROOTPROJECT} with OFF.

Well, what the heck, I just did that and force-pushed the PR.

I've not yet seen a good source tarball generation script using CMake. CMake can do it with ninja package_source, but I explicitly disabled it for SDL3 because it doesn't allow sufficient control.

I wasn't talking about generating a src tarball using cmake. I was talking about using a src tarball generated by autotools which doesn't include certain win32 stuff (of which some I added with the first patch in this PR, and the rest being the two plugins.)

I tried this and it seemed to do what makes sense. Building those plugins is so weird that someone should have to do it intentionally, IMHO.

Also, checking for the source files seemed to generate this warning. Is this what you were talking about?

Nope. This:

``` -- Configuring done CMake Error at CMakeLists.txt:550 (add_library): Cannot find source file: winamp/wavpack.rc Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx CMake Error at CMakeLists.txt:530 (add_library): Cannot find source file: winamp/in_wv.c Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx CMake Error at CMakeLists.txt:508 (add_library): Cannot find source file: audition/cool_wv4.c Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx CMake Error at CMakeLists.txt:550 (add_library): No SOURCES given to target: in_wv_lng CMake Error at CMakeLists.txt:530 (add_library): No SOURCES given to target: in_wv CMake Error at CMakeLists.txt:508 (add_library): No SOURCES given to target: cool_wv4 CMake Generate step failed. Build files cannot be regenerated correctly. ```
sezero commented 8 months ago

Additionally pushed 57c1452 after wvtest threading changes.

madebr commented 8 months ago

I wasn't talking about generating a src tarball using cmake. I was talking about using a src tarball generated by autotools which doesn't include certain win32 stuff (of which some I added with the first patch in this PR, and the rest being the two plugins.)

Why would you want to exclude win32 stuff from the source tarball? You want the tarball to contain everything, also see https://github.com/dbry/WavPack/pull/167#issuecomment-1761373598.

sezero commented 8 months ago

Why would you want to exclude win32 stuff from the source tarball?

I don't (If the question is directed at me), which is what this PR is all about.....

dbry commented 8 months ago

Okay, I think this is great now. Thanks to you both!

cmake: build win32 cooledit and winamp plugings only if the folder exist Otherwise, cmake script fails at configuration step when targeting win32

@sezero my confusion here was I misread this to indicate that the script would still fail if the folder didn't exist (but of course you meant that this fixes the script failing).

In any event, I prefer the current solution. Nobody should build those obsolete plugins unless they know exactly what they're doing (especially the 64-bit versions).

But, I did use this to build a new version of the Cool Edit plugin that took advantage of the multi-threading stuff, and it made a significant speed-up. I still appreciate the ability to build those without firing up a Windows VM (I run Cool Edit under Wine).