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

Boost.Test 1.65.1, 1.66.0 link problem (Windows) #94

Closed
db4 opened this issue Jan 30, 2018 · 23 comments
Closed

Boost.Test 1.65.1, 1.66.0 link problem (Windows) #94

db4 opened this issue Jan 30, 2018 · 23 comments
Assignees

Comments

@db4
Copy link

db4 commented Jan 30, 2018

Consider the following test example:
conanfile.txt

[requires]
Boost.Test/1.65.1@bincrafters/stable

[generators]
cmake

CMakeLists.txt

project(test CXX)
cmake_minimum_required(VERSION 3.10)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

add_executable(main main.cpp)
target_link_libraries(main CONAN_PKG::Boost.Test)

main.cpp

#define BOOST_TEST_MODULE Test
#include <boost/test/unit_test.hpp>

Trying to build it:

mkdir build
cd build
conan install ..
cmake .. -G "Visual Studio 15 Win64"
cmake --build . --config Release

The last command fails with:

Link:
  C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.12.25827\bin\HostX86\x64\link.exe /ERRORREPORT:QUEUE /O
  UT:"C:\Users\dbely\conan\test-boost\build\bin\main.exe" /INCREMENTAL:NO /NOLOGO C:\.conan\lrp4h4by\1\lib\libboost_prg_exec_monitor.lib C:
  \.conan\lrp4h4by\1\lib\libboost_test_exec_monitor.lib C:\.conan\lrp4h4by\1\lib\libboost_unit_test_framework.lib C:\.conan\678o_6yu\1\lib\
  libboost_container.lib C:\.conan\vlo9eck0\1\lib\libboost_exception.lib C:\.conan\kwmtgrd6\1\lib\libboost_regex.lib C:\.conan\xhm4dv19\1\l
  ib\libboost_timer.lib C:\.conan\9u6x411z\1\lib\libboost_chrono.lib C:\.conan\h46afy7v\1\lib\libboost_system.lib kernel32.lib user32.lib g
  di32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTUAC:"level='asInvoker' uiA
  ccess='false'" /manifest:embed /PDB:"C:/Users/dbely/conan/test-boost/build/bin/main.pdb" /SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMP
  AT /IMPLIB:"C:/Users/dbely/conan/test-boost/build/lib/main.lib" /MACHINE:X64  /machine:x64 main.dir\Release\main.obj
libboost_prg_exec_monitor.lib(cpp_main.obj) : error LNK2019: unresolved external symbol "int __cdecl cpp_main(int,char * * const)" (?cpp_ma in@@YAHHQEAPEAD@Z) referenced in function main [C:\Users\dbely\conan\test-boost\build\main.vcxproj]
C:\Users\dbely\conan\test-boost\build\bin\main.exe : fatal error LNK1120: 1 unresolved externals [C:\Users\dbely\conan\test-boost\build\mai n.vcxproj]

If I edit .vcxproj file to remove libboost_prg_exec_monitor.lib from link libraries, or reorder them to make libboost_test_exec_monitor.lib coming first, it links OK.

@db4 db4 changed the title Boost.Test 1.65.1 link problem (Windows) Boost.Test 1.65.1, 1.66.0 link problem (Windows) Jan 30, 2018
@db4
Copy link
Author

db4 commented Jan 30, 2018

Just tested - the same problem also exists for boost_test 1.66.0

@solvingj
Copy link
Member

Thank you for testing, we will re-test this after we're done with the 1.66.0 set. we are still working on it.

@lasote
Copy link

lasote commented Feb 12, 2018

