Skip to content

Several CMake modernization and MSVC compile fix #5

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

Open
SunBlack wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Several CMake modernization and MSVC compile fix #5

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
79 changes: 31 additions & 48 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
# patches are most welcome! Please send them to the development mailing list.


cmake_minimum_required(VERSION 3.6 FATAL_ERROR)
cmake_minimum_required(VERSION 3.6...4.2 FATAL_ERROR)

@dg0yt dg0yt Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Only choose 4.2 if you actually verified that it does the right thing with CMake policies for 4.2 across all platforms and configurations.

@SunBlack SunBlack Feb 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Tested it with 4.2 locally and as this port didn't required any adjustment in the CMake 4.2-PR-Drafts in vcpkg I don't assume there is any breakage in other platforms.


set (CMAKE_CXX_STANDARD 14)
set (CMAKE_CXX_STANDARD_REQUIRED 11)
Expand Down Expand Up @@ -82,46 +82,30 @@ find_package(Threads REQUIRED)

# Find the Boost and Xerces libraries

set(Boost_USE_MULTITHREADED ON)
find_package(Boost
find_package(Boost CONFIG REQUIRED
COMPONENTS
smart_ptr
crc
math
program_options
thread
filesystem
variant
format
lexical_cast
uuid
OPTIONAL_COMPONENTS
system
)
if (NOT Boost_FOUND)
set(BOOST_ROOT CACHE PATH "Location of the boost root directory" )
message(FATAL_ERROR
"Unable to find boost library.
Please set the BOOST_ROOT to point to the boost distribution files."
)
endif(NOT Boost_FOUND)

find_package(Xerces)
if (NOT Xerces_FOUND)
set(XERCES_ROOT CACHE PATH "Location of the xerces library")
message(FATAL_ERROR
"Unable to find xerces library.
Please set the the XERCES_ROOT to point to the root of the xerces directory."
)
endif (NOT Xerces_FOUND)

set(XML_LIBRARIES ${Xerces_LIBRARY})
set(XML_INCLUDE_DIRS ${Xerces_INCLUDE_DIR})

find_package(XercesC REQUIRED)

if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
add_definitions(-DLINUX)
find_package(ICU REQUIRED)
set(XML_LIBRARIES ${XML_LIBRARIES} ${ICU_LIBRARIES})
set(XML_INCLUDE_DIRS ${XML_INCLUDE_DIRS} ${ICU_INCLUDE_DIRS})
elseif(${CMAKE_SYSTEM_NAME} STREQUAL "Windows")
add_definitions(-DWINDOWS)
endif()

add_definitions(${Boost_LIB_DIAGNOSTIC_DEFINITIONS})
add_definitions(-DBOOST_ALL_NO_LIB -DXERCES_STATIC_LIBRARY)
add_definitions(-DE57_REFIMPL_REVISION_ID=${PROJECT_NAME}-${${PROJECT_NAME}_MAJOR_VERSION}.${${PROJECT_NAME}_MINOR_VERSION}.${${PROJECT_NAME}_BUILD_VERSION}-${${PROJECT_NAME}_BUILD_TAG})

configure_file (
Expand All @@ -134,12 +118,6 @@ include_directories(
include
include/time_conversion
src/refimpl
${XML_INCLUDE_DIRS}
${Boost_INCLUDE_DIR}
)

link_directories(
${Boost_LIBRARY_DIRS}
)


Expand All @@ -161,6 +139,15 @@ set_target_properties( E57RefImpl
PROPERTIES DEBUG_POSTFIX "-d"
)

target_link_libraries( E57RefImpl
PUBLIC
Boost::crc
Boost::smart_ptr
Boost::uuid
Threads::Threads

@dg0yt dg0yt Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Threads::Threads isn't mirrored in the installed CMake config.

@SunBlack SunBlack Feb 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

See notes above:

Note: Didnt't tried whether Threads::Threads is really necessary - just switched to imported target for this.

XercesC::XercesC
)

#
# Files for LAS format support
#
Expand All @@ -170,6 +157,11 @@ add_library( LASReader STATIC
include/LASReader.h
)

target_link_libraries( LASReader
PUBLIC
E57RefImpl
)

#
# Time conversion utilities
#
Expand All @@ -191,16 +183,12 @@ add_executable( DemoWrite01
)
target_link_libraries( DemoWrite01
E57RefImpl
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
)
add_executable( DemoRead01
src/examples/DemoRead01.cpp
)
target_link_libraries( DemoRead01
E57RefImpl
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
)

#
Expand All @@ -214,41 +202,36 @@ target_link_libraries( las2e57
E57RefImpl
LASReader
time_conversion
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
)
add_executable( e57fields
src/tools/e57fields.cpp
)
target_link_libraries( e57fields
E57RefImpl
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
Boost::smart_ptr
)
add_executable( e57xmldump
src/tools/e57xmldump.cpp
)
target_link_libraries( e57xmldump
E57RefImpl
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
)
add_executable( e57validate
src/tools/e57validate.cpp
)
target_link_libraries( e57validate
E57RefImpl
${XML_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
)
add_executable( e57unpack
src/tools/e57unpack.cpp
)
target_link_libraries( e57unpack
E57RefImpl
${XML_LIBRARIES}
${Boost_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT}
Boost::filesystem
Boost::format
Boost::lexical_cast
Boost::program_options
Boost::variant
)

#
Expand Down
82 changes: 0 additions & 82 deletions cmake/Modules/FindICU.cmake
View file
Open in desktop

This file was deleted.

84 changes: 0 additions & 84 deletions cmake/Modules/FindXerces.cmake
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions include/E57Simple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
#include "E57Foundation.h"
#endif

#ifdef WIN32

@dg0yt dg0yt Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
#ifdef WIN32
#ifdef _WIN32

@SunBlack SunBlack Feb 4, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not in this case, as this definition only corresponds to the one below and is therefore consistent in this way. This library requires a major modernization and a of it would be to refactor the usage of macros, but that would have only inflated the PR further and therefore I adjusted it to the current style.

#include <windows.h>
#endif

using namespace std;
using namespace boost;

Expand Down
13 changes: 12 additions & 1 deletion src/refimpl/E57RefImplConfig.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,17 @@ IF (E57RefImpl_LIBRARY_DEBUG AND NOT E57RefImpl_LIBRARY_RELEASE)
SET(E57RefImpl_LIBRARIES ${E57RefImpl_LIBRARY_DEBUG})
ENDIF()

include(CMakeFindDependencyMacro)
find_dependency(Boost COMPONENTS crc smart_ptr uuid)
find_dependency(XercesC)

list(APPEND E57RefImpl_LIBRARIES
Boost::crc
Boost::smart_ptr
Boost::uuid
XercesC::XercesC
)

IF (E57RefImpl_LIBRARY)
set(E57RefImpl_LIBRARY ${E57RefImpl_LIBRARY} CACHE FILEPATH "The E57RefImpl library")
# Remove superfluous "debug" / "optimized" keywords from
Expand All @@ -134,7 +145,7 @@ ELSE(E57RefImpl_LIBRARY)
ENDIF(E57RefImpl_LIBRARY)

IF (E57RefImpl_FOUND)
SET(E57RefImpl_INCLUDE_DIRS ${E57RefImpl_INCLUDE_DIR} "E57RefImpl include directory")
SET(E57RefImpl_INCLUDE_DIRS ${E57RefImpl_INCLUDE_DIR})
ENDIF()


4 changes: 0 additions & 4 deletions src/refimpl/E57SimpleImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@
# include <sys\stat.h>
# include <windows.h>
//#include <stdint.h> //if you need this then remove <boost/cstdint.hpp> in E57Foundation.h line 48
#pragma warning(disable:4996)
# include <boost/uuid/uuid.hpp>
# include <boost/uuid/uuid_generators.hpp>
# include <boost/uuid/uuid_io.hpp>
# elif defined(__GNUC__)
# define _LARGEFILE64_SOURCE
# define __LARGE64_FILES
Expand Down