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

CMake support #19

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
*.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.


backtrace-supported\.h
Copy link
Owner

Choose a reason for hiding this comment

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

Does cmake support building in a separate directory, so that the sources can be read-only? If so, let's not modify .gitignore here.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not really that familiar with autotools but I've built this library using ./configure and make and it generated headers in source directory, as well as .a files. So I thought having .a and not .h in gitignore was a bug, not a feature. If I'm wrong I'll remove this line.

Copy link
Owner

Choose a reason for hiding this comment

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

The expectation is that people will run the equivalent of

mkdir x
cd x
../libbacktrace/configure

rather than running .configure. It's OK to run ./configure, but it shouldn't be necessary, and we need to make sure that running configure from a different directory works without modifying the source directory.

Copy link
Author

Choose a reason for hiding this comment

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

I've reverted my changes in 28d7445

Choose a reason for hiding this comment

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

Does cmake support building in a separate directory, so that the sources can be read-only? If so, let's not modify .gitignore here.

Yes, via this command:

cmake -S <source-dir> -B <build-dir> [optional_arguments]    # configure step
cmake --build <build-dir>     # build step

The old CMake also allows separate build via:

mkdir build
cd build
cmake <path-to-source>


config\.h

106 changes: 106 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
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

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

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
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
${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
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/backtrace-supported.h.cmake
${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}")
51 changes: 51 additions & 0 deletions backtrace-supported.h.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* backtrace-supported.h.cmake -- Whether stack backtrace is supported.
Copy link
Owner

Choose a reason for hiding this comment

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

Is there some way we could generate this file from backtrace-supported.h.in, so that we don't have the unnecessary duplication?

Copy link
Author

Choose a reason for hiding this comment

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

I've checked right now and it's entirely possible. I'll do proper changes right away. config.h.in uses autotools-specific syntax, so unfortunately we need CMake variant.

Copyright (C) 2012-2016 Free Software Foundation, Inc.
Based backtrace-supported.h.in, written by Ian Lance Taylor, Google.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:
(1) Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
(2) Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in
the documentation and/or other materials provided with the
distribution.
(3) The name of the author may not be used to
endorse or promote products derived from this software without
specific prior written permission.
THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT,
INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF SUCH DAMAGE. */
/* The file backtrace-supported.h.in is used by configure to generate
the file backtrace-supported.h. The file backtrace-supported.h may
be #include'd to see whether the backtrace library will be able to
get a backtrace and produce symbolic information. */
/* BACKTRACE_SUPPORTED will be #define'd as 1 if the backtrace library
should work, 0 if it will not. Libraries may #include this to make
other arrangements. */
#cmakedefine01 BACKTRACE_SUPPORTED
/* BACKTRACE_USES_MALLOC will be #define'd as 1 if the backtrace
library will call malloc as it works, 0 if it will call mmap
instead. This may be used to determine whether it is safe to call
the backtrace functions from a signal handler. In general this
only applies to calls like backtrace and backtrace_pcinfo. It does
not apply to backtrace_simple, which never calls malloc. It does
not apply to backtrace_print, which always calls fprintf and
therefore malloc. */
#cmakedefine01 BACKTRACE_USES_MALLOC
/* BACKTRACE_SUPPORTS_THREADS will be #define'd as 1 if the backtrace
library is configured with threading support, 0 if not. If this is
0, the threaded parameter to backtrace_create_state must be passed
as 0. */
#cmakedefine01 BACKTRACE_SUPPORTS_THREADS
/* BACKTRACE_SUPPORTS_DATA will be #defined'd as 1 if the backtrace_syminfo
will work for variables. It will always work for functions. */
#cmakedefine01 BACKTRACE_SUPPORTS_DATA
14 changes: 14 additions & 0 deletions cmake/libbacktraceConfig.cmake
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
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
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