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

Add optional binary relocatability #507

Merged
merged 10 commits into from
Aug 29, 2023
5 changes: 4 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,11 @@ message(STATUS "-------------------------------------------\n")
set(GZ_PHYSICS_RESOURCE_DIR "${CMAKE_SOURCE_DIR}/resources")

# Plugin install dirs
set(GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR
${GZ_LIB_INSTALL_DIR}/gz-${GZ_DESIGNATION}-${PROJECT_VERSION_MAJOR}/engine-plugins
)
set(GZ_PHYSICS_ENGINE_INSTALL_DIR
${CMAKE_INSTALL_PREFIX}/${GZ_LIB_INSTALL_DIR}/gz-${GZ_DESIGNATION}-${PROJECT_VERSION_MAJOR}/engine-plugins
${CMAKE_INSTALL_PREFIX}/${GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR}
)

#============================================================================
Expand Down
4 changes: 3 additions & 1 deletion dartsim/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ gz_build_tests(
${PROJECT_LIBRARY_TARGET_NAME}-sdf
${PROJECT_LIBRARY_TARGET_NAME}-heightmap
${PROJECT_LIBRARY_TARGET_NAME}-mesh
TEST_LIST tests)
TEST_LIST tests
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})

foreach(test ${tests})

Expand Down
43 changes: 43 additions & 0 deletions include/gz/physics/InstallationDirectories.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (C) 2023 Open Source Robotics Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

#ifndef GZ_PHYSICS_INSTALLATION_DIRECTORIES_HH_
#define GZ_PHYSICS_INSTALLATION_DIRECTORIES_HH_

#include <string>

#include <gz/physics/config.hh>
#include <gz/physics/Export.hh>

namespace gz
{
namespace physics
{
inline namespace GZ_PHYSICS_VERSION_NAMESPACE {

/// \brief getInstallPrefix return the install prefix of the library
/// i.e. CMAKE_INSTALL_PREFIX unless the library has been moved
GZ_PHYSICS_VISIBLE std::string getInstallPrefix();

/// \brief getEngineInstallDir return the install directory of the engines
GZ_PHYSICS_VISIBLE std::string getEngineInstallDir();

}
}
}

#endif
2 changes: 1 addition & 1 deletion include/gz/physics/config.hh.in
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#define GZ_PHYSICS_VERSION "${PROJECT_VERSION}"
#define GZ_PHYSICS_VERSION_FULL "${PROJECT_VERSION_FULL}"

#define GZ_PHYSICS_ENGINE_INSTALL_DIR "${GZ_PHYSICS_ENGINE_INSTALL_DIR}"
#define GZ_PHYSICS_ENGINE_INSTALL_DIR _Pragma ("GCC warning \"'GZ_PHYSICS_ENGINE_INSTALL_DIR' macro is deprecated, use gz::physics::getEngineInstallDir() function instead. \"") "${GZ_PHYSICS_ENGINE_INSTALL_DIR}"

#define GZ_PHYSICS_VERSION_HEADER "Gazebo Physics, version ${PROJECT_VERSION_FULL}\nCopyright (C) 2017 Open Source Robotics Foundation.\nReleased under the Apache 2.0 License.\n\n"

Expand Down
12 changes: 11 additions & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
gz_get_libsources_and_unittests(sources gtest_sources)
gz_create_core_library(SOURCES ${sources} CXX_STANDARD 17)
gz_add_get_install_prefix_impl(GET_INSTALL_PREFIX_FUNCTION gz::physics::getInstallPrefix
GET_INSTALL_PREFIX_HEADER gz/physics/InstallationDirectories.hh
OVERRIDE_INSTALL_PREFIX_ENV_VARIABLE GZ_PHYSICS_INSTALL_PREFIX)

set_property(
SOURCE InstallationDirectories.cc
PROPERTY COMPILE_DEFINITIONS
GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR="${GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR}"
)

target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME}
PUBLIC
Expand All @@ -11,7 +19,9 @@ target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME}

gz_build_tests(
TYPE UNIT
SOURCES ${gtest_sources})
SOURCES ${gtest_sources}
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})

