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

CMakeDetermineCompileFeatures throwing errors #851

Closed
KillerDiek opened this issue Jun 19, 2024 · 24 comments · Fixed by #852
Closed

CMakeDetermineCompileFeatures throwing errors #851

KillerDiek opened this issue Jun 19, 2024 · 24 comments · Fixed by #852

Comments

@KillerDiek
Copy link

Hello, obligatory very new to this and C++ programming in general. I'm trying to build the files using the provided guide and whenever I try and run the build, no matter what I change, I get this information:

Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.22631.
The CXX compiler identification is MSVC 19.40.33811.0
Detecting CXX compiler ABI info
Detecting CXX compiler ABI info - done
Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.40.33807/bin/Hostx64/x64/cl.exe - skipped
Detecting CXX compile features
Detecting CXX compile features - done
CMake Error at cmake/config.cmake:14 (include):
  include could not find requested file:

    CMakeDetermineCompileFeatures
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)


Found PostgreSQL: C:/Program Files/PostgreSQL/16/lib/libpq.lib (found version "16.3")
Looking for poll
Looking for poll - not found
CMake Error at cmake/config.cmake:42 (cmake_determine_compile_features):
  Unknown CMake command "cmake_determine_compile_features".
Call Stack (most recent call first):
  CMakeLists.txt:27 (include)


Configuring incomplete, errors occurred!

I'm very lost on what to do and I've been at this for several hours now so I'm throwing in the towel and asking here. Any help is much appreciated.

@jtv
Copy link
Owner

jtv commented Jun 19, 2024

Is it possible that you need a different CMake version?

@tt4g
Copy link
Contributor

tt4g commented Jun 20, 2024

It seems that CMakeDetermineCompileFeatures has been renamed to CMakeDetermineCompilerSupport and cmake_determine_compile_features to cmake_determine_compiler_support in CMake version 3.30 (not released yet): https://gitlab.kitware.com/cmake/cmake/-/commit/588371d2d5fdda0340b29058147f49e40723ba71
If you are using version 3.30.0-rc3, that is probably the cause.

include(CMakeDetermineCompileFeatures)

cmake_determine_compile_features(CXX)

This problem should be avoided by making the following changes to cmake/config.cmake:

+if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.30.0)
+    include(CMakeDetermineCompilerSupport)
+    cmake_determine_compiler_support(CXX)
+else()
+    include(CMakeDetermineCompileFeatures)
+    cmake_determine_compile_features(CXX)
+endif()
 
 include(CheckIncludeFileCXX)
 include(CheckFunctionExists)
 include(CheckSymbolExists)
-include(CMakeDetermineCompileFeatures)
 include(CheckCXXSourceCompiles)
 include(CMakeFindDependencyMacro)

(skip...)

 
-cmake_determine_compile_features(CXX)
 cmake_policy(SET CMP0057 NEW)

@KillerDiek
Copy link
Author

That solved it, thank you for your help and patience, I truly appreciate it!

@jtv
Copy link
Owner

jtv commented Jun 23, 2024

Reopening this. It's not enough to have a solution that users can find for themselves. It should work out of the box, so I'll integrate the fix.

Thanks again @tt4g!

@jtv jtv reopened this Jun 23, 2024
jtv added a commit that referenced this issue Jun 23, 2024
Thanks @tt4g.
@jtv jtv closed this as completed in #852 Jun 23, 2024
@jtv jtv closed this as completed in 0444b8b Jun 23, 2024
@killerbot242
Copy link
Contributor

I spotted this problem yesterday, and asked around on the cmake support/forum.

The solution implemented here, will indeed solve the problem. But it seems by design this snippet is bad and should not be used.

The answer I got from Craig Scott:
That file is an internal implementation detail of CMake. It is not part of CMake’s public API, projects and developers should not be trying to use it directly. It can (and apparently now has in the 3.30 release) change without notice.

See here : https://discourse.cmake.org/t/cmakedeterminecompilerfeatures-cmake-is-no-more-in-3-30-bug-or-not/11176

kind regards,
Lieven

@jtv
Copy link
Owner

jtv commented Jul 7, 2024

No word on what we should do instead? Because we probably need to do something.

@jtv
Copy link
Owner

jtv commented Jul 7, 2024

