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.

Sign up for GitHub

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

Jump to bottom

Linking errors on MSVC (2017 Community) #86

Closed
cntaylor opened this issue Jul 11, 2019 · 4 comments
Closed

Linking errors on MSVC (2017 Community) #86

cntaylor opened this issue Jul 11, 2019 · 4 comments

Comments

@cntaylor
Copy link
Contributor

Description

In MSVC 2017 (Community), including the gtsam library causes linking errors. Specifically, gstam_matlab_wrapper, testLieMatrix, testLieVector, testLieScalar, and testMatrix does not finish compiling. During the linking phase, 100s of errors are generated, but they all have to do with an "unresolved external symbol __declspec(dllimport)" and it talks about Eigen. An example error:

 Error	LNK2019	unresolved external symbol "__declspec(dllimport) public: __cdecl gtsam::LieVector::LieVector(class Eigen::Matrix<double,-1,1,0,-1,1> const &)" (__imp_??0LieVector@gtsam@@QEAA@AEBV?$Matrix@N$0?0$00$0A@$0?0$00@Eigen@@@Z) referenced in function "public: virtual void __cdecl LieVectorInvariantsTest::run(class TestResult &)" (?run@LieVectorInvariantsTest@@UEAAXAEAVTestResult@@@Z)	check_base_program	C:\AFIT\lib\gtsam\build\gtsam\base\tests\testLieVector.obj

Steps to reproduce

  1. Use the branch msvc-fixes
  2. Run Cmake
  • Some switches of possible importance:
    • GTSAM_INSTALL_MATLAB_TOOLBOX is on
    • GTSAM_BUILD_TESTS is on
    • GTSAM_BUILD_WRAP is on
    • GTSAM_ALLOW_DEPRECATED... is off
    • GTSAM_BUILD_UNSTABLE is off
    • MEX directory and executable location are set appropriately
    • GTSAM_USE_SYSTEM_EIGEN is off (I don't have Eigen installed separately, so it can't be a conflict)
    • GTSAM_WITH_TBB is off
    • GTSAM_MEX_BUILD_STATIC_MODULE is off (documentation says it must be off for Windows!)
  1. Open the solution file and try to compile. gtsam compiles properly. Unit tests and gtsam_matlab_wrapper does not.

Expected behavior

Code completes compiling without errors

Environment

Windows 10, Visual Studio 2017 Community, Boost 1.67. Errors appear in DEBUG or RELEASE.

Additional information

I am not sure, but I think it has to do with something like the problem described on this webpage. Because Matrix is not declaring the size of the Matrix up front, then you have a not fully defined type being used as an interface to a dll. That is my best guess though and I have never dug into how dlls work on Windows.
@cntaylor cntaylor mentioned this issue Jul 11, 2019
Merged
2 tasks
@cntaylor
Copy link
Contributor Author

I think I have fixed this issue ... I will push up my changes after I am done testing and then hopefully close this issue, but wanted to get some thoughts/questions out there.

The primary source of error was LieVector, LieScalar, and LieMatrix. Because there are no .cpp files associated with these classes, the test and wrapper functions complained during compile as they try to call the functions in these header files. I can fix all the compilation problems (everything builds, but I haven't tried the generated MATLAB toolbox yet ... waiting on a re-compile) by simply removing these three files from all the CMakeLists.txt files and from any file that tries to call them. BUT, it disturbs me that everything compiles in Linux, even with the headers for these classes but not the .cpp files. Why does Linux compile without LieVector, LieScalar, or LieMatrix.cpp (and pass all the tests, including testLieMatrix) when there are no .cpp files defining what those classes are?

Thoughts?

@cntaylor
Copy link
Contributor Author

Pull request #87 closes this issue...

@dellaert
Copy link
Member

PR #87 is not the right fix, see my comment there...

@dellaert dellaert reopened this Jul 13, 2019
@cntaylor
Copy link
Contributor Author

Fixed with most recent PR #87 that is in the develop branch now.

varunagrawal added a commit that referenced this issue Apr 17, 2021
@varunagrawal
Squashed 'wrap/' changes from bae34fac8..b80bc63cf
db373df 
b80bc63cf Merge pull request #90 from borglab/fix/tpl_dependency
015b12da5 Merge pull request #86 from borglab/feature/optionalargs
362851980 address review comments
e461ca50e Merge pull request #89 from borglab/fix/template_iostream
2d413db57 add pybind cpp generation dependency on tpl file
79881c25e include pybind11 iostream for ostream_redirect in example tpl
5e8323c25 fix test fixture
95495726a Merge branch 'master' into feature/optionalargs
5af826840 clean up the _py_args_names method to reduce copy-pasta
844ff9229 add identifier parsing to _type
c3adca7a4 remove extra spaces from Type repr
350b531d7 slight test improvement
fd4f37578 cleaner default argument parsing
6013deacb overpowered default argument parsing rule
dbcda0ea2 fix unit tests for __repr__ ref  vs ptr
1c23c42e4 fix pointer vs const ref in __repr__
9b40350f1 update matlab tests
df7e9023c handle __repr__ with default arguments
092ef489b update pybind_wrapper for default arguments
3a2d7aa8a unit test default argument pybind
61a2b114e implement default argument parser
c2b92ffec unit test for parsing default arguments

git-subtree-dir: wrap
git-subtree-split: b80bc63cf466f9751e8059c0abb4a4d73b23efbe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cntaylor @dellaert