Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows cross-compilation is broken #1201

Closed
nurupo opened this issue Sep 24, 2018 · 3 comments
Closed

Windows cross-compilation is broken #1201

nurupo opened this issue Sep 24, 2018 · 3 comments
Assignees
Labels
P1 High priority
Milestone

Comments

@nurupo
Copy link
Member

nurupo commented Sep 24, 2018

Looks like #1191 broke Windows cross-compilation builds, even though we tried our best not to break them.

https://travis-ci.org/TokTok/c-toxcore/builds/432322743

Scanning dependencies of target random_testing
make[2]: *** No rule to make target 'sodium_LIBRARY_RELEASE-NOTFOUND', needed by 'random_testing.exe'.  Stop.
make[2]: *** Waiting for unfinished jobs....
make[2]: *** No rule to make target 'sodium_LIBRARY_RELEASE-NOTFOUND', needed by 'auto_file_saving_test.exe'.  Stop.
make[2]: *** Waiting for unfinished jobs....
@nurupo nurupo added the P1 High priority label Sep 24, 2018
@nurupo nurupo added this to the v0.2.x milestone Sep 24, 2018
@nurupo nurupo changed the title Windows cross-compilation builds broken Windows cross-compilation is broken Sep 24, 2018
@nurupo
Copy link
Member Author

nurupo commented Sep 24, 2018

Alright, so here is a summary of what is going on. CMake tries to find a shared library first.

# Try to find both static and shared variants of sodium
set(sodium_USE_STATIC_LIBS OFF)
find_package(sodium)

Then it checks if sodium target exists, under the assumption that the target not existing means that the static libsodium was not found. If the static libsodium was not found, it tries to find a static libsodium.

if (NOT TARGET sodium)
set(sodium_USE_STATIC_LIBS ON)
find_package(sodium REQUIRED)
endif()

Our Windows builds use static libsodium, so you'd expect the first find_package(sodium) to fail, the if-condition evaluate to True and then the second find_package(sodium) to succeed. However, what is happening is that the if-condition is always false, as /cmake/Findsodium.cmake always creates the sodium target, no matter if it failed to find the library or not.

add_library(sodium ${_LIB_TYPE} IMPORTED)

The proper way to check if shared libsodium was not found would be

if (NOT sodium_FOUND)

instead of checking if sodium target was created.

(Interestingly enough, @sphaerophoria did write the proper check the first time around, but then changed it to the broken one because the proper one was causing issues on Ubuntu 14.04 for them.)

However, this proper check exposes another issue, while the the second find_package(sodium) call does properly find the static libsodium, it errors out when trying to create the sodium target, because it already exists, created by the first find_package(sodium) call:

-- Could NOT find sodium (missing:  sodium_LIBRARY_RELEASE sodium_LIBRARY_DEBUG) 
-- Found sodium: /root/prefix/i686/lib/libsodium.a  
CMake Error at cmake/Findsodium.cmake:251 (add_library):
  add_library cannot create imported target "sodium" because another target
  with the same name already exists.
Call Stack (most recent call first):
  cmake/Dependencies.cmake:31 (find_package)
  CMakeLists.txt:123 (include)

This situation with /cmake/Findsodium.cmake is a mess, it doesn't support being called multiple times, which is what we need in order to emulate the previous behavior of searching for shared libsodium and then falling back to static. We should either fix /cmake/Findsodium.cmake to allow for such behavior or revert f87f871.

@nurupo
Copy link
Member Author

nurupo commented Sep 24, 2018

This inconsistency in fallback on Linux and Windows is annoying, opened the issue with Findsodium.cmake's upstream jedisct1/libsodium#757 just so they know about it.

@sphaerophoria
Copy link

ill roll back the change tonight

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority
Development

No branches or pull requests

3 participants