drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
206 stars 52 forks source link

LLVM-MinGW Build Issues #323

Closed k8wu closed 1 year ago

k8wu commented 1 year ago

First - thanks to @tmiw for incorporating the necessary changes to make this work out of the box via WSL or a Linux virtual machine.

Here are some issues that I noticed with the instructions that were given:

Download LLVM MinGW at https://github.com/mstorsjo/llvm-mingw/releases/tag/20220906. Decompress into your preferred location. For example: tar xvf llvm-mingw-20220906-ucrt-ubuntu-18.04-x86_64.tar.xz Add LLVM MinGW to your PATH: export PATH=/path/to/llvm-mingw-20220906-ucrt-ubuntu-18.04-x86_64/bin:$PATH

Should we explain that the architecture can be something other than "x86_64"? For example, the LLVM-MinGW toolchain is currently available for i686, x86_64, armv7, and aarch64. It's doubtful that anyone's running Windows on a 32-bit ARM device anymore, but the other three would be valid these days.

Create a build folder inside freedv-gui: mkdir build_win64 Run CMake to configure the FreeDV build: cd build_win64 && cmake -DCMAKE_TOOLCHAIN_FILE=${PWD}/../cross-compile/freedv-mingw-llvm-[architecture].cmake .. Valid architectures are: aarch64, armv7, i686, x86_64

The "freedv-mingw-llvm-[architecture].cmake" files, which live under the /cross-compile directory in master, are not present in the latest "v1.8.6-20221217" tag. This will probably be addressed the next time you tag out a release, I would imagine. For now, I tested this process using master instead of any of the tags.

Additionally, because I was trying a build without any of the dependencies already available, I had to add a few options to signal that I wanted static builds of those dependencies. Thus, my configuration command line looked like this instead: cmake ${PWD}/../cross-compile/freedv-mingw-llvm-[architecture].cmake -DUSE_STATIC_SAMPLERATE=ON -DUSE_STATIC_SNDFILE=ON -DBOOTSTRAP_WXWIDGETS=ON ..

Build FreeDV as normal: make

I used make -j$(nproc) to take advantage of the additional CPUs/threads, otherwise this will take forever :)

Also, because I did not have Hamlib already compiled for this architecture, it failed. Perhaps we need to have a way to build that statically as well.

Create FreeDV installer: make package

I didn't get this far.

Because I prefer to use ninja-build, I did try the build process with -GNinja on the cmake command line, but when it got to the end of the configuration step, it failed with this error:

-- Configuring done
-- Generating done
CMake Error:
  Running

   '/usr/bin/ninja' '-C' '/home/mphipps/git/freedv-gui/build_win64' '-t' 'recompact'

  failed with:

   ninja: error: build.ninja:426: bad $-escape (literal $ must be written as $$)

This may be important in the future, since at some point I would guess that people might want to compile using native Windows build tools, which are available for x86, x86_64 (x64), and aarch64 (ARM64) now, and Windows supports using ninja-build for cross-platform projects. I'd imagine that it will be easier to support this than getting it to work using nmake on Windows. For now, getting the process streamlined for WSL/VM builds with Linux is good enough.

tmiw commented 1 year ago

Interesting, I actually didn't need to provide anything extra to cmake other than -DCMAKE_TOOLCHAIN_FILE=... for the build to automatically determine which dependencies to build itself. Can you provide cmake output when you just provide -DCMAKE_TOOLCHAIN_FILE=... and no other arguments? I wonder if it's something with WSL specifically as I was testing on a fairly clean Ubuntu 22.04 image without the distro's MinGW package installed.

k8wu commented 1 year ago

I usually use Debian instead of Ubuntu to minimize bloat, so I decided to start with a brand-new Debian Bullseye WSL installation, and I only installed the following Debian packages:

I then followed the instructions regarding setting up the cross-compiler and putting it in $PATH, cloning the freedv-gui repository, and then creating a build_win64 subdirectory underneath the top-level of my working copy. After that, I executed your requested cmake -DCMAKE_TOOLCHAIN_FILE=../cross-compile/freedv-mingw-llvm-aarch64.cmake .. command, and it was able to properly configure and make, resulting in a complete build. My guess is that it was pulling a Linux version of Hamlib in from somewhere in the old WSL installation, though I didn't see one in there before I wiped it out.

Aside from the other stuff I mentioned before, I noticed that the filename of the installer is FreeDV-1.8.5-win64.exe. This was for an ARM64 build, and it was from the master branch as opposed to any of the v1.8.5 ones. Just FYI - perhaps there needs to be something to signify that an installer is for ARM64 vs x86_64/x64 - and I am not sure what to do about the version numbering.

tmiw commented 1 year ago

I think the installer's file name will be straightforward to update (e.g. by setting CPACK_PACKAGE_FILE_NAME). I'm not so sure about the other known issues; those may require fixes in NSIS and/or CMake. Will experiment a bit and get back to you.

tmiw commented 1 year ago

I added PR #329 to clarify the README per your feedback. I also have a change in there that will result in make package generating an installer named FreeDV-[version]-windows-[architecture].exe. [version] here is derived from the PROJECT_VERSION and FREEDV_VERSION_TAG variables in CMakeLists.txt (which is still 1.8.5 in master as I haven't officially released 1.8.6 yet; 1.8.6 prerelease builds have been coming from the ms-2020c branch, which occasionally pulls down from master). [architecture] comes from the file passed into CMAKE_TOOLCHAIN_FILE.

k8wu commented 1 year ago

I used the branch specified in PR #329 to run through the process for aarch64 and x86_64, and both are demonstrating what you have said in your latest comment. This will certainly make it easier to build and install new versions of the program until the build process is updated to use this new method - if that's the idea?

tmiw commented 1 year ago

I used the branch specified in PR #329 to run through the process for aarch64 and x86_64, and both are demonstrating what you have said in your latest comment. This will certainly make it easier to build and install new versions of the program until the build process is updated to use this new method - if that's the idea?

I think it would be good long term to not have to rely on any special Docker containers or scripts, just running cmake followed by make. I'd like some more runtime before doing that, though, especially since the changes impact non-Windows builds too.

In any case, though, it sounds like the PR is good to merge for now. 👍