raspberrypi / picotool

BSD 3-Clause "New" or "Revised" License
525 stars 86 forks source link

MSYS2 / MinGW support #61

Closed DavidEGrayson closed 1 year ago

DavidEGrayson commented 1 year ago

I like to use MSYS2 and its MinGW GCC toolchain for all my open source work; it really is a pleasure because it lets you compile native Windows software from an environment with all the usual Linux utilities, where everything works almost like Linux does. With MSYS2, you don't have to do a bunch of special work to get your code compiling on Windows.

This pull request makes all the changes necessary to get this building with MinGW in MSYS2. I don't think it will break anything for users of MSVC, but I haven't tested it with MSVC.

kilograham commented 1 year ago

cool thx, we'll obviously try it on Windows before building which means pain/delay :-)

ndabas commented 1 year ago

I don't think these changes are needed at all, I've been building picotool on Windows with MSYS2 + MinGW GCC just fine with the standard build commands:

pacman -S --noconfirm --needed autoconf automake mingw-w64-x86_64-cmake git libtool make mingw-w64-x86_64-ninja mingw-w64-x86_64-toolchain mingw-w64-x86_64-libusb mingw-w64-x86_64-hidapi pkg-config wget

cd picotool
mkdir -p build
cd build
cmake -G Ninja .. -DCMAKE_BUILD_TYPE=Release
cmake --build .

The first pacman command might have some stuff which isn't needed by picotool, I'm using the build environment for a few other things as well. (It could also be cleaned up with pacboy or $MINGW_PACKAGE_PREFIX.)

DavidEGrayson commented 1 year ago

@ndabas Thanks for the info. I'm curious how your CMake managed to find libusb. For me WIN32 is defined in CMake because we are building a native Windows program, so it skips the code path that would cause it to use pkg-config to find libusb-1.0, but using pkg-config is what it needs to do.

Here is a shell session from an MSYS2 MINGW64 shell showing me attempting to follow your build instructions. I just updated all my MSYS2 packages before doing this.

$ pacman -S --needed autoconf automake mingw-w64-x86_64-cmake git libtool make mingw-w64-x86_64-ninja mingw-w64-x86_64-toolchain mingw-w64-x86_64-libusb mingw-w64-x86_64-hidapi pkg-config wget
warning: autoconf-wrapper-20221207-1 is up to date -- skipping
warning: automake-wrapper-20221207-1 is up to date -- skipping
warning: mingw-w64-x86_64-cmake-3.25.2-1 is up to date -- skipping
warning: git-2.39.1-1 is up to date -- skipping
warning: libtool-2.4.7-3 is up to date -- skipping
warning: make-4.4-1 is up to date -- skipping
warning: mingw-w64-x86_64-ninja-1.11.1-3 is up to date -- skipping
warning: mingw-w64-x86_64-binutils-2.40-1 is up to date -- skipping
warning: mingw-w64-x86_64-crt-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-ada-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-fortran-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-libgfortran-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-libs-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gcc-objc-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-gdb-12.1-5 is up to date -- skipping
warning: mingw-w64-x86_64-gdb-multiarch-12.1-5 is up to date -- skipping
warning: mingw-w64-x86_64-headers-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-libgccjit-12.2.0-10 is up to date -- skipping
warning: mingw-w64-x86_64-libmangle-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-libwinpthread-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-make-4.4-2 is up to date -- skipping
warning: mingw-w64-x86_64-pkgconf-1~1.8.0-2 is up to date -- skipping
warning: mingw-w64-x86_64-tools-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-winpthreads-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-winstorecompat-git-10.0.0.r216.gca58fc56b-1 is up to date -- skipping
warning: mingw-w64-x86_64-libusb-1.0.26-1 is up to date -- skipping
warning: mingw-w64-x86_64-hidapi-0.13.1-2 is up to date -- skipping
warning: pkgconf-1.9.4-1 is up to date -- skipping
warning: wget-1.21.3-2 is up to date -- skipping
 there is nothing to do

$ pkg-config libusb-1.0 --libs --cflags
-IC:/msys64/mingw64/include/libusb-1.0 -lusb-1.0

$ which cmake
/mingw64/bin/cmake

$ cd Documents/picotool/

$ git log -1
commit 03f28122cc170535f35e5bf8fa37be024489f5eb (HEAD -> master, origin/master, origin/HEAD, mine/master, david/master)
Author: akhil harihar <akhilharihar@users.noreply.github.com>
Date:   Thu Dec 2 19:58:59 2021 +0530

    fix README.md incorrect cmake function name

$ git diff

$ mkdir build

$ cd build

$ export PICO_SDK_PATH=~/Documents/pico-sdk

$ cmake -G Ninja .. -DCMAKE_BUILD_TYPE=Release
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/msys64/mingw64/bin/c++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
Using PICO_SDK_PATH from environment ('C:/Users/david/Documents/pico-sdk')
-- Could NOT find LIBUSB (missing: LIBUSB_INCLUDE_DIR)
CMake Error at CMakeLists.txt:34 (message):
  picotool cannot be built because libUSB is not found

-- Configuring incomplete, errors occurred!
See also "C:/Users/david/Documents/picotool/build/CMakeFiles/CMakeOutput.log".
ndabas commented 1 year ago

@DavidEGrayson I found the missing piece: I had added this line to my build script, which should make it all work:

export LIBUSB_ROOT="/mingw64"
DavidEGrayson commented 1 year ago

OK. I think it's better to use pkg-config to find libraries instead of requiring users to add environment variables, hence the pull request. (But for MSVC users, my pull request should preserve the old behavior of ignoring pkg-config.)

Even if we convince the CMake scripts to find libusb, there are still some problems in the code:

$ export LIBUSB_ROOT=/mingw64
$ cmake -G Ninja .. -DCMAKE_BUILD_TYPE=Release
Using PICO_SDK_PATH from environment ('C:/Users/david/Documents/pico-sdk')
-- Found LIBUSB: C:/msys64/mingw64/lib/libusb-1.0.dll.a
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/david/Documents/picotool/build
$ cmake --build .
[3/4] Building CXX object CMakeFiles/picotool.dir/main.cpp.obj
C:/Users/david/Documents/picotool/main.cpp:57: warning: "ERROR_CANCELLED" redefined
   57 | #define ERROR_CANCELLED -10
      |
In file included from C:/msys64/mingw64/include/winbase.h:2811,
                 from C:/msys64/mingw64/include/windows.h:70,
                 from C:/msys64/mingw64/include/libusb-1.0/libusb.h:64,
                 from C:/Users/david/Documents/picotool/picoboot_connection/picoboot_connection.h:13,
                 from C:/Users/david/Documents/picotool/picoboot_connection/picoboot_connection_cxx.h:10,
                 from C:/Users/david/Documents/picotool/main.cpp:30:
C:/msys64/mingw64/include/winerror.h:418: note: this is the location of the previous definition
  418 | #define ERROR_CANCELLED __MSABI_LONG(1223)
      |
[4/4] Linking CXX executable picotool.exe

This warning is fixed in my pull request, by one of the lines I change in main.cpp. The author of that file was using MSC_VER to detect a Windows environment, but many of the things they were using it for also apply to MSYS2.

ndabas commented 1 year ago

@DavidEGrayson agreed, this PR is the clean way of doing it.