(Mentioning @killerbot242 because Github may not notify you on a closed ticket. In fact I'd be in favour of opening a new one.)

@killerbot242
Copy link
Contributor

killerbot242 commented Jul 8, 2024

Hi Jeroen (@jtv ), could you answer what you try to accomplish on the cmake discourse link above. I asked for the apis to use, but Craig turned the question around, and would like to know what you are trying to do.

@killerbot242
Copy link
Contributor

I have the impression, you are doing this in order to get the value of "CMAKE_CXX_COMPILE_FEATURES" ?

but one can just do this, no include of something needed it seems ???

cmake_minimum_required(VERSION 3.30)
project(CompileFeatures)
message("CMAKE_CXX_COMPILE_FEATURES : ${CMAKE_CXX_COMPILE_FEATURES}")

@tt4g
Copy link
Contributor

tt4g commented Jul 8, 2024

I found include(CMakeDetermineCompileFeatures) added in fc2a530.

As @killerbot242 pointed out, it looks like it was added for the following code that references CMAKE_CXX_COMPILE_FEATURES .

fc2a530#diff-7520a891ea97b695753e3db2aaf7b2e26a6ab475ff0d62e4071355f74a06304dR43-R47

cmake_determine_compile_features(CXX)
cmake_policy(SET CMP0057 NEW)
if (cxx_attribute_deprecated IN_LIST CMAKE_CXX_COMPILE_FEATURES)
  set(PQXX_HAVE_DEPRECATED)
endif ()

Since this code has already been removed, include(CMakeDetermineCompileFeatures) may not be necessary.

@killerbot242
Copy link
Contributor

with respect to keeping the config.cmake clean, I think the following are also no longer needed:

CheckIncludeFileCXX --> there are no more checks for certain include files (at least not here)
CheckSymbolExists --> it is still using check_function_exists for "poll" --> CheckFunctionExists.

I have not checked the purpose of CMakeFindDependencyMacro, by its name it sounds like another internal api not to be used ??

@killerbot242
Copy link
Contributor

by the way, modern cmake discourages GLOB, and advices to explicitly enumerate the source files

@tt4g
Copy link
Contributor

tt4g commented Jul 8, 2024

@killerbot242 Can you send a PR?

@killerbot242
Copy link
Contributor

what do you think of the idea, to include stuff only where needed ?

Incorporate feature checks based on C++ feature test mac

include(pqxx_cxx_feature_checks)

==> so only in there, we actually need :

  • include(CheckCXXSourceCompiles)
  • CMAKE_REQUIRED_DEFINITIONS backup before compile checks
  • CMAKE_REQUIRED_DEFINITIONS restore after compile checks
  • CMAKE_REQUIRED_QUIET ON/OFF

@killerbot242
Copy link
Contributor

detect_code_compiled also seems no longer used and can be removed ?

@tt4g
Copy link
Contributor

tt4g commented Jul 8, 2024

Obviously unused CMake functions, and include() of modules that do not exist in the official CMake documentation can be removed.
Other than that, there is no need to change them right now, as they may cause unexpected side effects.

@killerbot242
Copy link
Contributor

I can create PRs , most probably per subtopic, but I can not make branches in here in github, and I am not familiar with the process on github, and this specific repo ?
And quick pointers on your preferred procedure ?

@tt4g
Copy link
Contributor

tt4g commented Jul 8, 2024

  1. You can press the fork button on GitHub to create a copy of this repository in your account
  2. Create a branch in the forked repository and work on it
  3. When you are done working on the branch, push it to the forked repository
  4. Then, visit this repository and you will see a button to create a Pull Request

You can find many explanations of this procedure by searching for "GitHub Fork Pull Request".

See also GitHub Document:

@jtv
Copy link
Owner

jtv commented Jul 10, 2024

Thanks @killerbot242, some PRs would be most appreciated! I'm in a place with 74% packet loss, high temperatures, and incredible noise — and CMake is a topic for which I rely mostly on contributors.

(Also I like to see contributors credited in the git logs. I used to spell this out in change logs, but it's easy to get wrong and mixing it in with the technical details tends to become messy. Better if git just tells people that you contributed something.)

@killerbot242
Copy link
Contributor

I tried to create my first commit/push to the fork, and it fails :-( during the push.

git push
remote: Support for password authentication was removed on August 13, 2021.
remote: Please see https://docs.github.com/get-started/getting-started-with-git/about-remote-repositories#cloning-with-https-urls for information on currently recommended modes of authentication.
fatal: Authentication failed for 'https://github.com/killerbot242/libpqxx.git/'

Any suggestions ?

killerbot242 added a commit to killerbot242/libpqxx that referenced this issue Jul 15, 2024
The following comments explains why it was added, and that indeed it is no longer needed, as such.
jtv#851 (comment)

Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff.
Nothing on the outside should use it. This was feedback given by kitware developers.
@killerbot242
Copy link
Contributor

I was able to create a token, and with that I could push.

killerbot242 added a commit to killerbot242/libpqxx that referenced this issue Jul 15, 2024
    The following comments explains why it was added, and that indeed it is no longer needed, as such.
    jtv#851 (comment)

    Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff.
    Nothing on the outside should use it. This was feedback given by kitware developers.
@tt4g
Copy link
Contributor

tt4g commented Jul 15, 2024

Can you create a pull request to merge pushed code into this repository?

@killerbot242
Copy link
Contributor

first PR is created, once this one is processed I will move on to the next one. I would like to bring them in little chunks, each focusing at one topic.

jtv pushed a commit that referenced this issue Jul 20, 2024
The following comments explains why it was added, and that indeed it is no longer needed, as such.
    #851 (comment)

    Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff.
    Nothing on the outside should use it. This was feedback given by kitware developers.
@jtv
Copy link
Owner

jtv commented Jul 28, 2024

That you for that by the way @killerbot242 - I find it's much better to get through these things in small chunks.

jtv pushed a commit that referenced this issue Aug 1, 2024
* remove a use added for a purpose which is gone in the meantime.
The following comments explains why it was added, and that indeed it is no longer needed, as such.
#851 (comment)

Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff.
Nothing on the outside should use it. This was feedback given by kitware developers.

* not on master should be on a branch

Revert "remove a use added for a purpose which is gone in the meantime."

This reverts commit 7fdd2f8.

* based upon the CMAKE_CXX_STANDARD, cmake can generate an option like : -std=c++17, -std=c++20, ...
Just before the try_compile sections are exectued, the following happens:
- CMAKE_REQUIRED_DEFINITIONS is back-up-ed
- next it gets adjusted, to contain the cmpiler option to specif the language standard, this is done by setting ${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}
- some exampes of that are: CMAKE_CXX17_STANDARD_COMPILE_OPTION: -std=c++17 , CMAKE_CXX20_STANDARD_COMPILE_OPTION: -std=c++20
- and that value becomes the new value for CMAKE_REQUIRED_DEFINITIONS
- next all the try_compile statements are issued
- and afterwards the CMAKE_REQUIRED_DEFINITIONS is restored

Now all of that is not needed, since the minimum cmake required is 3.8.
And in cmake 3.8 an adjustment was made so the standard selection compile option is passed to try_compile invocations.
See here : https://cmake.org/cmake/help/latest/policy/CMP0067.html#policy:CMP0067 : Honor language standard in try_compile() source-file signature.
This means that any cmake version >= 3.8 is using by default this new behavior (unless someone would overrule it and put it explicitly to old).
I have done experiments with this, toether with the changed cmake_minimum_required syntax (it can have range), whic allowed me to have newer cmake behave as 3.7 and 3.8 ... , and in 3.7 the lnaguag standard indeed was not passed on, but in 3.8 it is.

Conclusion : this entire process mentioned above is not needed, this is the default behavior of cmake (since 3.8).
jtv pushed a commit that referenced this issue Oct 22, 2024
* remove a use added for a purpose which is gone in the meantime.
The following comments explains why it was added, and that indeed it is no longer needed, as such.
#851 (comment)

Note that neither of these 2 cmake modules are for public use, they are internal cmake stuff.
Nothing on the outside should use it. This was feedback given by kitware developers.

* not on master should be on a branch

Revert "remove a use added for a purpose which is gone in the meantime."

This reverts commit 7fdd2f8.

* no need for include for try_compile
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 a pull request may close this issue.

4 participants