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.

Sign up for GitHub

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

More distro friendly cmake fixes. #655

Merged
merged 1 commit into from
Nov 13, 2022
Merged

More distro friendly cmake fixes. #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ option(SLANG_FUZZ_TARGET "Enables changes to make binaries easier to fuzz test"
OFF)
option(BUILD_SHARED_LIBS "Generate shared libraries instead of static" OFF)

set(SLANG_LIB_NAME
"slang"
CACHE STRING "Default output library name")

set(SLANG_CLANG_TIDY
""
CACHE STRING "Run clang-tidy during the build with the given binary")
Expand Down Expand Up @@ -154,6 +158,59 @@ if(NOT CMAKE_INSTALL_RPATH AND BUILD_SHARED_LIBS)
set(CMAKE_INSTALL_RPATH ${base} ${base}/${relDir})
endif()

set(STD_FS_LIB "")
# ==== The test below is based on:
Copy link
Owner

Choose a reason for hiding this comment

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

Do you have an example compiler where this is still needed? Note that slang does not build with a pre-C++17 compiler, and the minimum supported gcc version is 9 (and clang is 10) neither of which need the separate lib. In a few months I will bump the minimum gcc version to 10 and start requiring C++20 support so I'm wondering if there's some old system that will be broken if not supporting an older version.

Copy link
Contributor Author

@cbalint13 cbalint13 Nov 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

  • Yes, RHEL 8 (having gcc 8.x ) will fail without this.
  • Not tested (myself) with other combinations outside fedora/rhel products.

A well proven routine is used from pybind11 library.

Copy link
Owner

Choose a reason for hiding this comment

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

OK, I can merge this for now, but please be aware that I don't have the ability to maintain the codebase for very old compilers and so the build may or may not work for GCC 8 (and as mentioned I will be raising the minimum required C++ version to 20 early in 2023) and may be broken by any future changes that I make. Even the GCC team does not support GCC 8 anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Currently rhel8 is fine, in case you start require c++20, that's it, will drop out rhel8 support.
  • There would be still some releng way to invoke gcc 10 even on RHEL 8, will update packaging at that time.

If interested in rhel/fedora native repo having sv-lang the automated (weekly) builds are here .

# https://github.com/pybind/pybind11/blob/master/tests/CMakeLists.txt
#
# It is under a BSD-style license, see:
# https://github.com/pybind/pybind11/blob/master/LICENSE
#
# Check if we need to add -lstdc++fs or -lc++fs or nothing
if(DEFINED CMAKE_CXX_STANDARD AND CMAKE_CXX_STANDARD LESS 17)
set(STD_FS_NO_LIB_NEEDED TRUE)
elseif(MSVC)
set(STD_FS_NO_LIB_NEEDED TRUE)
else()
file(
WRITE ${CMAKE_CURRENT_BINARY_DIR}/main.cpp
"#include <filesystem>\nint main(int argc, char ** argv) {\n std::filesystem::path p(argv[0]);\n return p.string().length();\n}"
)
try_compile(
STD_FS_NO_LIB_NEEDED ${CMAKE_CURRENT_BINARY_DIR}
SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp
COMPILE_DEFINITIONS -std=c++17)
try_compile(
STD_FS_NEEDS_STDCXXFS ${CMAKE_CURRENT_BINARY_DIR}
SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp
COMPILE_DEFINITIONS -std=c++17
LINK_LIBRARIES stdc++fs)
try_compile(
STD_FS_NEEDS_CXXFS ${CMAKE_CURRENT_BINARY_DIR}
SOURCES ${CMAKE_CURRENT_BINARY_DIR}/main.cpp
COMPILE_DEFINITIONS -std=c++17
LINK_LIBRARIES c++fs)
endif()

if(STD_FS_NEEDS_STDCXXFS)
set(STD_FS_LIB stdc++fs)
elseif(STD_FS_NEEDS_CXXFS)
set(STD_FS_LIB c++fs)
elseif(STD_FS_NO_LIB_NEEDED)
message(WARNING "Unknown C++17 compiler - not passing -lstdc++fs")
endif()
# ==== (end of test for stdc++fs)

if(NOT "${STD_FS_LIB}")
cbalint13 marked this conversation as resolved.
Show resolved Hide resolved
message(STATUS "Linking with: ${STD_FS_LIB}")
list(APPEND SLANG_LIBRARIES ${STD_FS_LIB})
endif()

