Skip to content

Commit

Permalink
Remove unneeded standard passing (#869)
Browse files Browse the repository at this point in the history
* 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).
  • Loading branch information
killerbot242 authored Aug 1, 2024
1 parent 86f7d62 commit d26e9c2
Showing 1 changed file with 0 additions and 16 deletions.
16 changes: 0 additions & 16 deletions cmake/config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ check_function_exists("poll" PQXX_HAVE_POLL)

set(CMAKE_REQUIRED_LIBRARIES pq)

# check_cxx_source_compiles requires CMAKE_REQUIRED_DEFINITIONS to specify
# compiling arguments. Workaround: Push CMAKE_REQUIRED_DEFINITIONS
if(CMAKE_REQUIRED_DEFINITIONS)
set(def "${CMAKE_REQUIRED_DEFINITIONS}")
endif()
set(CMAKE_REQUIRED_DEFINITIONS
${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}
)
set(CMAKE_REQUIRED_QUIET ON)

# Incorporate feature checks based on C++ feature test mac
Expand All @@ -57,14 +49,6 @@ if(!no_need_fslib)
link_libraries(stdc++fs)
endif()

# check_cxx_source_compiles requires CMAKE_REQUIRED_DEFINITIONS to specify
# compiling arguments. Workaround: Pop CMAKE_REQUIRED_DEFINITIONS
if(def)
set(CMAKE_REQUIRED_DEFINITIONS ${def})
unset(def CACHE)
else()
unset(CMAKE_REQUIRED_DEFINITIONS CACHE)
endif()
set(CMAKE_REQUIRED_QUIET OFF)

set(AC_CONFIG_H_IN "${PROJECT_SOURCE_DIR}/include/pqxx/config.h.in")
Expand Down

0 comments on commit d26e9c2

Please sign in to comment.