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

Preliminary CMake configure support on Windows (#228) #666

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
3 changes: 2 additions & 1 deletion cmake/SetupGBench.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ CPMAddPackage(
"BENCHMARK_ENABLE_GTEST_TESTS OFF"
)

target_compile_options(benchmark PRIVATE "-Wno-format-nonliteral")
include(UserverCxxCompileOptionsIfSupported)
userver_target_cxx_compile_options_if_supported(benchmark PRIVATE "-Wno-format-nonliteral")
if (NOT TARGET benchmark::benchmark)
add_library(benchmark::benchmark ALIAS benchmark) # Unify link names
endif()
4 changes: 3 additions & 1 deletion cmake/Stacktrace.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ function(_make_stacktrace_target TARGET Boost_VERSION_STRING)
endif()

add_library(userver-stacktrace INTERFACE)
if(USERVER_FEATURE_STACKTRACE)
if(WIN32)
target_link_libraries("${TARGET}" INTERFACE Boost::stacktrace_windbg)
elseif(USERVER_FEATURE_STACKTRACE)
target_link_libraries("${TARGET}" INTERFACE Boost::stacktrace_backtrace backtrace dl)
else()
target_link_libraries("${TARGET}" INTERFACE Boost::stacktrace_basic dl)
Expand Down
2 changes: 1 addition & 1 deletion cmake/UserverRequireDWCAS.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function (userver_target_require_dwcas target visibility)
return()
endif()

if(NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND NOT CMAKE_SYSTEM MATCHES "BSD")
if(NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND NOT CMAKE_SYSTEM MATCHES "BSD" AND NOT WIN32)
list(APPEND TEST_LIBRARIES "atomic")
endif()

Expand Down
9 changes: 7 additions & 2 deletions cmake/UserverVenv.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,12 @@ function(userver_venv_setup)

set(venv_dir "${parent_directory}/${venv_name}")
set(venv_bin_dir "${venv_dir}/bin")
set("${python_output_var}" "${venv_bin_dir}/python" PARENT_SCOPE)
find_program(venv_python
NAMES python python3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd use NO_CACHE, but it appeared in 3.21

It's easier to check WIN32 and either use bin or Scripts based on that

Always using either python or python3 is OK as long as both are present on Windows, choose something that works

PATHS "${venv_dir}/bin" "${venv_dir}/Scripts"
REQUIRED NO_DEFAULT_PATH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently support cmake 3.14+, but REQUIRED appeared in cmake 3.18

)
set("${python_output_var}" "${venv_python}" PARENT_SCOPE)

# A unique venv is set up once for the whole build.
# For example, a userver gRPC cmake script may be included multiple times
Expand Down Expand Up @@ -164,7 +169,7 @@ function(userver_venv_setup)
)
execute_process(
COMMAND
"${venv_bin_dir}/python3" -m pip install
"${venv_python}" -m pip install
--disable-pip-version-check
-U ${pip_requirements}
${ARG_PIP_ARGS}
Expand Down
2 changes: 1 addition & 1 deletion cmake/modules/FindLibEv.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ _userver_module_begin(
)

_userver_module_find_include(
NAMES ev.h
NAMES ev.h libev/ev.h
)

_userver_module_find_library(
Expand Down
4 changes: 2 additions & 2 deletions core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ add_subdirectory(${USERVER_THIRD_PARTY_DIRS}/http-parser http-parser)
target_link_libraries(${PROJECT_NAME} PRIVATE userver-http-parser userver-llhttp)

set(USERVER_UBOOST_CORO_DEFAULT ON)
if(CMAKE_SYSTEM_NAME MATCHES "Darwin" AND CMAKE_SYSTEM_PROCESSOR MATCHES "arm64")
if(CMAKE_SYSTEM_NAME MATCHES "Darwin" AND CMAKE_SYSTEM_PROCESSOR MATCHES "arm64" OR CMAKE_SYSTEM_NAME MATCHES "Windows")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use either CMAKE_SYSTEM_NAME MATCHES "Windows" or WIN32 consistently

# Use system Boost.Context and Boost.Coroutine2 with latest patches
# for arm64, otherwise userver will crash at startup.
#
Expand Down Expand Up @@ -206,7 +206,7 @@ _userver_directory_install(COMPONENT core
)

target_include_directories(${PROJECT_NAME} SYSTEM BEFORE PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/libc_include_fixes>
$<$<NOT:$<CXX_COMPILER_ID:MSVC>>:$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/libc_include_fixes>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably NOT WIN32?

)

# The bug is only triggered with optimizations enabled -- TAXICOMMON-1729
Expand Down
20 changes: 16 additions & 4 deletions universal/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,18 @@ list(REMOVE_ITEM SOURCES ${UNIT_TEST_SOURCES} ${BENCH_SOURCES} ${INTERNAL_SOURCE
set(CMAKE_THREAD_PREFER_PTHREAD ON)
set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)
if (WIN32)
set(userver_boost_stacktrace_component stacktrace_windbg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just find stacktrace_windbg in OPTIONAL_COMPONENTS

else()
set(userver_boost_stacktrace_component stacktrace_basic)
endif()

find_package(Boost REQUIRED
COMPONENTS
program_options
filesystem
regex
stacktrace_basic
${userver_boost_stacktrace_component}
OPTIONAL_COMPONENTS
stacktrace_backtrace
)
Expand Down Expand Up @@ -78,9 +84,15 @@ endif()
add_library(userver-internal-compile-options INTERFACE)
userver_target_require_dwcas(userver-internal-compile-options INTERFACE)
target_compile_features(userver-internal-compile-options INTERFACE cxx_std_17)
target_compile_options(userver-internal-compile-options INTERFACE
"-Wall" "-Wextra" "-Wpedantic"
)
if (MSVC)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no MSVC for now, Clang on Windows is enough :)
We are not ready for MSVC's bugs and lack of certain intrinsics

target_compile_options(userver-internal-compile-options INTERFACE
"/W4" "/sdl"
)
else()
target_compile_options(userver-internal-compile-options INTERFACE
"-Wall" "-Wextra" "-Wpedantic"
)
endif()

include(UserverCxxCompileOptionsIfSupported)
userver_target_cxx_compile_options_if_supported(userver-internal-compile-options INTERFACE
Expand Down
Loading