if(TARGET UNIT_FindFeatures_TEST)
target_link_libraries(UNIT_FindFeatures_TEST
Expand Down
151 changes: 151 additions & 0 deletions src/InstallationDirectories.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
* Copyright (C) 2023 Open Source Robotics Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

#include <regex>

#include <gz/physics/config.hh>
#include <gz/physics/InstallationDirectories.hh>

namespace gz
{
namespace physics
{
inline namespace GZ_PHYSICS_VERSION_NAMESPACE {

// We locally import the gz::common::joinPaths function
// See https://github.com/gazebosim/gz-physics/pull/507#discussion_r1186919267
// for more details

// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/FilesystemBoost.cc#L507
#ifndef WIN32
static const char preferred_separator = '/';
#else // Windows
static const char preferred_separator = '\\';
#endif
const std::string separator(const std::string &_p)
{
return _p + preferred_separator;
}

// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/Filesystem.cc#L227
std::string checkWindowsPath(const std::string _path)
{
if (_path.empty())
return _path;

// Check if this is a http or https, if so change backslashes generated by
// jointPaths to '/'
if ((_path.size() > 7 && 0 == _path.compare(0, 7, "http://")) ||
(_path.size() > 8 && 0 == _path.compare(0, 8, "https://")))
{
return std::regex_replace(_path, std::regex(R"(\\)"), "/");
}

// This is a Windows path, convert all '/' into backslashes
std::string result = std::regex_replace(_path, std::regex(R"(/)"), "\\");
std::string drive_letters;

// only Windows contains absolute paths starting with drive letters
if (result.length() > 3 && 0 == result.compare(1, 2, ":\\"))
{
drive_letters = result.substr(0, 3);
result = result.substr(3);
}
result = drive_letters + std::regex_replace(
result, std::regex("[<>:\"|?*]"), "");
return result;
}

// Function imported from
// https://github.com/gazebosim/gz-common/blob/ignition-common4_4.6.2/src/Filesystem.cc#L256
std::string joinPaths(const std::string &_path1,
const std::string &_path2)
{

/// This function is used to avoid duplicated path separators at the
/// beginning/end of the string, and between the two paths being joined.
/// \param[in] _path This is the string to sanitize.
/// \param[in] _stripLeading True if the leading separator should be
/// removed.
auto sanitizeSlashes = [](const std::string &_path,
bool _stripLeading = false)
{
// Shortcut
if (_path.empty())
return _path;

std::string result = _path;

// Use the appropriate character for each platform.
#ifndef _WIN32
char replacement = '/';
#else
char replacement = '\\';
#endif

// Sanitize the start of the path.
size_t index = 0;
size_t leadingIndex = _stripLeading ? 0 : 1;
for (; index < result.length() && result[index] == replacement; ++index)
{
}
if (index > leadingIndex)
result.erase(leadingIndex, index-leadingIndex);

// Sanitize the end of the path.
index = result.length()-1;
for (; index < result.length() && result[index] == replacement; --index)
{
}
index += 1;
if (index < result.length()-1)
result.erase(index+1);
return result;
};

std::string path;
#ifndef _WIN32
path = sanitizeSlashes(sanitizeSlashes(separator(_path1)) +
sanitizeSlashes(_path2, true));
#else // _WIN32
std::string path1 = sanitizeSlashes(checkWindowsPath(_path1));
std::string path2 = sanitizeSlashes(checkWindowsPath(_path2), true);
std::vector<char> combined(path1.length() + path2.length() + 2);
if (::PathCombineA(combined.data(), path1.c_str(), path2.c_str()) != NULL)
Copy link
Member

Choose a reason for hiding this comment

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

in https://build.osrfoundation.org/job/ign_physics-pr-win/2336/console

src\InstallationDirectories.cc(130,9): error C2039: 'PathCombineA': is not a member of '`global namespace'' 

Copy link
Member

@scpeters scpeters Aug 28, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just getting back to these, I believe that is the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, this is what I did in conda-forge: conda-forge/gz-physics-feedstock@d548779 . Probably I forgot to report it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2e1a99f .

Copy link
Contributor

Choose a reason for hiding this comment

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

Something may need to be linked as well?

InstallationDirectories.obj : error LNK2001: unresolved external symbol __imp_PathCombineA [C:\J\workspace\ign_physics-pr-win\ws\build\gz-physics6\src\gz-physics6.vcxproj]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously yes, see last commit of https://github.com/conda-forge/gz-physics-feedstock/pull/11/commits , my bad. -_-

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hopefully fixed by 62f2a32 .

{
path = sanitizeSlashes(checkWindowsPath(std::string(combined.data())));
}
else
{
path = sanitizeSlashes(checkWindowsPath(separator(path1) + path2));
}
#endif // _WIN32
return path;
}


std::string getEngineInstallDir()
{
return gz::physics::joinPaths(
getInstallPrefix(), GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR);
}

}
}
}
4 changes: 3 additions & 1 deletion test/integration/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ gz_build_tests(
gz-plugin${GZ_PLUGIN_VER}::loader
INCLUDE_DIRS
${PROJECT_SOURCE_DIR}/src
TEST_LIST list)
TEST_LIST list
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})

if (BUILD_TESTING)
foreach(test ${list})
Expand Down
2 changes: 2 additions & 0 deletions test/performance/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ gz_build_tests(
SOURCES ${tests}
INCLUDE_DIRS
${PROJECT_SOURCE_DIR}/src
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
)
4 changes: 3 additions & 1 deletion test/regression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ gz_get_sources(tests)

gz_build_tests(
TYPE REGRESSION
SOURCES ${tests})
SOURCES ${tests}
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})
4 changes: 3 additions & 1 deletion tpe/lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,6 @@ gz_build_tests(
${PROJECT_LIBRARY_TARGET_NAME}-sdf
${PROJECT_LIBRARY_TARGET_NAME}-mesh
${tpelib_target}
TEST_LIST tests)
TEST_LIST tests
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})
4 changes: 3 additions & 1 deletion tpe/plugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ gz_build_tests(
gz-common${GZ_COMMON_VER}::gz-common${GZ_COMMON_VER}
${PROJECT_LIBRARY_TARGET_NAME}-sdf
${PROJECT_LIBRARY_TARGET_NAME}-mesh
TEST_LIST tests)
TEST_LIST tests
ENVIRONMENT
GZ_PHYSICS_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX})

foreach(test ${tests})

Expand Down