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

Build error caused by overwriting CMAKE_CXX_COMPILER #2343

Closed
3 of 5 tasks
ongjunjie opened this issue Aug 7, 2020 · 2 comments · Fixed by #2344
Closed
3 of 5 tasks

Build error caused by overwriting CMAKE_CXX_COMPILER #2343

ongjunjie opened this issue Aug 7, 2020 · 2 comments · Fixed by #2344
Assignees
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@ongjunjie
Copy link

Building unit tests with a combination of CMake and MSVC fails when selecting generators other than Visual Studio Project (ie. NMake, Ninja). The build failure is due to CMAKE_CXX_COMPILER being overwritten.

if (MSVC)
execute_process(COMMAND ${CMAKE_CXX_COMPILER} OUTPUT_VARIABLE CXX_VERSION_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_VARIABLE CXX_VERSION_RESULT ERROR_STRIP_TRAILING_WHITESPACE)
set(CMAKE_CXX_COMPILER "${CXX_VERSION_RESULT}; MSVC_VERSION=${MSVC_VERSION}; MSVC_TOOLSET_VERSION=${MSVC_TOOLSET_VERSION}")
else()
execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version OUTPUT_VARIABLE CXX_VERSION_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()

I would be happy to open a PR, but I do not understand the intent of the block of code that causes the problem. My first thought is to remove the entire block of code. If the message printed at the end is important, CMake already provides CMAKE_CXX_COMPILER_ID and CMAKE_CXX_COMPILER_VERSION. There's no need to reinvent the wheel.

What is the issue you have?

Can't build unit test, see above.

Please describe the steps to reproduce the issue.

$ mkdir build
$ cd build
$ cmake -G "NMake Makefiles" .. -DJSON_BuildTests=On
$ cmake --build .

Can you provide a small but working code example?

No.

What is the expected behavior?

Project gets built.

And what is the actual behavior instead?

Build fails.

Which compiler and operating system are you using?

  • Compiler: Visual Studio 2015
  • Operating system: Windows 7

Which version of the library did you use?

  • latest release version 3.9.1
  • other release - please state the version: ___
  • the develop branch

If you experience a compilation error: can you compile and run the unit tests?

  • yes
  • no - please copy/paste the error message below
test\CMakeFiles\doctest_main.dir\flags.make(5): fatal error U1034: syntax error : separator missing
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.
@nlohmann
Copy link
Owner

nlohmann commented Aug 7, 2020

The goal of

if (MSVC)
execute_process(COMMAND ${CMAKE_CXX_COMPILER} OUTPUT_VARIABLE CXX_VERSION_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_VARIABLE CXX_VERSION_RESULT ERROR_STRIP_TRAILING_WHITESPACE)
set(CMAKE_CXX_COMPILER "${CXX_VERSION_RESULT}; MSVC_VERSION=${MSVC_VERSION}; MSVC_TOOLSET_VERSION=${MSVC_TOOLSET_VERSION}")
else()
execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version OUTPUT_VARIABLE CXX_VERSION_RESULT OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()

is to have as much detail on the compiler as possible. I played around a bit with the variables provided by CMake, but every now and then missed a detail. However, looking at

set(CMAKE_CXX_COMPILER "${CXX_VERSION_RESULT}; MSVC_VERSION=${MSVC_VERSION}; MSVC_TOOLSET_VERSION=${MSVC_TOOLSET_VERSION}")

it seems this should rather be

set(CXX_VERSION_RESULT "${CXX_VERSION_RESULT}; MSVC_VERSION=${MSVC_VERSION}; MSVC_TOOLSET_VERSION=${MSVC_TOOLSET_VERSION}")

Overwriting CXX_VERSION_RESULT really makes no sense here. Would this fix your issue?

@ongjunjie
Copy link
Author

Yes. Now that line makes sense.

ongjunjie pushed a commit to ongjunjie/json that referenced this issue Aug 7, 2020
@nlohmann nlohmann added release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Aug 9, 2020
@nlohmann nlohmann self-assigned this Aug 9, 2020
@nlohmann nlohmann added this to the Release 3.9.2 milestone Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants