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 Build Error for RelWithDebInfo #95

Open
smessmer opened this issue Jul 23, 2023 · 6 comments
Open

Windows Build Error for RelWithDebInfo #95

smessmer opened this issue Jul 23, 2023 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@smessmer
Copy link
Contributor

Setup: cryptopp-cmake and cryptopp are subfolders of my main project. cryptopp-cmake is included with add_subdirectory and CRYPTOPP_SOURCES points to the folder containing cryptopp.

Building works fine on Ubuntu, but on WIndows it fails with

=> Module : cryptopp
-- Using branch HEAD for tests
-- [cryptopp] CMake version 3.26.4
-- [cryptopp] System Windows
-- [cryptopp] Processor AMD64
-- [cryptopp] CMAKE_HOST_SYSTEM_PROCESSOR : AMD64
-- [cryptopp]      CMAKE_SYSTEM_PROCESSOR : AMD64
-- [cryptopp] Target architecture detected as: x86_64 -> CRYPTOPP_AMD64
-- Looking for C++ include winapifamily.h
-- Looking for C++ include winapifamily.h - found
-- The ASM_MASM compiler identification is MSVC
-- Found assembler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/ml64.exe
-- Found OpenMP_C: -openmp (found version "2.0") 
-- Found OpenMP_CXX: -openmp (found version "2.0") 
-- Found OpenMP: TRUE (found version "2.0")  
-- [cryptopp] Generating cmake package config files
-- [cryptopp] Generating pkgconfig files
-- [cryptopp] Platform: x86_64
-- [cryptopp] Compiler definitions:  _WIN32_WINNT=0x0A00
-- [cryptopp] Compiler options: /DWIN32 /D_WINDOWS /EHsc   /FIwinapifamily.h;/GR;/MP;/EHsc
-- [cryptopp] Build type: RelWithDebInfo
-- Building version 0.11.3+90.gee5b0db6
-- Linking to Dokan x86_64
-- This is Windows. Will not install man page
-- Building version 0.11.3+90.gee5b0db6
-- WIX package version is 0.11.3
-- Configuring done (57.0s)
-- Generating done (0.5s)
-- Build files have been written to: D:/a/cryfs/cryfs/build
cmake --build . --config RelWithDebInfo
Microsoft (R) Build Engine version 16.11.2+f32259642 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Microsoft\VC\v160\Microsoft.CppBuild.targets(436,5): error MSB8013: This project doesn't contain the Configuration and Platform combination of RelWithDebInfo|x64. [D:\a\cryfs\cryfs\build\ZERO_CHECK.vcxproj]
Error: Process completed with exit code 1.

I'm not sure why this happens, in the previous cryptopp-cmake, it worked fine. Before upgrading to cryptopp 8.8.0 and this new cmake, CI passes and after upgrading, it fails.

Upgrade commit: cryfs/cryfs@ee5b0db
CI Error: https://github.com/cryfs/cryfs/actions/runs/5571367891/job/15085389729

Note that building with -DCMAKE_BUILD_TYPE=Release or -DCMAKE_BUILD_TYPE=Debug fail with a different error, so maybe this bug only exists for -DCMAKE_BUILD_TYPE=RelWithDebInfo.

@abdes
Copy link
Owner

abdes commented Jul 23, 2023 via email

@Vollstrecker
Copy link
Collaborator

I guess this happens because we only handle Debug and Release, so the message saying this configuration is unknown, ist right. I know there is a way to tell cmake that RelWithDebInfo should be treated as Release (or Debug or whatever you like), but I'm out for the next 2 weeks. But the other errors would be interesting, too.

@Oddegamra
Copy link
Contributor

I've run into this problem as well. I think it would be preferable to have an opt-out of rewriting the available configuration types, or only doing it if cryptopp is built as a stand-alone project (via CMAKE_SOURCE_DIR == CMAKE_CURRENT_SOURCE_DIR, for example). Modifying the build environment of parent projects seems like breaking behavior.

@Vollstrecker
Copy link
Collaborator

What do you mean with modifying the build env of parent? If you're talking about the property, this only sets the choices for the dropdown in cmake-gui but nonetheless it's just a string var. If you want to set it to "mycooltype" you can do that. The problem is, the original code only knows Debug and Release, and we also have those 2 configs. If you use something different the outcome is undefined.

So there are 3 options, we can either allow different config-types, or we can error out with hint to choose one of the supported types, or we find someone that takes the time to deeply check the function of the other build-types.

The mapping I mentioned earlier works only for imported targets, so that's no option.

@Oddegamra
Copy link
Contributor

To clarify, I am talking about setting CMAKE_CONFIGURATION_TYPES as cache variable (plus FORCE), which will affect all projects including Cryptopp as subproject as well. Generally, my understanding is setting CMAKE_xxx variables as CACHE variables should be done with extreme caution, since it is usually impossible to determine how this will impact upstream projects.

That being said, I believe that the following two CMakeLists.txt files are a minimum example triggering this particular problem.

First, a main CMakeLists.txt:

cmake_minimum_required(VERSION 3.21)
set(CMAKE_CONFIGURATION_TYPES Debug Release RelWithDebInfo)
project(cmaketest LANGUAGES CXX)
add_executable(main c.cpp)
add_subdirectory(a)

Then, the CMakeLists.txt in directory a (this would be cryptopp-cmake when reduced by 99%):

cmake_minimum_required(VERSION 3.20)
project(a)
set (CMAKE_CONFIGURATION_TYPES "Debug;Release" CACHE STRING "Config types" FORCE)
add_library(a a.cpp)

When using the Visual Studio generator, the target "main" is generated with Debug, RelWithDebInfo and Release, while target "a" contains only configurations for Debug and Release, thus making it impossible to built the whole solution with RelWithDebInfo.

I suspect that this behavior is related to CMake policy differences between version 3.20 and 3.21, because bumping the minimum required version from 3.20 to 3.21 does produce a RelWithDebInfo target for subproject A. The likely candidate would be CMP0126. Adding set(CMAKE_POLICY_DEFAULT_CMP0126 NEW) to the main CMakeLists.txt does indeed override what cryptopp-cmake is doing with CMAKE_CONFIGURATION_TYPES, and generates a RelWithDebInfo configuration for it as well.

@abdes
Copy link
Owner

abdes commented Aug 4, 2023

Given that RelWithDebugInfo should be nothing more then Release with the compiler options to generate debug info, I'm fine with adding support for it in cryptopp-cmake. This should be no more than adding '-g' for most compilers, with probably some problems with MSVC maybe, as usual... 😄

The second issue is whether we are inadvertently impacting the parent project's build environment by setting global CMake variables in the cache. This should not be the as designed behavior. We do not want that to happen, and if it's happening, it's by mistake 😊 .

@Oddegamra you are welcome to submit fixes for one or both of the above issues. We are open to contributions from the community using cryptopp-cmake. The fixing by @Vollstrecker or myself may take some time as both of us seem to be quite busy with other stuff these days...

@abdes abdes added enhancement New feature or request good first issue Good for newcomers labels Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants