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

Migrate to using TinyXML2 #264

Merged
merged 28 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3c70dc9
Remove vendored TinyXML
mjcarroll Apr 29, 2020
f2fe2eb
Add FindTinyXML2 CMake Logic
mjcarroll Apr 29, 2020
253ae22
Add comment about origin and license
mjcarroll Apr 29, 2020
b15f9a0
Add XmlUtils from patch
mjcarroll Apr 29, 2020
aca9716
First pass find and replace of TiXml types
mjcarroll Apr 29, 2020
4ad0d58
Find and replace tinyxml header
mjcarroll Apr 29, 2020
29210cf
Migrate Converter.cc to TinyXML2
mjcarroll Apr 30, 2020
6dd6d07
Migrate parser.cc to TinyXML2
mjcarroll Apr 30, 2020
62f2772
Fix Converter UNIT test for TinyXML2
mjcarroll May 27, 2020
3f87a24
Fix parser_urdf_TEST
mjcarroll May 27, 2020
345652a
Clean up code check
mjcarroll Jun 1, 2020
ac4bd1f
Update CI dependencies
mjcarroll Jun 1, 2020
6f33b6f
Allow trim to remove newlines as well
mjcarroll Jun 11, 2020
8ed29dc
Address reviewer feedback
mjcarroll Jun 11, 2020
aaf159f
Use std::string version of trim
mjcarroll Jun 11, 2020
3bd0e28
Remove trailing quote in example sdf
mjcarroll Jun 11, 2020
7e0a5ca
Revert uncommenting variable
mjcarroll Jun 22, 2020
ec2d820
Add test for XmlUtils
mjcarroll Jun 22, 2020
de73258
include tinyxml2.h where needed
scpeters Jun 25, 2020
111bcc0
Make internal URDF parser use tinyxml2
mjcarroll Jul 13, 2020
02463a6
Fix lint
mjcarroll Jul 13, 2020
b8af137
Don't add dllexport on Windows
mjcarroll Jul 13, 2020
f21871e
Address reviewer feedback about documentation
mjcarroll Jul 14, 2020
9c9b74a
Change ValueStr -> Name for consistency
mjcarroll Jul 14, 2020
47817f0
Remove unneeded return
mjcarroll Jul 14, 2020
afe61c8
Fix linting
mjcarroll Jul 14, 2020
4af3373
Style
azeey Jul 15, 2020
7155942
Migration guide and changelog
scpeters Jul 16, 2020
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 .github/workflows/linux-ubuntu-bionic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
sudo sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable $(lsb_release -cs) main" > /etc/apt/sources.list.d/gazebo-stable.list';
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys D2486D2DD83DB69272AFE98867170598AF249743;
sudo apt-get update;
sudo apt -y install cmake build-essential curl g++-8 git mercurial libtinyxml-dev libxml2-utils ruby-dev python-psutil cppcheck;
sudo apt -y install cmake build-essential curl g++-8 git mercurial libtinyxml2-dev libxml2-utils ruby-dev python-psutil cppcheck;
sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-8 800 --slave /usr/bin/g++ g++ /usr/bin/g++-8 --slave /usr/bin/gcov gcov /usr/bin/gcov-8;
# workaround for https://github.com/rubygems/rubygems/issues/3068
# suggested in https://github.com/rubygems/rubygems/issues/3068#issuecomment-574775885
Expand Down
18 changes: 1 addition & 17 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -109,24 +109,8 @@ set (build_warnings "" CACHE INTERNAL "build warnings" FORCE)
set (sdf_cmake_dir ${PROJECT_SOURCE_DIR}/cmake CACHE PATH
"Location of CMake scripts")

include (${sdf_cmake_dir}/SDFUtils.cmake)

if (UNIX)
option (USE_EXTERNAL_TINYXML "Use external TinyXML" ON)
elseif(WIN32)
option (USE_EXTERNAL_TINYXML "Use external TinyXML" OFF)
else()
message (STATUS "Unknown platform")
BUILD_ERROR("Unknown platform")
endif()

message (STATUS "\n\n====== Finding 3rd Party Packages ======")
# Use of tinyxml. System installation on UNIX. Internal copy on WIN
if (USE_EXTERNAL_TINYXML)
message (STATUS "Using system tinyxml")
else()
message (STATUS "Using internal tinyxml code")
endif()
include (${sdf_cmake_dir}/SDFUtils.cmake)
include (${sdf_cmake_dir}/SearchForStuff.cmake)
message (STATUS "----------------------------------------\n")

