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

CMake support #19

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

CMake support #19

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
72ce7c6
CMake support for MinGW toolchain
tehKaiN Jul 16, 2018
b08cb33
generated headers ar now placed in build dir, added -frandom-seed note
tehKaiN Jul 17, 2018
9f5df7e
Removed backtrace-supported.h.cmake
tehKaiN Jul 17, 2018
28d7445
Removed things I've added to .gitignore
tehKaiN Jul 18, 2018
cd6f82f
fix for builds outside source tree
tehKaiN Feb 8, 2019
b3b27a5
removed GLOB 'cuz it messes with backtrace-supported.h install
tehKaiN Feb 11, 2019
fa2fe5e
cmake compile & install works on ubuntu 16.04
tehKaiN Feb 11, 2019
e653f86
fix for case where ${CMAKE_EXECUTABLE_FORMAT} isn't set to any value
tehKaiN Feb 21, 2019
c6c1ce2
add copyright
AllanZyne Mar 20, 2024
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
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
*.o
*.lo
*.a
*.la
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure what is changing here--no reason for .gitignore to change at all.

*.la
109 changes: 109 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
cmake_minimum_required(VERSION 3.5)
Copy link
Owner

Choose a reason for hiding this comment

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

I suppose we should add copyright header like the one in configure.ac.

Copy link

@jcfr jcfr Aug 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Since this project is just being converted to CMake, I suggest to use the latest 3.12.1

Copy link
Author

Choose a reason for hiding this comment

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

I'll change CMake version, good point.

Regarding project name, I thought so too, but then it occurred to me that most CMake-packaged libs omit "lib" in their name - you type find_library(curl) instead of find_library(libcurl). Also, IIRC if you tell cmake to find_library(blah) it will look for libblah.a automatically, so find_library(libbacktrace) would result inf looking for liblibbacktrace.a. I also wonder, for which package is FindBacktrace designed? What do you think about my points?


project(backtrace)
Copy link
Owner

Choose a reason for hiding this comment

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

backtrace or libbacktrace? People usually say libbacktrace, and that is what autoconf uses. Should probably libbacktrace unless there is some reason not to.

Copy link

@jcfr jcfr Aug 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I suggest to use libbacktrace, that way it will be different from the FindBacktrace CMake module associated with backtrace(3)


# Automake uses -frandom-seed initialized with file name of given file
# but AFAIK it can't be done on CMake, so here's always same seed
Copy link

Choose a reason for hiding this comment

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

This should be possible, which file is expected to use as seed ?

Copy link
Author

Choose a reason for hiding this comment

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

According to automake each file was using it's own file for seed. I don't think there's strong reason for that, so I guess we can just use same seed for every file.

set(CMAKE_CXX_FLAGS "-DHAVE_CONFIG_H -funwind-tables -frandom-seed=mySeed -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -g -O2")
set(CMAKE_C_FLAGS "-DHAVE_CONFIG_H -funwind-tables -frandom-seed=mySeed -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -g -O2")
Copy link

Choose a reason for hiding this comment

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

Public, private and interface flags should be differentiated and directly associated with the library target. For example:

target_compile_options(${PROJECT_NAME} PRIVATE -funwind-tables -frandom-seed=mySeed -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual)

  • If there are compile option that must be used by project using the library, they should be declared as INTERFACE. INTERFACE means that project linking against the libbacktrace imported target will transitively use the flag.

  • HAVE_CONFIG_H is not used in the project and should probably be removed.

  • -g -O2 flags are automatically listed when building the project with RELWITHDEBINFO build type. Is this a strong requirement ?

The default flags for each build type are available here:

https://github.com/Kitware/CMake/blob/da3dc2f0cfb8e2aed207c21e419a60525eea0c6f/Modules/Compiler/GNU.cmake#L42-L47


file(GLOB sources
tehKaiN marked this conversation as resolved.
Show resolved Hide resolved
atomic.c dwarf.c fileline.c posix.c print.c sort.c state.c backtrace.c
simple.c pecoff.c read.c alloc.c config.h
)
file(GLOB export_headers
tehKaiN marked this conversation as resolved.
Show resolved Hide resolved
${CMAKE_CURRENT_BINARY_DIR}/backtrace.h
${CMAKE_CURRENT_BINARY_DIR}/backtrace-supported.h
)

add_library(${PROJECT_NAME} ${sources} ${export_headers})
include_directories(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR})
Copy link

Choose a reason for hiding this comment

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

Instead, prefer setting the variable set(CMAKE_INCLUDE_CURRENT_DIR ON) at the top of the project


if(CMAKE_COMPILER_IS_GNUCC)
set(BACKTRACE_SUPPORTED 1)

# Assume multi-threaded environment
set(BACKTRACE_SUPPORTS_THREADS 1)

# Assume ELF/DWARF, meaning that BACKTRACE_SUPPORTS_DATA is hard-coded on.
Copy link
Owner

Choose a reason for hiding this comment

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

This seems like an unfortunate assumption, since we have PE and XCOFF support, and we really need Mach-O support. Do people not use cmake on those systems? Though I suppose we could also fix this up later.

