Skip to content

Commit

Permalink
put back pdal_defines generation into ./include/pdal #108
Browse files Browse the repository at this point in the history
  • Loading branch information
hobu committed Jan 21, 2013
1 parent a87a280 commit b8ea198
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 7 deletions.
3 changes: 1 addition & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,7 @@ install(FILES AUTHORS.txt LICENSE.txt

configure_file(
"${PROJECT_SOURCE_DIR}/pdal_defines.h.in"
"${PROJECT_BINARY_DIR}/pdal_defines.h")
include_directories(${PROJECT_BINARY_DIR})
"${PDAL_INCLUDE_DIR}/pdal/pdal_defines.h")

This comment has been minimized.

Copy link
@chambbj

chambbj Jan 22, 2013

Member

This expands to ${PROJECT_BINARY_DIR}/include/pdal/pdal_defines.h, but src/CMakeLists.txt expects pdal_defines.h to be in ${PROJECT_SOURCE_DIR}/include/pdal/. (See line 487 of src/CMakeLists.txt). Running CMake on a clean build directory gives

CMake Error at src/CMakeLists.txt:563 (add_library):
  Cannot find source file:

    /Users/bchambers/dev/pdal-trunk/include/pdal/pdal_defines.h

Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx

#------------------------------------------------------------------------------
# subdirectory controls
Expand Down
5 changes: 1 addition & 4 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ set(PDAL_CONFIG_HPP
${PDAL_HEADERS_DIR}/pdal_export.hpp
${PDAL_HEADERS_DIR}/pdal_internal.hpp
${PDAL_HEADERS_DIR}/pdal_config.hpp
${PROJECT_BINARY_DIR}/pdal_defines.h
${PDAL_HEADERS_DIR}/pdal_defines.h

This comment has been minimized.

Copy link
@chambbj

chambbj Jan 22, 2013

Member

This expands to ${PROJECT_SOURCE_DIR}/include/pdal/pdal_defines.h, but CMakeLists.txt configures pdal_defines.h into ${PROJECT_BINARY_DIR}/include/pdal/ (see line 554 of CMakeLists.txt).

I think the most straightforward fix is to change this to

${PROJECT_BINARY_DIR}/include/pdal/pdal_defines.h
)

set (PDAL_CONFIG_CPP
Expand Down Expand Up @@ -728,6 +728,3 @@ install(TARGETS ${PDAL_LIB_NAME} ${PDAL_C_LIB_NAME} ${PDAL_CSV_WRITER_NAME}
install(DIRECTORY ${PDAL_HEADERS_DIR}
DESTINATION ${PDAL_INCLUDE_DIR}
FILES_MATCHING PATTERN "*.h" PATTERN "*.hpp")

This comment has been minimized.

Copy link
@chambbj

chambbj Jan 22, 2013

Member

With the configured pdal_defines.h residing in ${PROJECT_BINARY_DIR}/include/pdal/ relying on this install is insufficient.


install(FILES "${PROJECT_BINARY_DIR}/pdal_defines.h"
DESTINATION "${PDAL_INCLUDE_DIR}/pdal")

This comment has been minimized.

Copy link
@chambbj

chambbj Jan 22, 2013

Member

A modified version of this needs to be reinstated for library users to have access to the configured pdal_defines.h.

install(FILES "${PROJECT_BINARY_DIR}/include/pdal/pdal_defines.h"
        DESTINATION "${PDAL_INCLUDE_DIR}/pdal")
2 changes: 1 addition & 1 deletion src/pdal_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#include <pdal/pdal_config.hpp>

#include <sstream>
#include "pdal_defines.h"
#include <pdal/pdal_defines.h>

#ifdef PDAL_HAVE_LIBGEOTIFF
#include <geotiff.h>
Expand Down

2 comments on commit b8ea198

@hobu
Copy link
Member Author

@hobu hobu commented on b8ea198 Jan 22, 2013

Choose a reason for hiding this comment

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

Please push the changes you suggest.

@chambbj
Copy link
Member

Choose a reason for hiding this comment

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

Pull request #115

Please sign in to comment.