Closed bstaint closed 4 years ago
Using "set" to specify the C++ standard is not portable nor is it a modern cmake best practice.
The C++ standard is already being expressed in a compiler independent manner, so whatever is going wrong must be some cmake nuance or a compiler bug. Please upload CMakeError.log and CMakeOutput.log.
It's redundant to specify target_compile_features twice with a different set of constraints, and it might be causing problems here. In the top level cmake file you'll see this.
add_library(nana) add_library(nana::nana ALIAS nana) target_compile_features(nana PUBLIC cxx_std_17) # need after cxx_std_14 or cxx_std_17 ?? target_compile_features(nana PUBLIC cxx_nullptr PUBLIC cxx_range_for PUBLIC cxx_lambdas PUBLIC cxx_decltype_auto PUBLIC cxx_defaulted_functions PUBLIC cxx_deleted_functions PUBLIC cxx_auto_type PUBLIC cxx_decltype_incomplete_return_types PUBLIC cxx_defaulted_move_initializers PUBLIC cxx_noexcept PUBLIC cxx_rvalue_references )
That second target_compile_features block may be the problem. Try commenting it out and try again, and if that doesn't work, uncomment it and then comment out the "cxx_std_17" line.
References:
Did you try the msbuild or the vcxproj method to compile nana?
https://ci.appveyor.com/project/qPCR4vir/nana
ppetraki@vanguard:~/Sandbox/hacking/nana-develop$ ls build/vc2019/ nana.sln nana.vcxproj nana.vcxproj.filters
Tks. I only test it, i don't know much about cmake.
1.cpp
#if !defined (__cplusplus) || (__cplusplus < 201703L)
#error NOCXX17
#endif
int main() {}
cl 1.cpp /std:c++17
, will output:
用于 x86 的 Microsoft (R) C/C++ 优化编译器 19.22.27905 版 版权所有(C) Microsoft Corporation。保留所有权利。
1.cpp 1.cpp(2): fatal error C1189: #error: NOCXX17
Hi, thank you for testing and reporting! This was a hot fix for others peoples problems, but thank to report this new problem. I will try to fix it. For your concrete problem: if you don't know cmake well, then it is best for you to use the provided VS sln and projects in Nana and Nana-demo repositories.
Now @ppetraki, this is in a .cmake file for Filesystem selection. This is not to set the global c++ flag, that is set directly with the cmake command you pointed. What would be a better alternative to ˋset (CMAKE_REQUIRED_FLAGS ...)ˋ? I mean, are you sure you know what this is for? poeple may get confused. This is setting the flag for only the test.
@qPCR4vir CMAKE_REQUIRED_FLAGS is a internal cmake option. You're playing with fire there.
If someone wants to be C++17 (or 20) from top to bottom to get all the compiler optimizations then they're going to have to do something like this. https://gitlab.kitware.com/cmake/cmake/issues/17146 , or hack the project.
IMHO you must distinguish between required language support and required libraries. If Nana 1.x is working to stay as a C++11 AND UP project, then the target_compile_features line should be std_cxx_11.
https://crascit.com/2015/03/28/enabling-cxx11-in-cmake/ https://cmake.org/cmake/help/v3.12/manual/cmake-compile-features.7.html
Right now it's a PUBLIC requirement, which means it's transitive. Worse, if I try to override it, it kinda ignores me. Working with an older nana branch, when setting std_cxx_11 and deleting all those extra compile options. It removed the --std= entirely from the command line, which means it's floating to the latest version that the compiler supports. I would be reasonable to expect that if I declare that 11 is the min, then the project self-configures to use boost, and if it isn't there, it fails. It overrode my intent.
https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/
For example, the property COMPILE_OPTIONS encodes a list of options to be passed to the compiler when building the target. If a target must be built with all warnings enabled, for instance, this list should contain the option -Wall. This is a private property used only when building the target and won’t affect its users in any way.
On the other hand, the property INTERFACE_COMPILE_FEATURES stores which features must be supported by the compiler when building users of the target. For instance, if the public header of a library contains a variadic function template, this property should contain the feature cxx_variadic_templates. This instructs CMake that applications including this header will have to be built by a compiler that understands variadic templates.
Again the emphasis is on language support, not additional libraries provided by newer versions of the language. That is a library dependency, of which you have quite a few already.
Supporting boost, gnu c++, clang c++, and msvc++ is going to require you to code to the lowest common denominator for "filesystem support". See, https://www.bfilipek.com/2019/05/boost-to-stdfs.html . Not everything from boost made it into std::filesystem. Apparently it isn't even consistent across versions of std::filesystem per the clang bug I just filed against 1.7.3. https://github.com/qPCR4vir/nana/issues/37
Also see, "Portable linking for C++17 std::filesystem" https://gitlab.kitware.com/cmake/cmake/issues/17834 and https://github.com/vector-of-bool/CMakeCM/blob/master/modules/FindFilesystem.cmake
To get out of std::fs config hell. You could set a new floor version like c++20 for Nana when you commit to 2.x. Make it a breaking change even. The project is small and the community around it is nimble enough to tolerate that.
We don't all get to move up to the latest version of C++ in our established projects, even if it the standard is backwards compatible. The whole fear of the unknown FUD keeps many projects at the C++ std they were conceived with. Sure we can edit the subproject, but now we get to maintain it too.
So to recap, what you really have today "per the cmake code", is a C++17 project, that can go backwards to as far as C++11, iif the developer changes the minimum required version. At which point you must use boost. Or, you can stay on 17 and either user the std or boost, depending on the whether the fslib provided by your compiler suite fully complies with the standard, and if it doesn't, you get to use boost.
The CI doesn't even appear to be testing a range of C++ versions so... users are going to be the first ones to find out that when this breaks. One day, someone is going to add lambda arguments that use auto to libNana and it's going to break on C++11 because the CI didn't catch it. That's "language support". You've conflated language support with additional libraries provided by the standard and it's made a huge config matrix to test which frankly, isn't creating a lot of value for your project and is hurting it in real ways. I can't even build it out of the box as a straight C++17 project anymore (1.7.3 RC).
We just want to get the bits off the disk and into memory.
So how do we move forward from here as the cats already out of the bag?
Hi @ppetraki,
Thank, interesting - it is very valuable what you said. I wish I had more time to continue this, really. I will try to respond. Some of the links are very enjoyable, but none have any relation with CMAKE_REQUIRED_FLAGS. This is not only a common and the recommended, but the only way to use check_cxx_source_compiles
(I think). So, don't worry, no fire here. I would like very much if after carefully reading the relevant link about check_cxx_source_compiles and the link abot MSVC @bstaint provided in the original post you may sugest a better variant for our select_filesystem.cmake
: the "error" come from a fail in line 104 and in line 110 , because MS uses other flags. The irony is that VC have had good support for filesystem for long time. But with VC I always use our slns and projects. This is a very concrete issue. The rest of what you write is another history. So...
People said: simple things have to be simple to do, which almost means common things have to be simple to do. Some time ago the common thing was c++11, and the nana "trivial" build was for c++11. But now it is c++17 and we are in the transition to that. Still, one have the possibility to do complex things.. at the cost of some difficulties. You certainly can continue to use c++11 and nana but with some care in the build system configuration, and is not our priority anymore.
library dependency, of which you have quite a few already
sorry? what dependency? .. ah, you probable mean the std c++ lib and the OS (win32, X11 and related basic OS support for paint, threads, mouse, fonts, etc.)
Supporting boost, ...
we have never supported boost. We had no interest in that. Boost can very well support by self and don't need our support. We have (ab)used boost to implement the sdt::filesystem some compilers (libraries) don't provide in some platforms (gcc/MinGW). Years ago this was difficult, but doable. But the c++ std evolved and diverged from boost. And the nana own filesystem implementation evolved too but in the std direction. Now we prefer to use nana implementation if no std is available rather than boost. And yes, the first thing we will do with c++20 version of nana is to eliminate all the "legacy" c++11 code and update to c++17, and then begin to experiment and adopt all the 4 big thing in c++20. It will be a lot of fun.
Strongly consider committing to a single build system, you have like 4 now,
hmm, good idea... but some people will like to kill you
Hi @ppetraki,
[select filesystem discussion]
The cmake script vector-of-bool wrote succinctly captures the discovery of std filesystem support without having to resort to platform specific define checks for C++17 support.
https://github.com/vector-of-bool/CMakeCM/blob/master/modules/FindFilesystem.cmake
Adopting this would dramatically cut down on the complexity of "select filesystem" code and reduce overall maintenance. I would draw the line with a platform supported by cmake if "set(CMAKE_CXX_STANDARD 17)" didn't do the right thing on said platform. That would require updating the min cmake version to make that work. As opposed to what you have now, which is brittle.
Also, it's not shown in the users posting, but the cxx_std_17 check would have failed at the very beginning of the project configuration if the compiler was judged to be unable to support C++17.
But now it is c++17 and we are in the transition to that. Still, one have the possibility to do complex things.. at the cost of some difficulties. You certainly can continue to use c++11 and nana but with some care in the build system configuration, and is not our priority anymore.
I've never seen a library that rolls it minimum language standard forward in the same major release version. If it's not a priority then please consider dumping the mention of 11/14 support from the front page, it's really confusing.
library dependency, of which you have quite a few already
sorry? what dependency? .. ah, you probable mean the std c++ lib and the OS (win32, X11 and related basic OS support for paint, threads, mouse, fonts, etc.)
Correct.
Supporting boost, ...
we have never supported boost. We had no interest in that. Boost can very well support by self and
Then I'm confused again, there are ifdefs and cmake logic around boost filesystem in the current codebase. If you don't support it then it shouldn't be there.
And yes, the first thing we will do with c++20 version of nana is to eliminate all the "legacy" c++11 code and update to c++17, and then begin to experiment and adopt all the 4 big thing in c++20. It will be a lot of fun.
:-}
Strongly consider committing to a single build system, you have like 4 now,
hmm, good idea... but some people will like to kill you
Well, if you continue on this path, you might want to kill yourself. Codeblocks is cross platform, so that matrix is multiplied by 3, and cmake basically supports everything. You easily have 10 build variations across IDEs and platforms today. IMHO once you hit 2.x, "one" build system should be the official version that the CI checks all the build/link type variations and the rest is community supported. You want to be the GUI business, not the build system business right?
Hi! I have simplified the script, eliminating the direct c++17 test, and now test only fs. Could you please test again with MSVC and clang(https://github.com/qPCR4vir/nana/issues/36, https://github.com/qPCR4vir/nana/issues/37), etc., before I merge back to the RC? Here: https://github.com/qPCR4vir/nana/tree/fs A key point for MSVC will be set (CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS_ORIGINAL} /std:c++17") Please change /std:c++17 until it compile correctly.
In my tests I have (Ubuntu/gcc 8.3):
-- Performing Test CXX_std__cpp17_FS_stdcppfs - Success
-- C++: -std=c++17; Filesystem library: stdc++fs
and (MinGW/gcc 9.2)
-- Performing Test CXX_std__cpp17_FS_BuiltIn - Success
-- C++: -std=c++17; Filesystem library: builtin
msvc output:
Performing Test CXX_std__cpp17_FS_BuiltIn
Performing Test CXX_std__cpp17_FS_BuiltIn - Failed
Performing Test CXX_std__cpp17_FS_stdcppfs
Performing Test CXX_std__cpp17_FS_stdcppfs - Failed
Performing Test CXX_std__cpp17_FS_cppfs
Performing Test CXX_std__cpp17_FS_cppfs - Failed
Performing Test CXX_std_cpp17_FS_BuiltIn
Performing Test CXX_std_cpp17_FS_BuiltIn - Success
C++: /std:c++17; Filesystem library: builtin
Thank!
There is still a bug where the root cmakefile's c++ std will be overwritten by the filesystem check because you directly manipulate those variables without previously saving it's state. cmake's handles this with, https://cmake.org/cmake/help/v3.0/module/CMakePushCheckState.html.
Without this, the project's --std will always be dragged back to c++17 after you check for fs support.
This is how v.o.b addressed it, making his cmake script project and state independent.
There is still a bug where the root cmakefile's c++ std will be overwritten by the filesystem check because you directly manipulate those variables without previously saving it's state. cmake's handles this with, https://cmake.org/cmake/help/v3.0/module/CMakePushCheckState.html.
Could you elaborate on that?
Without this, the project's --std will always be dragged back to c++17 after you check for fs support.
No, no. Here we don't change "the project's --std " we only change the cmake test variables which do not affect the project. Ok, we can set back like in line set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES_ORIGINAL}) the set (CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS_ORIGINAL}")
too. But this don't affect the project.
The only project state we change is like in: target_link_libraries (nana PUBLIC stdc++fs)
And when it find a working fs, it is good that it change this variables (so next user test will be consistent with this). If this make an error, that is good, and said the user it have some conflict. If std fs need some flags the user dont want, he have to set NANA_CMAKE_NANA_FILESYSTEM_FORCE and the std he want and avoid std fs.
This is how v.o.b addressed it, making his cmake script project and state independent.
It's pretty clear. You just go ahead and write into _REQURIED_FLAGS etc without saving the original state. The push/pop functions provided by CMakePushCheckState protect those vars. It's been around since 3.0.2 so it's probably a good idea to use it.
write into _REQURIED_FLAGS etc without saving the original state.
we save before change: line 97, 98: set (CMAKE_REQUIRED_FLAGS_ORIGINAL ${CMAKE_REQUIRED_FLAGS})
The push/pop functions provided by CMakePushCheckState protect those vars. It's been around since 3.0.2 so it's probably a good idea to use it.
we would need to complicate the logic even more to decide when we want to leak the variables changed and when not, which is what we are doing manually. This variables are global for some reason (to be used by downstream-tests), not just local arguments. CMakePushCheckState make actually it locals.
错误:
57行加上:
include (CheckIncludeFileCXX)
解决之后:
__cplusplus < 201703L
在msvc下似乎要加:/Zc:__cplusplus /std:c++latest
改成:
set (CMAKE_REQUIRED_FLAGS "/Zc:__cplusplus /std:c++latest ${CMAKE_CXX_FLAGS}")
可以正常编译。参考链接: https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/