From 987839a0518d757c4c5554bf7b89367870dc24f1 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 12 Jul 2016 11:59:21 +0300 Subject: [PATCH 1/2] Add a config.h, containing GPOS_DEBUG and other flags that affect ABI. Before this patch, consumers of libgpos had to know out-of-band which flags were used to compile libgpos, because e.g. libgpos compiled with GPOS_DEBUG would only work if the application using libgpos was also compiled with GPOS_DEBUG. This is because many of the structs differ depending on GPOS_DEBUG. Same for the architecture flags, like GPOS_i386. The new config.h file is #included from a few central other header files, to make sure it gets included in any application that uses other gpos headers. We probably should include config.h from *all* other gpos header files, to be sure, but this seems to be enough for ORCA and GPDB at least. --- CMakeLists.txt | 23 ++++++++++------------- config.h.in | 26 ++++++++++++++++++++++++++ libgpos/include/gpos/_api.h | 2 ++ libgpos/include/gpos/assert.h | 1 + 4 files changed, 39 insertions(+), 13 deletions(-) create mode 100644 config.h.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 2cd2f8d..8b4305b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -108,13 +108,6 @@ set_property( DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS $<$:GPOS_DEBUG>) -# Turn on platform-specific defines. -set_property( - DIRECTORY - APPEND PROPERTY COMPILE_DEFINITIONS - GPOS_${CMAKE_SYSTEM_NAME} - GPOS_${CMAKE_SYSTEM_PROCESSOR}) - # Autodetect bit-width if not already set by toolchain file. if (NOT GPOS_ARCH_BITS) # Autodetect bit-width. @@ -133,11 +126,6 @@ if (NOT GPOS_ARCH_BITS) endif() endif() -set_property( - DIRECTORY - APPEND PROPERTY COMPILE_DEFINITIONS - GPOS_${GPOS_ARCH_BITS}BIT) - # Need pthreads. find_package(Threads REQUIRED) if (NOT CMAKE_USE_PTHREADS_INIT) @@ -158,9 +146,14 @@ endif() include_directories(libgpos/include) -# Generate version-number header. +# Generate version-number and config headers. configure_file(version.h.in ${PROJECT_BINARY_DIR}/libgpos/include/gpos/version.h) +configure_file(config.h.in + ${PROJECT_BINARY_DIR}/libgpos/include/gpos/config.h) + +# Make the generated header files available at compilation of libgpos itself. +include_directories(${PROJECT_BINARY_DIR}/libgpos/include/) # Compile .cpp source files under libgpos/src into monolithic gpos library. add_library(gpos @@ -432,12 +425,16 @@ if (VERBOSE_INSTALL_PATH) install(DIRECTORY libgpos/include/gpos DESTINATION "${installpath}/include") install(FILES "${PROJECT_BINARY_DIR}/libgpos/include/gpos/version.h" DESTINATION "${installpath}/include/gpos") + install(FILES "${PROJECT_BINARY_DIR}/libgpos/include/gpos/config.h" + DESTINATION "${installpath}/include/gpos") else() get_filename_component(full_install_name_dir "${CMAKE_INSTALL_PREFIX}/lib" ABSOLUTE) install(TARGETS gpos DESTINATION lib) install(DIRECTORY libgpos/include/gpos DESTINATION include) install(FILES "${PROJECT_BINARY_DIR}/libgpos/include/gpos/version.h" DESTINATION include/gpos) + install(FILES "${PROJECT_BINARY_DIR}/libgpos/include/gpos/config.h" + DESTINATION include/gpos) endif() # Mac OS X handles searching for dynamic libraries differently from other diff --git a/config.h.in b/config.h.in new file mode 100644 index 0000000..b15a9b9 --- /dev/null +++ b/config.h.in @@ -0,0 +1,26 @@ +//--------------------------------------------------------------------------- +// Greenplum Database +// Copyright (C) 2016 Greenplum, Inc. +// +// @filename: +// config.h +// +// @doc: +// Various compile-time options that affect binary +// compatibility. +// +//--------------------------------------------------------------------------- +#ifndef GPOS_config_H +#define GPOS_config_H + +#cmakedefine GPOS_DEBUG 1 + +// Thes comes from CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_PROCESSOR +#define GPOS_${CMAKE_SYSTEM_NAME} 1 +#define GPOS_${CMAKE_SYSTEM_PROCESSOR} 1 + +// This comes from GPOS_ARCH_BITS (based on CMAKE_SIZEOF_VOID_P) +#define GPOS_${GPOS_ARCH_BITS}BIT 1 + +#endif // GPOS_config_H +// EOF diff --git a/libgpos/include/gpos/_api.h b/libgpos/include/gpos/_api.h index 2da7c2d..c83f867 100644 --- a/libgpos/include/gpos/_api.h +++ b/libgpos/include/gpos/_api.h @@ -17,6 +17,8 @@ #ifndef GPOS_api_H #define GPOS_api_H +#include "gpos/config.h" + #ifdef __cplusplus extern "C" { diff --git a/libgpos/include/gpos/assert.h b/libgpos/include/gpos/assert.h index 16bd5be..00e2d31 100644 --- a/libgpos/include/gpos/assert.h +++ b/libgpos/include/gpos/assert.h @@ -22,6 +22,7 @@ #ifndef GPOS_assert_H #define GPOS_assert_H +#include "gpos/config.h" // retail assert; available in all builds #define GPOS_RTL_ASSERT(x) ((x) ? ((void)0) : \ From a80c4627f34431d9f910bc8fb8a63bf12a9fe3b2 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 15 Aug 2016 20:35:26 +0300 Subject: [PATCH 2/2] Also set GPOS_DEBUG, if CMAKE_BUILD_TYPE is Debug. --- CMakeLists.txt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8b4305b..251dc7a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,11 @@ if(NOT CMAKE_BUILD_TYPE) endif(NOT CMAKE_BUILD_TYPE) message(STATUS "Build type: ${CMAKE_BUILD_TYPE}") +# Turn on GPOS_DEBUG define for DEBUG builds. +if (CMAKE_BUILD_TYPE MATCHES "Debug") + set(GPOS_DEBUG 1) +endif() + # Override CMAKE_SYSTEM_PROCESSOR if it has been explicitly set in a toolchain file. if (FORCED_CMAKE_SYSTEM_PROCESSOR) set(CMAKE_SYSTEM_PROCESSOR ${FORCED_CMAKE_SYSTEM_PROCESSOR}) @@ -102,12 +107,6 @@ if (COMPILER_HAS_FNO_OMIT_FRAME_POINTER) SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer") endif() -# Turn on GPOS_DEBUG define for DEBUG builds. -cmake_policy(SET CMP0043 NEW) -set_property( - DIRECTORY - APPEND PROPERTY COMPILE_DEFINITIONS $<$:GPOS_DEBUG>) - # Autodetect bit-width if not already set by toolchain file. if (NOT GPOS_ARCH_BITS) # Autodetect bit-width.