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

Minor CMake change to better integrate c++17 and modernize GTest-integration #174

Merged
merged 2 commits into from
Oct 27, 2020

Conversation

emmenlau
Copy link
Contributor

This is a minor PR that improves the integration of c++17 and of testing, and specifically of GTest. It brings better compatibility with FindGTest from (future) cmake.

Please let me know if it would need more motivation? :-)

@emmenlau
Copy link
Contributor Author

The AppVeyor build error seems unrelated, unless I'm mistaken? It says:

LINK: command "C:\PROGRA~2\MI0E91~1.0\VC\bin\amd64\link.exe /nologo @CMakeFiles\test_xtensor_blas.dir\objects1.rsp /out:test_xtensor_blas.exe /implib:test_xtensor_blas.lib /pdb:C:\projects\xtensor-blas\test\test_xtensor_blas.pdb /version:0.0 /MANIFEST:NO /debug /INCREMENTAL /subsystem:console C:\xtensor-conda\Library\mingw-w64\lib\libopenblas.dll.a C:\xtensor-conda\Library\lib\gtest_main-mdd.lib C:\xtensor-conda\Library\lib\gtest-mdd.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib" failed (exit code 1120) with the following output:
gtest-mdd.lib(gtest-all.cc.obj) : warning LNK4099: PDB 'gtestd.pdb' was not found with 'gtest-mdd.lib(gtest-all.cc.obj)' or at 'C:\projects\xtensor-blas\test\gtestd.pdb'; linking object as if no debug info
gtest-mdd.lib(gtest-all.cc.obj) : error LNK2019: unresolved external symbol __std_reverse_trivially_swappable_4 referenced in function "void __cdecl std::_Reverse_unchecked1<enum testing::internal::edit_distance::EditType *>(enum testing::internal::edit_distance::EditType * const,enum testing::internal::edit_distance::EditType * const,struct std::integral_constant<unsigned __int64,4>)" (??$_Reverse_unchecked1@PEAW4EditType@edit_distance@internal@testing@@@std@@YAXQEAW4EditType@edit_distance@internal@testing@@0U?$integral_constant@_K$03@0@@Z)
test_xtensor_blas.exe : fatal error LNK1120: 1 unresolved externals
NMAKE : fatal error U1077: 'C:\xtensor-conda\Library\bin\cmake.exe' : return code '0xffffffff'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe"' : return code '0x2'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe"' : return code '0x2'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\nmake.exe"' : return code '0x2'

@emmenlau
Copy link
Contributor Author

Ah but the build error on Azure is related. I think my change only works with cmake >= 3.18. I'll investigate...

@emmenlau emmenlau force-pushed the bda_gtest_upgrade branch 2 times, most recently from e82a80e to 22d822a Compare October 21, 2020 13:27
@emmenlau
Copy link
Contributor Author

Ok, it seems my changes trigger a very rare problem with Visual Studio, that symbol __std_reverse_trivially_swappable_4 is not resolved. Its no well known and I could not find a clear solution, but reports indicate that a linking is performed with a Visual Studio linker that is older than the one that produced the Google test binaries.

I'll try to build with a downloaded GTest to confirm this.

@emmenlau emmenlau force-pushed the bda_gtest_upgrade branch 2 times, most recently from 5c6dcaa to 7a14938 Compare October 21, 2020 16:38
@JohanMabille
Copy link
Member

JohanMabille commented Oct 22, 2020

Ok, it seems my changes trigger a very rare problem with Visual Studio, that symbol __std_reverse_trivially_swappable_4 is not resolved. Its no well known and I could not find a clear solution, but reports indicate that a linking is performed with a Visual Studio linker that is older than the one that produced the Google test binaries.

Thanks for catching this, it looks like this is the reason for the failure of #172.

test/CMakeLists.txt Outdated Show resolved Hide resolved
@JohanMabille
Copy link
Member

Looks like there are two unrelated changes here, improving gtest support and changes relative to C++17 support. Although these changes are small, they have enough impact on the codebase so that it makes sense to split the PR.

@emmenlau
Copy link
Contributor Author

Yes you are right - I'll split and update the comments.

@JohanMabille JohanMabille merged commit 8afdee5 into xtensor-stack:master Oct 27, 2020
@emmenlau emmenlau deleted the bda_gtest_upgrade branch October 27, 2020 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants