sccn / liblsl

C++ lsl library for multi-modal time-synched data transmission over the local network
Other
115 stars 67 forks source link

MinGW builds require additional libraries #58

Closed samuelpowell closed 3 years ago

samuelpowell commented 4 years ago

Additional libraries are required to enable the build of liblsl, with unit tests, under MinGW. The resultant library and executables test good on x64, but not on x86.

I do not really grok CMakeLists, and I note that you prefer an inline test for MinGW rather than a long form if( MINGW ) in your build tooling, so I am attaching the diffs (which are against the 1.13 release branch) rather than opening a PR.

liblsl_mingw.diff.txt lsl_test_internal_mingw.diff.txt

cboulay commented 4 years ago

Thanks @samuelpowell

I can't test at the moment. For when I do test, can you please tell me what your build environment is? Are you cross compiling? I have Win 10, Ubuntu 18.04, and latest MacOS available to me.

Before I test, can you try linking these additional libs to the lslobj and/or lslboost targets instead of liblsl? I guess then it won't be necessary to link them to lsl_test_internal or any future tests we come up with.

samuelpowell commented 4 years ago

please tell me what your build environment is? Are you cross compiling? I have Win 10, Ubuntu 18.04, and latest MacOS available to me.

That's a complex one!

I am building binaries using a system that's been put together to cross-compile binary dependencies that are used by Julia packages, in particular, my Julia interface to LSL. Please note that there is nothing Julia specific about the resultant outputs, they are just tarballs containing the library and any other binary artefacts, license files, etc.

The system does everything in CI using docker and various root filesystems which provide the cross-compilers. The actual build is executed using this script, the meat of which is:

cd $WORKSPACE/srcdir
cd liblsl-1.13.0
# Add license file
install_license LICENSE
# Link against real time and correct C->C++ library paths on linux
if [[ ${target} == x86_64-linux-* || ${target} == aarch64-linux-* || ${target} == powerpc64le-linux-* ]]; then
    export CXXFLAGS="-lrt"
    export CFLAGS="-lrt -Wl,-rpath-link,/opt/${target}/${target}/lib64"
fi
if [[ ${target} == i686-linux-* || ${target} == arm-linux-* ]]; then
    export CXXFLAGS="-lrt"
    export CFLAGS="-lrt -Wl,-rpath-link,/opt/${target}/${target}/lib"
fi
# Enable C++ 2011 support and patch for MinGW
if [[ ${target} == *-w64-* ]]; then
    atomic_patch -p1 $WORKSPACE/srcdir/liblsl_mingw.diff
    # Line endings are confused on the test internal CMakeLists, fix them before patch
    dos2unix testing/CMakeLists.txt
    atomic_patch -p1 $WORKSPACE/srcdir/lsl_test_internal_mingw.diff
    export CXXFLAGS="-std=c++11"
fi
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=$prefix -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} -DCMAKE_BUILD_TYPE=Release -DLSL_UNIXFOLDERS=1 -DLSL_NO_FANCY_LIBNAME=1 -DLSL_UNITTESTS=1 ../
make -j${nproc}
# We can't run unit-tests as we are cross-compiling
#./lslver
#./testing/lsl_test_internal 
#./testing/lsl_test_exported 
make install

Once built in CI, the resultant binaries are automatically packaged up and made available here.

We're successfully cross-compiling across Linux (x86, x64, ARMv7, Aarch64) with glibc and muscl, FreeBSD, MacOS, Windows (x86, x64t), but only a subset of those are tested, and I know there are problems with some of the builds, in particular Windows x86 and Aarch64. I'll try and fix some of these in time.

Before I test, can you try linking these additional libs to the lslobj and/or lslboost targets instead of liblsl? I guess then it won't be necessary to link them to lsl_test_internal or any future tests we come up with.

I'll have a go at this as soon as time permits and report back. Feel free to ping if you don't hear from me in a week or so.

samuelpowell commented 3 years ago

I can confirm that adding the specified libraries to lslobj works fine. I have tested this on the latest release (1.14) and the library and example programs build and work out of the box on MinGW. The cmake configuration was modified as follows:

target_link_libraries(lslobj
    PRIVATE
        lslboost
        winmm
        bcrypt
        ws2_32
        mswsock
    PUBLIC
        $<$<AND:$<BOOL:${LSL_DEBUGLOG}>,$<PLATFORM_ID:Linux>>:dl>
        $<$<BOOL:${NEEDS_LIBRT}>:rt>
    )
tstenner commented 3 years ago

This will break the build on all other platforms. Right now, the embedded boost subset is linked against bcrypt, mswsock and ws2_32 on windows so is (hopefully) liblsl via the dependency on lslboost. What's missing in your list is iphlpapi, a new dependency for the network interface enumeration.

samuelpowell commented 3 years ago

Thanks @tstenner. I wasn't suggesting adding this verbatim, it was a response to @cboulay's comment regarding adding these link libraries to lslobj rather than individual targets. As per the diff in the original post, wrapping the additional libraries with if(MINGW) ... endif() should be sufficient to avoid breaking anything else, should it not?

With regard to the exact libraries added, I can see that there are some changes on master (this was done on 1.14) which mean this list can be trimmed down.

If you're willing to merge something along these lines I'll happily checkout master, see what else is required, and open a PR.

tstenner commented 3 years ago

Ah, ok. I skipped the discussion from 2020. I'm reasonably sure that the current master doesn't need any changes for MinGW, but I'd be happy to fix this if it's still needed.

samuelpowell commented 3 years ago

No problem. I'll check on master now and close/update as required.

samuelpowell commented 3 years ago

Master builds out of the box, closing. Thanks @tstenner @cboulay .