Copy link
Author

Choose a reason for hiding this comment

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

I vote for adding things later, since I've never worked with XCOFF or Mach-O files and platforms supporting them. Therefore I can't add support to them, so I guess there's little more options than putting notice in README that CMake support is in it's infancy.

Copy link
Author

Choose a reason for hiding this comment

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

fixed in 9f5df7e

set(BACKTRACE_SUPPORTS_DATA 1)

find_package(ZLIB)
if(ZLIB_FOUND)
SET(HAVE_LIBZ 1)
SET(HAVE_ZLIB 1)
target_link_libraries(${PROJECT_NAME} z)
Copy link

Choose a reason for hiding this comment

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

target_link_libraries(${PROJECT_NAME} ZLIB::ZLIB)

else()
SET(HAVE_LIBZ 0)
SET(HAVE_ZLIB 0)
endif()

if(WIN32)
# Typical MinGW config
# DWARF2 exception handling could be detected based on parsing gcc --version
set(BACKTRACE_USES_MALLOC 1)
SET(BACKTRACE_ELF_SIZE unused)
SET(BACKTRACE_XCOFF_SIZE unused)
SET(HAVE_ATOMIC_FUNCTIONS 1)
SET(HAVE_CLOCK_GETTIME 1)
SET(HAVE_DECL_STRNLEN 1)
SET(HAVE_DLFCN_H 0)
SET(HAVE_DL_ITERATE_PHDR 0)
SET(HAVE_FCNTL 0)
SET(HAVE_GETEXECNAME 0)
SET(HAVE_GETIPINFO 1)
SET(HAVE_INTTYPES_H 1)
SET(HAVE_LINK_H 0)
SET(HAVE_LOADQUERY 0)
SET(HAVE_LSTAT 0)
SET(HAVE_MEMORY_H 1)
SET(HAVE_READLINK 0)
SET(HAVE_STDINT_H 1)
SET(HAVE_STDLIB_H 1)
SET(HAVE_STRINGS_H 1)
SET(HAVE_STRING_H 1)
SET(HAVE_SYNC_FUNCTIONS 1)
SET(HAVE_SYS_LDR_H 0)
SET(HAVE_SYS_MMAN_H 0)
SET(HAVE_SYS_STAT_H 1)
SET(HAVE_SYS_TYPES_H 1)
SET(HAVE_UNISTD_H 1)
else()
set(BACKTRACE_SUPPORTED 0)
endif()
else()
set(BACKTRACE_SUPPORTED 0)
endif()

# Generate backtrace-supported.h and config.h
# backtrace-supported.h.in has syntax which works with CMake out of the box so
# let's not duplicate things unnecessarily.
# config.h.in ain't parsed properly so we need slightly different version.
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/backtrace-supported.h.in
${CMAKE_CURRENT_BINARY_DIR}/backtrace-supported.h
)
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake
${CMAKE_CURRENT_BINARY_DIR}/config.h
)

#install libbacktrace and header files
set(INSTALL_LIB_DIR lib/libbacktrace)
set(INSTALL_INCLUDE_DIR include/libbacktrace)
set(INSTALL_CMAKE_DIR CMake)

# Install CMake files
install(TARGETS ${PROJECT_NAME}
DESTINATION ${INSTALL_LIB_DIR}
EXPORT lib${PROJECT_NAME}-targets
)
install(EXPORT lib${PROJECT_NAME}-targets DESTINATION ${INSTALL_CMAKE_DIR})
install(FILES
${CMAKE_SOURCE_DIR}/cmake/lib${PROJECT_NAME}Config.cmake
${CMAKE_SOURCE_DIR}/cmake/lib${PROJECT_NAME}ConfigVersion.cmake
DESTINATION ${INSTALL_CMAKE_DIR}
)

install(TARGETS ${PROJECT_NAME} DESTINATION "${INSTALL_LIB_DIR}")
install(FILES ${export_headers} DESTINATION "${INSTALL_INCLUDE_DIR}")
14 changes: 14 additions & 0 deletions cmake/libbacktraceConfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Usage:
Copy link
Owner

Choose a reason for hiding this comment

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

Add a copyright header.

#
#find_package(libbacktrace REQUIRED)
#include_directories(${libbacktrace_INCLUDE_DIRS})
#target_link_libraries(app libbacktrace)

if(libbacktrace_CONFIG_INCLUDED)
return()
endif()
set(libbacktrace_CONFIG_INCLUDED TRUE)

get_filename_component(SELF_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH)
include(${SELF_DIR}/libbacktrace-targets.cmake)
get_filename_component(libbacktrace_INCLUDE_DIRS "${SELF_DIR}/.." ABSOLUTE)
21 changes: 21 additions & 0 deletions cmake/libbacktraceConfigVersion.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
set(PACKAGE_VERSION "1.0.0")
Copy link
Owner

Choose a reason for hiding this comment

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

Add a copyright header.

Copy link

Choose a reason for hiding this comment

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