if(BUILD_SHARED_LIBS)
message(STATUS "Build SHARED library as: ${SLANG_LIB_NAME}")
else()
message(STATUS "Build STATIC library as: ${SLANG_LIB_NAME}")
endif()

include(external/CMakeLists.txt)
add_subdirectory(source)

Expand Down
2 changes: 1 addition & 1 deletion bindings/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pybind11_add_module(
python/types.cpp
python/util.cpp
${CMAKE_CURRENT_BINARY_DIR}/PySyntaxBindings.cpp)
target_link_libraries(pyslang PUBLIC slang::slang fmt::fmt)
target_link_libraries(pyslang PUBLIC slang::slang fmt::fmt ${SLANG_LIBRARIES})
target_include_directories(pyslang PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/python)
target_compile_definitions(pyslang PRIVATE VERSION_INFO=${PROJECT_VERSION})

Expand Down
12 changes: 6 additions & 6 deletions source/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,15 @@ add_library(

add_subdirectory(ast)

set(SLANG_SHARED_LIB_NAME slang)
add_library(slang::slang ALIAS slang_slang)
set_target_properties(
slang_slang
PROPERTIES CXX_VISIBILITY_PRESET hidden
VISIBILITY_INLINES_HIDDEN YES
VERSION ${PROJECT_VERSION}
SOVERSION ${PROJECT_VERSION_MAJOR}
EXPORT_NAME ${SLANG_SHARED_LIB_NAME}
OUTPUT_NAME ${SLANG_SHARED_LIB_NAME})
EXPORT_NAME ${SLANG_LIB_NAME}
OUTPUT_NAME ${SLANG_LIB_NAME})

# Compile options
target_compile_options(slang_slang PRIVATE ${SLANG_WARN_FLAGS})
Expand All @@ -113,9 +112,10 @@ target_include_directories(

# Link libraries
find_package(Threads)
target_link_libraries(slang_slang PRIVATE ${CMAKE_THREAD_LIBS_INIT})
target_link_libraries(slang_slang PRIVATE fmt::fmt)
target_link_libraries(slang_slang PUBLIC unordered_dense::unordered_dense)
target_link_libraries(
slang_slang
PRIVATE fmt::fmt ${CMAKE_THREAD_LIBS_INIT} ${SLANG_LIBRARIES}
PUBLIC unordered_dense::unordered_dense)

# If building the Python library we'll end up with a shared lib no matter what,
# so make sure we always build with PIC.
Expand Down
5 changes: 4 additions & 1 deletion tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ add_executable(
unittests/TypeTests.cpp
unittests/VisitorTests.cpp)

target_link_libraries(unittests PRIVATE slang::slang Catch2::Catch2 fmt::fmt)
target_link_libraries(
unittests
PRIVATE slang::slang Catch2::Catch2 fmt::fmt
PUBLIC ${SLANG_LIBRARIES})
target_compile_definitions(unittests PRIVATE UNITTESTS)

if(SLANG_CI_BUILD)
Expand Down
15 changes: 12 additions & 3 deletions tools/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,27 @@
add_executable(slang_driver driver/driver.cpp)
add_executable(slang::driver ALIAS slang_driver)

target_link_libraries(slang_driver PRIVATE slang::slang fmt::fmt)
target_link_libraries(
slang_driver
PRIVATE slang::slang fmt::fmt
PUBLIC ${SLANG_LIBRARIES})
set_target_properties(slang_driver PROPERTIES OUTPUT_NAME "slang")

add_executable(rewriter rewriter/rewriter.cpp)
target_link_libraries(rewriter PRIVATE slang::slang)
target_link_libraries(
rewriter
PRIVATE slang::slang
PUBLIC ${SLANG_LIBRARIES})

if(SLANG_FUZZ_TARGET)
message("Tweaking driver for fuzz testing")
target_compile_definitions(slang_driver PRIVATE FUZZ_TARGET)

target_compile_options(slang_driver PRIVATE "-fsanitize=fuzzer")
target_link_libraries(slang_driver PRIVATE "-fsanitize=fuzzer")
target_link_libraries(
slang_driver
PRIVATE "-fsanitize=fuzzer"
PUBLIC ${SLANG_LIBRARIES})
endif()

if(SLANG_INCLUDE_INSTALL)
Expand Down