Expand Down
2 changes: 1 addition & 1 deletion bitbucket-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pipelines:
- sh -c 'echo "deb http://packages.osrfoundation.org/gazebo/ubuntu-stable `lsb_release -cs` main" > /etc/apt/sources.list.d/gazebo-stable.list'
- wget http://packages.osrfoundation.org/gazebo.key -O - | apt-key add -
- apt-get update
- apt -y install cmake build-essential curl git libtinyxml-dev libxml2-utils ruby-dev python-psutil cppcheck
- apt -y install cmake build-essential curl git libtinyxml2-dev libxml2-utils ruby-dev python-psutil cppcheck
- gcc -v
- g++ -v
- gcov -v
Expand Down
42 changes: 42 additions & 0 deletions cmake/Modules/FindTinyXML2.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# CMake Logic to find system TinyXML2, sourced from:
# ros2/tinyxml2_vendor
# https://github.com/ros2/tinyxml2_vendor/commit/fde8000d31d68ff555431d63af3c324afba9f117#diff-120198e331f1dd3e7806c31af0cfb425

# The CMake Logic here is licensed under Apache License 2.0
# TinyXML2 itself is licensed under the zlib License

# TinyXML2_FOUND
# TinyXML2_INCLUDE_DIRS
# TinyXML2_LIBRARIES

# try to find the CMake config file for TinyXML2 first
find_package(TinyXML2 CONFIG QUIET)
if(TinyXML2_FOUND)
message(STATUS "Found TinyXML2 via Config file: ${TinyXML2_DIR}")
if(NOT TINYXML2_LIBRARY)
# in this case, we're probably using TinyXML2 version 5.0.0 or greater
# in which case tinyxml2 is an exported target and we should use that
if(TARGET tinyxml2)
set(TINYXML2_LIBRARY tinyxml2)
elseif(TARGET tinyxml2::tinyxml2)
set(TINYXML2_LIBRARY tinyxml2::tinyxml2)
endif()
endif()
else()
find_path(TINYXML2_INCLUDE_DIR NAMES tinyxml2.h)

find_library(TINYXML2_LIBRARY tinyxml2)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(TinyXML2 DEFAULT_MSG TINYXML2_LIBRARY TINYXML2_INCLUDE_DIR)

mark_as_advanced(TINYXML2_INCLUDE_DIR TINYXML2_LIBRARY)
endif()

# Set mixed case INCLUDE_DIRS and LIBRARY variables from upper case ones.
if(NOT TinyXML2_INCLUDE_DIRS)
set(TinyXML2_INCLUDE_DIRS ${TINYXML2_INCLUDE_DIR})
endif()
if(NOT TinyXML2_LIBRARIES)
set(TinyXML2_LIBRARIES ${TINYXML2_LIBRARY})
endif()
15 changes: 1 addition & 14 deletions cmake/SDFUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -135,30 +135,17 @@ macro (sdf_build_tests)
string(REGEX REPLACE ".cc" "" BINARY_NAME ${GTEST_SOURCE_file})
set(BINARY_NAME ${TEST_TYPE}_${BINARY_NAME})

if (NOT USE_EXTERNAL_TINYXML)
set(tinyxml_SRC
${PROJECT_SOURCE_DIR}/src/win/tinyxml/tinystr.cpp
${PROJECT_SOURCE_DIR}/src/win/tinyxml/tinyxmlerror.cpp
${PROJECT_SOURCE_DIR}/src/win/tinyxml/tinyxml.cpp
${PROJECT_SOURCE_DIR}/src/win/tinyxml/tinyxmlparser.cpp)
endif()

add_executable(${BINARY_NAME}
${GTEST_SOURCE_file}
${SDF_BUILD_TESTS_EXTRA_EXE_SRCS}
${tinyxml_SRC}
)

add_dependencies(${BINARY_NAME}
gtest gtest_main ${sdf_target}
)

link_directories(${IGNITION-MATH_LIBRARY_DIRS})

if (USE_EXTERNAL_TINYXML)
target_link_libraries(${BINARY_NAME} PRIVATE
${tinyxml_LIBRARIES})
endif()
target_link_libraries(${BINARY_NAME} ${tinyxml2_LIBRARIES})