Use CMakePackageConfigHelpers to automatically generate libbacktraceConfig.cmake (from libbacktraceConfig.cmake.in) and libbacktraceConfigVersion.cmake.


if(PACKAGE_VERSION VERSION_LESS PACKAGE_FIND_VERSION)
set(PACKAGE_VERSION_COMPATIBLE FALSE)
else()
if(PACKAGE_VERSION MATCHES "^([0-9]+)\\.")
set(CVF_VERSION_MAJOR "${CMAKE_MATCH_1}")
else()
set(CVF_VERSION_MAJOR PACKAGE_VERSION)
endif()

if(PACKAGE_FIND_VERSION_MAJOR STREQUAL CVF_VERSION_MAJOR)
set(PACKAGE_VERSION_COMPATIBLE TRUE)
else()
set(PACKAGE_VERSION_COMPATIBLE FALSE)
endif()

if(PACKAGE_FIND_VERSION STREQUAL PACKAGE_VERSION)
set(PACKAGE_VERSION_EXACT TRUE)
endif()
endif()
58 changes: 58 additions & 0 deletions config.h.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/* config.h.cmake */
Copy link
Owner

Choose a reason for hiding this comment

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

Similarly, can we generate this from config.h.in?

Copy link

Choose a reason for hiding this comment

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

config.h.in is useless from the perspective of cmake, as all of the entries in it are of the form #undef .....

A separate pre-processing step would be needed to transform the file into something cmake can use.

Copy link

Choose a reason for hiding this comment

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

Does this work?

file(READ "config.h.in" original_config_h_in)
string(REGEX REPLACE "#undef" "#cmakedefine" new_config_h_cmake "${original_config_h_in}")
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/config.h.cmake" "${new_config_h_cmake}")
configure_file("${CMAKE_CURRENT_BINARY_DIR}/config.h.cmake" "${CMAKE_CURRENT_BINARY_DIR}/config.h")

/* ELF size: 32 or 64 */
#cmakedefine BACKTRACE_ELF_SIZE
/* XCOFF size: 32 or 64 */
#cmakedefine BACKTRACE_XCOFF_SIZE
/* Define to 1 if you have the __atomic functions */
#cmakedefine HAVE_ATOMIC_FUNCTIONS 1
/* Define to 1 if you have the `clock_gettime' function. */
#cmakedefine HAVE_CLOCK_GETTIME 1
/* Define to 1 if you have the declaration of `strnlen', and to 0 if you
don't. */
#cmakedefine HAVE_DECL_STRNLEN 1
/* Define to 1 if you have the <dlfcn.h> header file. */
#cmakedefine HAVE_DLFCN_H 1
/* Define if dl_iterate_phdr is available. */
#cmakedefine HAVE_DL_ITERATE_PHDR 1
/* Define to 1 if you have the fcntl function */
#cmakedefine HAVE_FCNTL 1
/* Define if getexecname is available. */
#cmakedefine HAVE_GETEXECNAME 1
/* Define if _Unwind_GetIPInfo is available. */
#cmakedefine HAVE_GETIPINFO 1
/* Define to 1 if you have the <inttypes.h> header file. */
#cmakedefine HAVE_INTTYPES_H 1
/* Define to 1 if you have the `z' library (-lz). */
#cmakedefine HAVE_LIBZ 1
/* Define to 1 if you have the <link.h> header file. */
#cmakedefine HAVE_LINK_H 1
/* Define if AIX loadquery is available. */
#cmakedefine HAVE_LOADQUERY 1
/* Define to 1 if you have the `lstat' function. */
#cmakedefine HAVE_LSTAT 1
/* Define to 1 if you have the <memory.h> header file. */
#cmakedefine HAVE_MEMORY_H 1
/* Define to 1 if you have the `readlink' function. */
#cmakedefine HAVE_READLINK 1
/* Define to 1 if you have the <stdint.h> header file. */
#cmakedefine HAVE_STDINT_H 1
/* Define to 1 if you have the <stdlib.h> header file. */
#cmakedefine HAVE_STDLIB_H 1
/* Define to 1 if you have the <strings.h> header file. */
#cmakedefine HAVE_STRINGS_H 1
/* Define to 1 if you have the <string.h> header file. */
#cmakedefine HAVE_STRING_H 1
/* Define to 1 if you have the __sync functions */
#cmakedefine HAVE_SYNC_FUNCTIONS 1
/* Define to 1 if you have the <sys/ldr.h> header file. */
#cmakedefine HAVE_SYS_LDR_H 1
/* Define to 1 if you have the <sys/mman.h> header file. */
#cmakedefine HAVE_SYS_MMAN_H 1
/* Define to 1 if you have the <sys/stat.h> header file. */
#cmakedefine HAVE_SYS_STAT_H 1
/* Define to 1 if you have the <sys/types.h> header file. */
#cmakedefine HAVE_SYS_TYPES_H 1
/* Define to 1 if you have the <unistd.h> header file. */
#cmakedefine HAVE_UNISTD_H 1
/* Define if -lz is available. */
#cmakedefine HAVE_ZLIB 1