vs2019-cmake std filesystem 错误

[bstaint]

• Visual-Studio-2019-cmake-open-folder-style-project
  vs2019
  Windows 7
  cmake: 3.16.0
  develop分支

错误：

CMake Error at build/cmake/select_filesystem.cmake:57 (check_include_file_cxx):
  Unknown CMake command "check_include_file_cxx".
Call Stack (most recent call first):
  CMakeLists.txt:135 (include)

57行加上：
include (CheckIncludeFileCXX)

解决之后：

Looking for C++ include filesystem
Looking for C++ include filesystem - found
Looking for C++ include experimental/filesystem
Looking for C++ include experimental/filesystem - found
C++ Filesystem header:      <filesystem>
Performing Test CXX17_BUILTIN
Performing Test CXX17_BUILTIN - Failed
Performing Test CXX17_FLAG
Performing Test CXX17_FLAG - Failed
CMake Warning at build/cmake/select_filesystem.cmake:115 (message):
  nana requires C++17??, but your compiler does not support it.
Call Stack (most recent call first):
  CMakeLists.txt:135 (include)

__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/

[ppetraki]

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:

• https://crascit.com/2015/03/28/enabling-cxx11-in-cmake/
• https://resharper-support.jetbrains.com/hc/en-us/community/posts/360003534559-Unable-to-set-C-standard-in-a-Visual-Studio-2019-cmake-open-folder-style-project
• IntelliSense fails to detect _HAS_CXX17 macro with compile_commands.json microsoft/vscode-cpptools#1990

[ppetraki]

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

[bstaint]

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

[qPCR4vir]

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.

[qPCR4vir]

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.

[ppetraki]

@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. qPCR4vir#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?

0. Nana 1.x will have to continue to walk the line of a C++11 capable project, but is primary developed and tested on C++17. A CI regression suite must be established ensure 11 and 14 can build with Boost etc.
1. Do a better job of determining the suitability of the std::fs library provided by the target platform and fail at project configuration time if no suitable fallback is available (e.g. Boost).
2. Shim filesystem support and use the minimum API set available between boost and the std, code whatever helpers we need and keep them in our namespace.
3. Strongly consider committing to a single build system, you have like 4 now, which compounds the problem of proper fs detection and support.
4. Plan for a Nana-2.x branch that can start at C++20 (module support?) . At which point you should be able to drop the option for boost filesystem entirely.

[qPCR4vir]

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

[ppetraki]

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?

[qPCR4vir]

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(qPCR4vir#36, qPCR4vir#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

[bstaint]

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

[qPCR4vir]

Thank!

[ppetraki]

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.

[qPCR4vir]

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.

[ppetraki]

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.

[qPCR4vir]

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.