if (UNIX)
target_link_libraries(${BINARY_NAME} PRIVATE
Expand Down
34 changes: 4 additions & 30 deletions cmake/SearchForStuff.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,10 @@ include (${project_cmake_dir}/TargetArch.cmake)
target_architecture(ARCH)
message(STATUS "Building for arch: ${ARCH}")

if (USE_EXTERNAL_TINYXML)
#################################################
# Find tinyxml. Only debian distributions package tinyxml with a pkg-config
# Use pkg_check_modules and fallback to manual detection (needed, at least, for MacOS)
pkg_check_modules(tinyxml tinyxml)
if (NOT tinyxml_FOUND)
find_path (tinyxml_INCLUDE_DIRS tinyxml.h ${tinyxml_INCLUDE_DIRS} ENV CPATH)
find_library(tinyxml_LIBRARIES NAMES tinyxml)
set (tinyxml_FAIL False)
if (NOT tinyxml_INCLUDE_DIRS)
message (STATUS "Looking for tinyxml headers - not found")
set (tinyxml_FAIL True)
endif()
if (NOT tinyxml_LIBRARIES)
message (STATUS "Looking for tinyxml library - not found")
set (tinyxml_FAIL True)
endif()
endif()

if (tinyxml_FAIL)
message (STATUS "Looking for tinyxml.h - not found")
BUILD_ERROR("Missing: tinyxml")
endif()
else()
# Needed in WIN32 since in UNIX the flag is added in the code installed
add_definitions(-DTIXML_USE_STL)
include_directories (${PROJECT_SOURCE_DIR}/src/win/tinyxml)
set (tinyxml_LIBRARIES "tinyxml")
set (tinyxml_LIBRARY_DIRS "")
endif()
#################################################
# Find tinyxml2.
list(INSERT CMAKE_MODULE_PATH 0 "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules")
find_package(TinyXML2 REQUIRED)

################################################
# Find urdfdom parser. Logic:
Expand Down
2 changes: 1 addition & 1 deletion examples/simple.sdf
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
<model name='my_model'>
<link name='link'/>
</model>
</sdf>"
</sdf>
43 changes: 15 additions & 28 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ if (NOT USE_INTERNAL_URDF)
link_directories(${URDF_LIBRARY_DIRS})
endif()

if (USE_EXTERNAL_TINYXML)
link_directories(${tinyxml_LIBRARY_DIRS})
endif()

set (sources
Actor.cc
AirPressure.cc
Expand Down Expand Up @@ -62,22 +58,10 @@ set (sources
Utils.cc
Visual.cc
World.cc
XmlUtils.cc
)
include_directories(${CMAKE_CURRENT_SOURCE_DIR})

if (USE_EXTERNAL_TINYXML)
include_directories(${tinyxml_INCLUDE_DIRS})
else()
set(sources ${sources}
win/tinyxml/tinystr.cpp
win/tinyxml/tinyxmlerror.cpp
win/tinyxml/tinyxml.cpp
win/tinyxml/tinyxmlparser.cpp)

install (FILES win/tinyxml/tinyxml.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/sdformat-${SDF_VERSION})
endif()

if (USE_INTERNAL_URDF)
include_directories(${CMAKE_CURRENT_SOURCE_DIR}/urdf)
set(sources ${sources}
Expand Down Expand Up @@ -153,18 +137,23 @@ if (BUILD_SDF_TEST)
sdf_build_tests(Utils_TEST.cc)
endif()

if (NOT WIN32)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS XmlUtils.cc)
sdf_build_tests(XmlUtils_TEST.cc)
endif()

if (NOT WIN32)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS FrameSemantics.cc)
sdf_build_tests(FrameSemantics_TEST.cc)
endif()

if (NOT WIN32)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS Converter.cc EmbeddedSdf.cc)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS Converter.cc EmbeddedSdf.cc XmlUtils.cc)
sdf_build_tests(Converter_TEST.cc)
endif()

if (NOT WIN32)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS SDFExtension.cc parser_urdf.cc)
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS SDFExtension.cc parser_urdf.cc XmlUtils.cc)
sdf_build_tests(parser_urdf_TEST.cc)
if (NOT USE_INTERNAL_URDF)
target_compile_options(UNIT_parser_urdf_TEST PRIVATE ${URDF_CFLAGS})
Expand All @@ -178,7 +167,13 @@ endif()

sdf_add_library(${sdf_target} ${sources})
target_compile_features(${sdf_target} PUBLIC cxx_std_17)
target_link_libraries(${sdf_target} PUBLIC ${IGNITION-MATH_LIBRARIES})
target_link_libraries(${sdf_target} PUBLIC
${IGNITION-MATH_LIBRARIES}
${TinyXML2_LIBRARIES})

if (WIN32)
target_compile_definitions(${sdf_target} PRIVATE URDFDOM_STATIC)
endif()

target_include_directories(${sdf_target}
PUBLIC
Expand All @@ -187,14 +182,6 @@ target_include_directories(${sdf_target}
$<INSTALL_INTERFACE:${INCLUDE_INSTALL_DIR}/..>
)

if (USE_EXTERNAL_TINYXML)
target_link_libraries(${sdf_target} PRIVATE ${tinyxml_LIBRARIES})
else()
# Ignore the warnings from the internal library
set_target_properties(sdformat${SDF_MAJOR_VERSION} PROPERTIES
LINK_FLAGS "/IGNORE:4049 /IGNORE:4217")
endif()

message (STATUS "URDF_LIBRARY_DIRS=${URDF_LIBRARY_DIRS}")
message (STATUS "URDF_LIBRARIES=${URDF_LIBRARIES}")

Expand Down
Loading