I have the same issue with my monolithic package (https://github.com/conan-community/conan-boost/issues/80)

Related:

microsoft/vcpkg#2444
microsoft/vcpkg#2057
microsoft/vcpkg@e3dcfcb

Apparently, it works removing from the cpp_info.libs the *exec_monitor libs. I tried it and yes, it works, but honestly, I don't understand why and I don't know what are these *exe_monitor libs about.

Is ok to remove them from libraries to link with?

@grafikrobot Please give me some light 💡

@lasote
Copy link

lasote commented Feb 13, 2018

For what I can understand about the *exec_monitor library, probably I should add an option to the package link_exec_monitor (not affecting the package SHA), defaulted to False. Any feedback? @grafikrobot

@wpalfi
Copy link

wpalfi commented Feb 13, 2018

Same problem for me.
Workaround: replace
#include <boost/test/unit_test.hpp>
by
#include <boost/test/included/unit_test.hpp>
(single header usage variant, see here http://www.boost.org/doc/libs/1_66_0/libs/test/doc/html/boost_test/usage_variants.html)

@solvingj
Copy link
Member

Very interesting... great find! Although.. seems pretty annoying from user perspective. So is this a workaround, or just a nuance in the usage of the library that the user needs to be aware of? Trying to figure out if we should change our package or not.

@SSE4 SSE4 added this to TODO in Boost Modular Feb 13, 2018
@db4
Copy link
Author

db4 commented Feb 14, 2018

Actually this workaround allows to use boost test without linking any library. But boost docs say that "static library usage variant" still can be used, it's not deprecated.

As for *exec_monitor libs, they indeed look obsolete and completely replaced by boost_unit_test_framework lib; boost 1.66 docs don't even mention them. Test execution monitor was documented for the last time in http://www.boost.org/doc/libs/1_33_1/libs/test/doc/index.html, program execution monitor - in http://www.boost.org/doc/libs/1_58_0/libs/test/doc/html/index.html. And if BOOST_TEST_NO_LIB is not defined, only boost_unit_test_framework lib is auto-linked on <boost/test/unit_test.hpp> inclusion, not everything else. So personally I think that excluding *exec_monitor libs from the package would be OK.

@lasote
Copy link

lasote commented Feb 14, 2018

@db4 thanks. I will exclude it in the monocytic recipe and if someone thinks that it is wrong I will be glad to listen.

@wpalfi
Copy link

wpalfi commented Feb 14, 2018

Without knowing too much details: Vcpkg uses a "manual link" directory. Libs in this folder are not linked by the package manager but still available for manual linking (or boost auto linking?). This is useful for optional libraries that may lead to linker errors. I guess you can do something similar with conan?
microsoft/vcpkg#306
microsoft/vcpkg@e3dcfcb#diff-515a4577c3b9ef0e02821212ec9ae5fbR340

@lasote
Copy link

lasote commented Feb 14, 2018

Yes, I'm not removing the library, just not including it in the self.cpp_info.libs list, you could, of course, add it manually.

@solvingj
Copy link
Member

How would one have the path to such a lib with which to link manually/optionally (if it's not in the libs list)? Would this require some extra variable to be captured in the package_info() method, something like libs_optional?

@lasote
Copy link

lasote commented Feb 14, 2018

That is a good question, but until no one tell me that need that unknown exec_monitor library I will just remove it from the list. Otherwise I could add an option (excluded from package_id) to add it.

@wpalfi
Copy link

wpalfi commented Feb 15, 2018

How would one have the path to such a lib with which to link manually/optionally

With cmake it is CONAN_LIB_DIRS_BOOST.TEST

@solvingj
Copy link
Member

I'm saying that if we explicitly remove the exec_monitor lib from the self.cpp_info.libs list , then we won't have any handle to it in generated files. It won't be in that VAR. We would need a separate VAR to expose "optional" libs. I'm not necessarily advocating for a new standard VAR or anything, since this is the first case we've ever had a want for such a thing. Just proposing an possible solution for this case, and seeing if it makes sense.

@lasote
Copy link

lasote commented Feb 15, 2018

I think it can be nicely modeled with a new option (excluded from the sha). It's not needed a new cpp_info property. You could choose the option if include the "optionals" or not. But as I said, I'm removing it from the list until I get more feedback

@solvingj
Copy link
Member

solvingj commented Feb 15, 2018

Hrmn, If we're not going to include it in the SHA, then I don't think i would use an option. If the binary is going to be in the package either way, then the question is only whether or not the user wants to link with it. And, in that case, a new extra/optional generated variable (like a user variable) seems to make the most sense to me.

I guess it comes down to whether or not the user makes their choice upon conan install command by setting the option... or whether the user makes their choice within their CMake file by choosing to link to an extra cmake variable or not. Interesting question.

@SSE4
Copy link
Member

SSE4 commented Mar 3, 2018

I personally don't think we need option here. it's the same package, library is always exported, it's just not automatically linked as it's not exposed into self.cpp_info.libs.
and if someone wants to link with it for whatever reason (which I highly doubt), it's exported, and can be easily added without any modifications to the boost.test recipe.
therefore, I suggest to close an issue, as no modifications to our library are needed.

@grafikrobot
Copy link
Member

I'm with @SSE4 .. We just need to remove it and just document that it's there if someone really has a need for it.

@solvingj
Copy link
Member

solvingj commented Mar 5, 2018

Ok, so we do need to modify our library, and remove it. Got it, thanks. Let's keep this ticket open until thats done.

@db4
Copy link
Author

db4 commented Mar 21, 2018

Any progress with this? The fix looks simple enough, why not to commit it?

@grafikrobot
Copy link
Member

@db4 Sorry.. I've been "distracted" trying to fix the boost_python failures. I'll reprioritize and get to this later today.

@grafikrobot grafikrobot moved this from TODO to In Progress in Boost Modular Mar 22, 2018
@grafikrobot
Copy link
Member

@db4 fixed in testing/1.66.0, please try it and tell me if it works. If it works, I'll push to the other branches.

bincrafters/conan-boost_test@9409f5a

@db4
Copy link
Author

db4 commented Mar 23, 2018

@grafikrobot It works for me, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Boost Modular
  
DONE
Development

No branches or pull requests

6 participants