From d06ed33fc9abc4f149234acd246ea566d119a6b1 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 13 Sep 2021 17:40:13 -0500 Subject: [PATCH 01/10] Add functions and objects for Temporary Directories (#244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michael Carroll Co-authored-by: Alejandro Hernández Cordero --- include/ignition/common/TempDirectory.hh | 110 +++++++++++ src/CMakeLists.txt | 3 + src/Console.cc | 2 +- src/Console_TEST.cc | 19 +- src/SystemPaths_TEST.cc | 16 ++ src/TempDirectory.cc | 229 +++++++++++++++++++++++ src/TempDirectory_TEST.cc | 113 +++++++++++ test/test_config.h.in | 49 ++--- 8 files changed, 495 insertions(+), 46 deletions(-) create mode 100644 include/ignition/common/TempDirectory.hh create mode 100644 src/TempDirectory.cc create mode 100644 src/TempDirectory_TEST.cc diff --git a/include/ignition/common/TempDirectory.hh b/include/ignition/common/TempDirectory.hh new file mode 100644 index 000000000..b66c691e5 --- /dev/null +++ b/include/ignition/common/TempDirectory.hh @@ -0,0 +1,110 @@ +/* + * Copyright 2021 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 IGNITION_COMMON_TEMPDIRECTORY_HH_ +#define IGNITION_COMMON_TEMPDIRECTORY_HH_ + +#include +#include + +#include +#include +#include + +namespace ignition +{ + namespace common + { + class TempDirectoryPrivate; + + /// \brief Return the path to a directory suitable for temporary files. + /// + /// Calls std::filesystem::temp_directory_path, refer to the standard + /// documentation for your platform for behaviors. + /// \return A directory suitable for temporary files. + std::string IGNITION_COMMON_VISIBLE tempDirectoryPath(); + + /// \brief Create a directory in the tempDirectoryPath by expanding + /// a name template + /// + /// On execution, will create the directory: + /// "_parentPath"/"_baseName" + "XXXXXX", where XXXXXX will be filled + /// out by an OS-appropriate method (eg mkdtmp/_mktemp_s) + /// + /// \param[in] _baseName String to be prepended to the expanded template + /// \param[in] _parentPath Location to create the directory + /// \param[in] _warningOp Allow or suppress filesystem warnings + /// \return Path to newly-created temporary directory + std::string IGNITION_COMMON_VISIBLE createTempDirectory( + const std::string &_baseName, + const std::string &_parentPath, + const FilesystemWarningOp _warningOp = FSWO_LOG_WARNINGS); + + /// \class TempDirectory TempDirectory.hh ignitin/common/TempDirectory.hh + /// \brief Create a temporary directory in the OS temp location. + /// Upon construction, the current working directory will be set to this + /// new temporary directory. + /// Upon destruction, the current working directory will be restored to the + /// location when the TempDirectory object was constructed. + class IGNITION_COMMON_VISIBLE TempDirectory + { + /// \brief Create a directory in the tempDirectoryPath by expanding + /// a name template. This directory can also be automatically cleaned + /// up when the object goes out of scope. + /// + /// The TempDirectory will have the form $TMPDIR/_subdir/_prefixXXXXX/ + /// + /// \param[in] _prefix String to be expanded for the template + /// \param[in] _subDir Subdirectory in OS $TMPDIR, if desired + /// \param[in] _cleanup True to indicate that the filesystem should + /// be cleaned as part of the destructor + public: TempDirectory(const std::string &_prefix = "temp_dir", + const std::string &_subDir = "ignition", + bool _cleanup = true); + + /// \brief Destroy the temporary directory, removing from filesystem + /// if cleanup is true. + public: ~TempDirectory(); + + /// \brief Indicate if the TempDirectory object is in a valid state + /// and that the folder exists on the filesystem + /// \return true if the TempDirectory is valid + public: bool Valid() const; + + /// \brief Set if the folder on disk should be cleaned. + /// + /// This is useful if you wish to clean by default during a test, but + /// retain the contents of the TempDirectory if the test fails. + /// \param[in] _doCleanup True to indicate that the filesystem should + /// be cleaned as part of the destructor + public: void DoCleanup(bool _doCleanup); + + /// \brief Retrieve the current cleanup flag state + /// \return true if filesystem cleanup will occur + public: bool DoCleanup() const; + + /// \brief Retrieve the fully-expanded temporary directory path + /// \return the temporary directory path + public: std::string Path() const; + + /// \brief Private data pointer. + IGN_UTILS_IMPL_PTR(dataPtr) + }; + } // namespace common +} // namespace ignition +#endif // IGNITION_COMMON_TEMPDIRECTORY_HH_ + diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 90e333f1e..7b2974194 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -17,6 +17,9 @@ target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME} target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE ${ignition-math${IGN_MATH_VER}_INCLUDE_DIRS}) +target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PUBLIC + ${ignition-utils${IGN_UTILS_VER}_INCLUDE_DIRS}) + # Handle non-Windows configuration settings if(NOT WIN32) # Link the libraries that we don't expect to find on Windows diff --git a/src/Console.cc b/src/Console.cc index 0e8b1fbbb..ff013fe23 100644 --- a/src/Console.cc +++ b/src/Console.cc @@ -252,7 +252,7 @@ void FileLogger::Init(const std::string &_directory, if (isDirectory(logPath)) this->logDirectory = logPath; else - this->logDirectory = logPath.substr(0, logPath.rfind(separator(""))); + this->logDirectory = common::parentPath(logPath); this->initialized = true; diff --git a/src/Console_TEST.cc b/src/Console_TEST.cc index 2cc3805fd..28b05668c 100644 --- a/src/Console_TEST.cc +++ b/src/Console_TEST.cc @@ -33,23 +33,21 @@ const int g_messageRepeat = 4; class Console_TEST : public ::testing::Test { protected: virtual void SetUp() { - // Set IGN_HOMEDIR and store it - common::testing::TestSetHomePath(this->logBasePath); + this->temp = std::make_unique( + "test", "ign_common", true); + ASSERT_TRUE(this->temp->Valid()); + common::setenv(IGN_HOMEDIR, this->temp->Path()); } /// \brief Clear out all the directories we produced during this test. - public: virtual ~Console_TEST() + public: virtual void TearDown() { + ignLogClose(); EXPECT_TRUE(ignition::common::unsetenv(IGN_HOMEDIR)); - - if (ignition::common::isDirectory(this->logBasePath)) - { - ignLogClose(); - EXPECT_TRUE(ignition::common::removeAll(this->logBasePath)); - } } - private: std::string logBasePath; + /// \brief Temporary directory to run test in + private: std::unique_ptr temp; }; std::string GetLogContent(const std::string &_filename) @@ -58,6 +56,7 @@ std::string GetLogContent(const std::string &_filename) std::string path; EXPECT_TRUE(ignition::common::env(IGN_HOMEDIR, path)); path = ignition::common::joinPaths(path, _filename); + EXPECT_TRUE(ignition::common::exists(path)); // Open the log file, and read back the string std::ifstream ifs(path.c_str(), std::ios::in); diff --git a/src/SystemPaths_TEST.cc b/src/SystemPaths_TEST.cc index f20878fb0..a17950baa 100644 --- a/src/SystemPaths_TEST.cc +++ b/src/SystemPaths_TEST.cc @@ -26,6 +26,7 @@ #include "ignition/common/Util.hh" #include "ignition/common/StringUtils.hh" #include "ignition/common/SystemPaths.hh" +#include "ignition/common/TempDirectory.hh" #ifdef _WIN32 #define snprintf _snprintf @@ -36,11 +37,22 @@ using namespace ignition; const char kPluginPath[] = "IGN_PLUGIN_PATH"; const char kFilePath[] = "IGN_FILE_PATH"; +class TestTempDirectory : public ignition::common::TempDirectory +{ + public: TestTempDirectory(): + ignition::common::TempDirectory("systempaths", "ign_common", true) + { + } +}; + class SystemPathsFixture : public ::testing::Test { // Documentation inherited public: virtual void SetUp() { + this->temp = std::make_unique(); + ASSERT_TRUE(this->temp->Valid()); + common::env(kPluginPath, this->backupPluginPath); common::unsetenv(kPluginPath); @@ -59,6 +71,7 @@ class SystemPathsFixture : public ::testing::Test { common::setenv(kPluginPath, this->backupPluginPath); common::setenv(kFilePath, this->backupFilePath); + this->temp.reset(); } /// \brief Backup of plugin paths to be restored after the test @@ -69,6 +82,9 @@ class SystemPathsFixture : public ::testing::Test /// \brief Root of filesystem according to each platform public: std::string filesystemRoot; + + /// \brief Temporary directory to execute test in + public: std::unique_ptr temp; }; std::string SystemPathsJoin(const std::vector &_paths) diff --git a/src/TempDirectory.cc b/src/TempDirectory.cc new file mode 100644 index 000000000..8e44b38fc --- /dev/null +++ b/src/TempDirectory.cc @@ -0,0 +1,229 @@ +/* + * Copyright 2021 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 + +#include + +#ifdef _WIN32 +#include +#include +#include +#include +#endif + +using namespace ignition; +using namespace common; + +///////////////////////////////////////////////// +// Return true if success, false if error +inline bool fs_warn(const std::string &_fcn, + const std::error_code &_ec, + const FilesystemWarningOp &_warningOp = FSWO_LOG_WARNINGS) +{ + if (_ec) + { + if (FSWO_LOG_WARNINGS == _warningOp) + { + ignwarn << "Failed ignition::common::" << _fcn + << " (ec: " << _ec << " " << _ec.message() << ")\n"; + } + return false; + } + return true; +} + +///////////////////////////////////////////////// +// Helper implementation of std::filesystem::temp_directory_path +// https://en.cppreference.com/w/cpp/filesystem/temp_directory_path +// \TODO(anyone) remove when using `std::filesystem` in C++17 and greater. +std::string temp_directory_path(std::error_code& _err) +{ + _err = std::error_code(); +#ifdef _WIN32 + TCHAR temp_path[MAX_PATH]; + DWORD size = GetTempPathA(MAX_PATH, temp_path); + if (size > MAX_PATH || size == 0) { + _err = std::error_code( + static_cast(GetLastError()), std::system_category()); + } + temp_path[size] = '\0'; +#else + std::string temp_path; + if(!ignition::common::env("TMPDIR", temp_path)) + { + temp_path = "/tmp"; + } +#endif + return std::string(temp_path); +} + +///////////////////////////////////////////////// +std::string ignition::common::tempDirectoryPath() +{ + std::error_code ec; + auto ret = temp_directory_path(ec); + + if (!fs_warn("tempDirectoryPath", ec)) + { + ret = ""; + } + + return ret; +} + +///////////////////////////////////////////////// +/// \brief Internal method for createTempDirectory +/// +/// This is primarily to scope the "throw" behavior from when this +/// was copied from rclcpp. +std::string createTempDirectory( + const std::string &_baseName, + const std::string &_parentPath) +{ + std::string parentPath(_parentPath); + std::string templatePath = _baseName + "XXXXXX"; + + std::string fullTemplateStr = joinPaths(parentPath, templatePath); + if (!createDirectories(parentPath)) + { + std::error_code ec{errno, std::system_category()}; + errno = 0; + throw std::system_error(ec, "could not create the parent directory"); + } + +#ifdef _WIN32 + errno_t errcode = _mktemp_s(&fullTemplateStr[0], fullTemplateStr.size() + 1); + if (errcode) { + std::error_code ec(static_cast(errcode), std::system_category()); + throw std::system_error(ec, + "could not format the temp directory name template"); + } + const std::string finalPath{fullTemplateStr}; + if (!createDirectories(finalPath)) { + std::error_code ec(static_cast(GetLastError()), + std::system_category()); + throw std::system_error(ec, "could not create the temp directory"); + } +#else + const char * dirName = mkdtemp(&fullTemplateStr[0]); + if (dirName == nullptr) { + std::error_code ec{errno, std::system_category()}; + errno = 0; + throw std::system_error(ec, + "could not format or create the temp directory"); + } + const std::string finalPath{dirName}; +#endif + + return finalPath; +} + +///////////////////////////////////////////////// +std::string ignition::common::createTempDirectory( + const std::string &_baseName, + const std::string &_parentPath, + const FilesystemWarningOp _warningOp) +{ + std::string ret; + try { + ret = ::createTempDirectory(_baseName, _parentPath); + } + catch (const std::system_error &ex) + { + ret = ""; + if(FSWO_LOG_WARNINGS == _warningOp) + { + ignwarn << "Failed to create temp directory: " << ex.what() << "\n"; + } + } + return ret; +} + + +class ignition::common::TempDirectory::Implementation +{ + /// \brief Current working directory before creation of temporary dir. + public: std::string oldPath {""}; + + /// \brief Path of the temporary directory + public: std::string path {""}; + + /// \brief True if the temporary directory exists + public: bool isValid {false}; + + /// \brief True if the temporary directory should be cleaned up from + /// disk when the object goes out of scope. + public: bool doCleanup {true}; +}; + +///////////////////////////////////////////////// +TempDirectory::TempDirectory(const std::string &_prefix, + const std::string &_subDir, + bool _cleanup): + dataPtr(ignition::utils::MakeImpl()) +{ + + this->dataPtr->oldPath = common::cwd(); + this->dataPtr->doCleanup = _cleanup; + + auto tempPath = common::tempDirectoryPath(); + if (!_subDir.empty()) + { + tempPath = common::joinPaths(tempPath, _subDir); + } + this->dataPtr->path = common::createTempDirectory(_prefix, tempPath); + if (!this->dataPtr->path.empty()) + { + this->dataPtr->isValid = true; + common::chdir(this->dataPtr->path); + } +} + +///////////////////////////////////////////////// +TempDirectory::~TempDirectory() +{ + common::chdir(this->dataPtr->oldPath); + if (this->dataPtr->isValid && this->dataPtr->doCleanup) + { + common::removeAll(this->dataPtr->path); + } +} + +///////////////////////////////////////////////// +bool TempDirectory::Valid() const +{ + return this->dataPtr->isValid; +} + +///////////////////////////////////////////////// +void TempDirectory::DoCleanup(bool _doCleanup) +{ + this->dataPtr->doCleanup = _doCleanup; +} + +///////////////////////////////////////////////// +bool TempDirectory::DoCleanup() const +{ + return this->dataPtr->doCleanup; +} + +///////////////////////////////////////////////// +std::string TempDirectory::Path() const +{ + return this->dataPtr->path; +} diff --git a/src/TempDirectory_TEST.cc b/src/TempDirectory_TEST.cc new file mode 100644 index 000000000..a0c3ad7bb --- /dev/null +++ b/src/TempDirectory_TEST.cc @@ -0,0 +1,113 @@ +/* + * Copyright (C) 2021 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 + +#include +#include + +///////////////////////////////////////////////// +TEST(TempDirectory, tempDirectoryPath) +{ + // OS TMPDIR should never be empty + ASSERT_FALSE(ignition::common::tempDirectoryPath().empty()); +} + +///////////////////////////////////////////////// +TEST(TempDirectory, createTempDirectory) +{ + // OS TMPDIR should never be empty + auto tmpDir = ignition::common::tempDirectoryPath(); + ASSERT_FALSE(tmpDir.empty()); + + // Nominal case + // eg /tmp/fooXXXXXX + auto tmp = ignition::common::createTempDirectory("foo", tmpDir); + ASSERT_FALSE(tmp.empty()); + EXPECT_NE(std::string::npos, tmp.find("foo")); + EXPECT_NE(std::string::npos, tmp.find(tmpDir)); + + // Should also work for subdirectories + // eg /tmp/bar/fooXXXXXX + tmpDir = ignition::common::joinPaths(tmpDir, "bar"); + auto tmp2 = ignition::common::createTempDirectory("bar", tmpDir); + ASSERT_FALSE(tmp2.empty()); +} + +///////////////////////////////////////////////// +TEST(TempDirectory, createTempDirectoryEmptyBase) +{ + // OS TMPDIR should never be empty + auto tmpDir = ignition::common::tempDirectoryPath(); + ASSERT_FALSE(tmpDir.empty()); + + // Create with an empty basename (eg /tmp/XXXXXX) + auto tmp = ignition::common::createTempDirectory("", tmpDir); + ASSERT_FALSE(tmp.empty()); + EXPECT_EQ(std::string::npos, tmp.find("foo")); + EXPECT_NE(std::string::npos, tmp.find(tmpDir)); +} + +///////////////////////////////////////////////// +TEST(TempDirectory, TempDirectory) +{ + std::string path; + { + ignition::common::TempDirectory tmp("temp_dir", "ignition", true); + EXPECT_TRUE(tmp.Valid()); + EXPECT_TRUE(tmp.DoCleanup()); + path = tmp.Path(); + EXPECT_FALSE(path.empty()); + EXPECT_TRUE(ignition::common::exists(path)); + } + EXPECT_FALSE(ignition::common::exists(path)); +} + +///////////////////////////////////////////////// +TEST(TempDirectory, TempDirectoryNoClean) +{ + std::string path; + { + ignition::common::TempDirectory tmp("temp_dir", "ignition", false); + EXPECT_TRUE(tmp.Valid()); + EXPECT_FALSE(tmp.DoCleanup()); + path = tmp.Path(); + EXPECT_FALSE(path.empty()); + EXPECT_TRUE(ignition::common::exists(path)); + } + EXPECT_TRUE(ignition::common::exists(path)); + EXPECT_TRUE(ignition::common::removeDirectory(path)); +} + +///////////////////////////////////////////////// +TEST(TempDirectory, TempDirectoryNoCleanLater) +{ + std::string path; + { + ignition::common::TempDirectory tmp("temp_dir", "ignition", true); + EXPECT_TRUE(tmp.Valid()); + EXPECT_TRUE(tmp.DoCleanup()); + path = tmp.Path(); + EXPECT_FALSE(path.empty()); + EXPECT_TRUE(ignition::common::exists(path)); + tmp.DoCleanup(false); + EXPECT_FALSE(tmp.DoCleanup()); + } + EXPECT_TRUE(ignition::common::exists(path)); + EXPECT_TRUE(ignition::common::removeDirectory(path)); +} + diff --git a/test/test_config.h.in b/test/test_config.h.in index 0a4616552..aea2bf931 100644 --- a/test/test_config.h.in +++ b/test/test_config.h.in @@ -13,6 +13,7 @@ #include #include "ignition/common/Console.hh" #include "ignition/common/Filesystem.hh" +#include "ignition/common/TempDirectory.hh" #include "ignition/common/Util.hh" #define PROJECT_BINARY_PATH "${PROJECT_BINARY_DIR}" @@ -59,37 +60,9 @@ namespace ignition } else { - _tmpDir = common::joinPaths("${PROJECT_BINARY_DIR}", "tmp"); - return true; - } - } - - /// \brief Method to retrieve temporary home directory for tests - /// - /// This will update the contents of the home directory path variable - /// (HOME on Linux/MacOS, HOMEPATH on Windows) to this newly-set - /// directory - /// This additionally sets the HOME and HOMEPATH environment variables - /// - /// \param[inout] _homeDir Full path to the home directory - /// \return True if directory is set correctly, false otherwise - bool TestSetHomePath(std::string &_homeDir) - { - if (common::env("TEST_UNDECLARED_OUTPUTS_DIR", _homeDir)) - { - return ignition::common::setenv(IGN_HOMEDIR, _homeDir); - } - else - { - if (TestTmpPath(_homeDir)) - { - // Set both for linux and windows - return ignition::common::setenv(IGN_HOMEDIR, _homeDir); - } - else - { - return false; - } + _tmpDir = common::createTempDirectory("ignition", + common::tempDirectoryPath()); + return !_tmpDir.empty(); } } @@ -143,11 +116,14 @@ namespace ignition std::string testCaseName = testInfo->test_case_name(); this->logFilename = testCaseName + "_" + testName + ".log"; - common::testing::TestSetHomePath(this->logBasePath); + this->temp = std::make_unique( + "test", "ign_common", true); + ASSERT_TRUE(this->temp->Valid()); + common::setenv(IGN_HOMEDIR, this->temp->Path()); // Initialize Console - ignLogInit(common::joinPaths(this->logBasePath, "test_logs"), - this->logFilename); + ignLogInit(common::joinPaths(this->temp->Path(), "test_logs"), + this->logFilename); ignition::common::Console::SetVerbosity(4); @@ -184,7 +160,7 @@ namespace ignition public: virtual ~AutoLogFixture() { ignLogClose(); - ignition::common::removeAll(this->logBasePath); + EXPECT_TRUE(ignition::common::unsetenv(IGN_HOMEDIR)); } /// \brief String with the full path of the logfile @@ -195,6 +171,9 @@ namespace ignition /// \brief String with the base path to log directory private: std::string logBasePath; + + /// \brief Temporary directory to run test in + private: std::unique_ptr temp; }; } // namespace testing } // namespace common From bc7802ddf2f52bd6230e6e2906032b716279d652 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 15 Sep 2021 15:26:14 -0500 Subject: [PATCH 02/10] Remove ign-utils from TempDirectory (#248) Signed-off-by: Michael Carroll --- include/ignition/common/TempDirectory.hh | 7 ++++--- src/CMakeLists.txt | 3 --- src/TempDirectory.cc | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/include/ignition/common/TempDirectory.hh b/include/ignition/common/TempDirectory.hh index b66c691e5..4216fd712 100644 --- a/include/ignition/common/TempDirectory.hh +++ b/include/ignition/common/TempDirectory.hh @@ -23,7 +23,6 @@ #include #include -#include namespace ignition { @@ -101,8 +100,10 @@ namespace ignition /// \return the temporary directory path public: std::string Path() const; - /// \brief Private data pointer. - IGN_UTILS_IMPL_PTR(dataPtr) + IGN_COMMON_WARN_IGNORE__DLL_INTERFACE_MISSING + /// \brief Private data pointer + private: std::unique_ptr dataPtr; + IGN_COMMON_WARN_RESUME__DLL_INTERFACE_MISSING }; } // namespace common } // namespace ignition diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7b2974194..90e333f1e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -17,9 +17,6 @@ target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME} target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE ${ignition-math${IGN_MATH_VER}_INCLUDE_DIRS}) -target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PUBLIC - ${ignition-utils${IGN_UTILS_VER}_INCLUDE_DIRS}) - # Handle non-Windows configuration settings if(NOT WIN32) # Link the libraries that we don't expect to find on Windows diff --git a/src/TempDirectory.cc b/src/TempDirectory.cc index 8e44b38fc..aa1bb914a 100644 --- a/src/TempDirectory.cc +++ b/src/TempDirectory.cc @@ -155,7 +155,7 @@ std::string ignition::common::createTempDirectory( } -class ignition::common::TempDirectory::Implementation +class ignition::common::TempDirectoryPrivate { /// \brief Current working directory before creation of temporary dir. public: std::string oldPath {""}; @@ -175,7 +175,7 @@ class ignition::common::TempDirectory::Implementation TempDirectory::TempDirectory(const std::string &_prefix, const std::string &_subDir, bool _cleanup): - dataPtr(ignition::utils::MakeImpl()) + dataPtr(std::make_unique()) { this->dataPtr->oldPath = common::cwd(); From 9b0a8547681e3e69fb7f86917a4420517cb32e78 Mon Sep 17 00:00:00 2001 From: Louise Poubel Date: Wed, 22 Sep 2021 02:38:11 -0700 Subject: [PATCH 03/10] =?UTF-8?q?=F0=9F=8E=88=204.3.0~pre1=20(#251)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Louise Poubel --- CMakeLists.txt | 4 ++-- Changelog.md | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 80e403ea6..c5745afb8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.10.2 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-common4 VERSION 4.2.0) +project(ignition-common4 VERSION 4.3.0) #============================================================================ # Find ignition-cmake @@ -14,7 +14,7 @@ set(IGN_CMAKE_VER ${ignition-cmake2_VERSION_MAJOR}) #============================================================================ # Configure the project #============================================================================ -ign_configure_project(VERSION_SUFFIX) +ign_configure_project(VERSION_SUFFIX pre1) #============================================================================ # Set project-specific options diff --git a/Changelog.md b/Changelog.md index 4482e4a42..651db1732 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,26 @@ ## Ignition Common 4.x -## Ignition Common 4.X.X (202X-XX-XX) +## Ignition Common 4.3.0 (2021-09-XX) + +1. Remove ign-utils from TempDirectory + * [Pull request #248](https://github.com/ignitionrobotics/ign-common/pull/248) + +1. Add functions and objects for Temporary Directories + * [Pull request #244](https://github.com/ignitionrobotics/ign-common/pull/244) + +1. Fix memory corruption & leaks in Image + * [Pull request #240](https://github.com/ignitionrobotics/ign-common/pull/240) + +1. Fix a typo in VideoEncoder_TEST. + * [Pull request #231](https://github.com/ignitionrobotics/ign-common/pull/231) + +1. Fix segfault caused by destruction order of Event and Connection + * [Pull request #234](https://github.com/ignitionrobotics/ign-common/pull/234) + +1. Infrastructure + * [Pull request #62](https://github.com/ignitionrobotics/ign-common/pull/62) + * [Pull request #55](https://github.com/ignitionrobotics/ign-common/pull/55) + * [Pull request #241](https://github.com/ignitionrobotics/ign-common/pull/241) ## Ignition Common 4.2.0 (2021-08-02) From 14a0daa6185bd7a059095c010fffa4ceb1b57063 Mon Sep 17 00:00:00 2001 From: Steve Peters Date: Fri, 24 Sep 2021 13:09:02 -0700 Subject: [PATCH 04/10] README: link directly to Contributing page (#252) Signed-off-by: Steve Peters --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 61ed6dc51..7b2cd727a 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ Refer to the following table for information about important directories and fil # Contributing Please see -[CONTRIBUTING.md](https://github.com/ignitionrobotics/ign-gazebo/blob/main/CONTRIBUTING.md). +[CONTRIBUTING.md](https://ignitionrobotics.org/docs/all/contributing). # Code of Conduct From 8c7697ea087c566e8fc190a8d271f80dcd07fe49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Mon, 27 Sep 2021 18:57:03 +0200 Subject: [PATCH 05/10] Add macos install instructions (#253) Signed-off-by: ahcorde --- tutorials/install.md | 47 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tutorials/install.md b/tutorials/install.md index 886f08f35..7838ad349 100644 --- a/tutorials/install.md +++ b/tutorials/install.md @@ -21,6 +21,22 @@ sudo apt install libignition-common<#>-dev Be sure to replace `<#>` with a number value, such as 2 or 3, depending on which version you need. +### macOS + +On macOS, add OSRF packages: + ``` + ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" + brew tap osrf/simulation + ``` + +Install Ignition Common: + ``` + brew install ignition-common<#> + ``` + +Be sure to replace `<#>` with a number value, such as 3 or 4, depending on +which version you need. + ## Windows Install [Conda package management system](https://docs.conda.io/projects/conda/en/latest/user-guide/install/download.html). @@ -116,6 +132,36 @@ conda install libignition-cmake<#> libignition-math<#> --channel conda-forge sudo make install ``` +### macOS + +1. Clone the repository + ``` + git clone https://github.com/ignitionrobotics/ign-common -b ign-common<#> + ``` + Be sure to replace `<#>` with a number value, such as 3 or 4, depending on + which version you need. + +2. Install dependencies + ``` + brew install --only-dependencies ignition-common<#> + ``` + Be sure to replace `<#>` with a number value, such as 3 or 4, depending on + which version you need. + +3. Configure and build + ``` + cd ign-common + mkdir build + cd build + cmake .. + make + ``` + +4. Optionally, install + ``` + sudo make install + ``` + ### Windows This assumes you have created and activated a Conda environment while installing the Prerequisites. @@ -185,4 +231,3 @@ Follow these steps to run tests and static code analysis in your clone of this r ``` make codecheck ``` - From 2af68956f401203048c8b6336b0b3630ae5aca70 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 27 Sep 2021 18:08:16 -0500 Subject: [PATCH 06/10] 4.3.0 (#254) Signed-off-by: Michael Carroll --- CMakeLists.txt | 2 +- Changelog.md | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c5745afb8..fa9d0b56a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,7 +14,7 @@ set(IGN_CMAKE_VER ${ignition-cmake2_VERSION_MAJOR}) #============================================================================ # Configure the project #============================================================================ -ign_configure_project(VERSION_SUFFIX pre1) +ign_configure_project() #============================================================================ # Set project-specific options diff --git a/Changelog.md b/Changelog.md index 651db1732..ae54009ff 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,6 @@ ## Ignition Common 4.x -## Ignition Common 4.3.0 (2021-09-XX) +## Ignition Common 4.3.0 (2021-09-27) 1. Remove ign-utils from TempDirectory * [Pull request #248](https://github.com/ignitionrobotics/ign-common/pull/248) @@ -21,6 +21,10 @@ * [Pull request #62](https://github.com/ignitionrobotics/ign-common/pull/62) * [Pull request #55](https://github.com/ignitionrobotics/ign-common/pull/55) * [Pull request #241](https://github.com/ignitionrobotics/ign-common/pull/241) + +1. Documentation + * [Pull request #252](https://github.com/ignitionrobotics/ign-common/pull/252) + * [Pull request #253](https://github.com/ignitionrobotics/ign-common/pull/253) ## Ignition Common 4.2.0 (2021-08-02) From 773a8083d95bfb3dab16a493173bfa6609042012 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Thu, 14 Oct 2021 13:51:19 -0700 Subject: [PATCH 07/10] Add support for animation tension (#256) * Add support for animation tension Signed-off-by: Nate Koenig * Added a test and a todo Signed-off-by: Nate Koenig Co-authored-by: Nate Koenig --- graphics/include/ignition/common/Animation.hh | 27 +++++++++++++++++++ graphics/src/Animation.cc | 24 ++++++++++++++++- graphics/src/Animation_TEST.cc | 24 +++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/graphics/include/ignition/common/Animation.hh b/graphics/include/ignition/common/Animation.hh index de06e6374..5207d7eb5 100644 --- a/graphics/include/ignition/common/Animation.hh +++ b/graphics/include/ignition/common/Animation.hh @@ -117,6 +117,21 @@ namespace ignition public: PoseAnimation(const std::string &_name, const double _length, const bool _loop); + /// \brief Constructor + /// \param[in] _name String name of the animation. This should be unique. + /// \param[in] _length Length of the animation in seconds + /// \param[in] _loop True == loop the animation + /// \param[in] _tension The tension of the trajectory spline. The + /// default value of zero equates to a Catmull-Rom spline, which may + /// also cause the animation to overshoot keyframes. A value of one will + /// cause the animation to stick to the keyframes. This value should + /// be in the range 0..1. + /// \todo(nkoenig) Remove this in ign-common5, and use a single + /// consutrctory with a default _tension of 0. + public: PoseAnimation(const std::string &_name, + const double _length, const bool _loop, + double _tension); + /// \brief Create a pose keyframe at the given time /// \param[in] _time Time at which to create the keyframe /// \return Pointer to the new keyframe @@ -234,6 +249,18 @@ namespace ignition std::map _waypoints); + /// \brief Load all waypoints in the trajectory + /// \param[in] _waypoints Map of waypoints, where the key is the absolute + /// time of the waypoint and the value is the pose. + /// \param[in] _tension The tension of the trajectory spline. The + /// default value of zero equates to a Catmull-Rom spline, which may + /// also cause the animation to overshoot keyframes. A value of one will + /// cause the animation to stick to the keyframes. This value should + /// be in the range 0..1. + public: void SetWaypoints( + std::map + _waypoints, double _tension); + /// \brief Private data pointer. IGN_UTILS_IMPL_PTR(dataPtr) }; diff --git a/graphics/src/Animation.cc b/graphics/src/Animation.cc index 2234ef4b5..c9089dd10 100644 --- a/graphics/src/Animation.cc +++ b/graphics/src/Animation.cc @@ -91,6 +91,9 @@ class PoseAnimation::Implementation /// \brief determines if the interpolation splines need building public: bool build{false}; + + /// \brief Spline tension parameter. + public: double tension{0.0}; }; ///////////////////////////////////////////////// @@ -290,6 +293,15 @@ PoseAnimation::PoseAnimation(const std::string &_name, const double _length, { } +///////////////////////////////////////////////// +PoseAnimation::PoseAnimation(const std::string &_name, const double _length, + const bool _loop, double _tension) +: Animation(_name, _length, _loop), + dataPtr(ignition::utils::MakeImpl()) +{ + this->dataPtr->tension = math::clamp(_tension, 0.0, 1.0); +} + ///////////////////////////////////////////////// PoseKeyFrame *PoseAnimation::CreateKeyFrame(const double _time) { @@ -309,6 +321,7 @@ void PoseAnimation::BuildInterpolationSplines() this->dataPtr->positionSpline->AutoCalculate(false); this->dataPtr->rotationSpline->AutoCalculate(false); + this->dataPtr->positionSpline->Tension(this->dataPtr->tension); this->dataPtr->positionSpline->Clear(); this->dataPtr->rotationSpline->Clear(); @@ -511,6 +524,15 @@ common::PoseAnimation *TrajectoryInfo::Waypoints() const void TrajectoryInfo::SetWaypoints( std::map _waypoints) { + this->SetWaypoints(_waypoints, 0.0); +} + +///////////////////////////////////////////////// +void TrajectoryInfo::SetWaypoints( + std::map _waypoints, + double _tension) +{ + _tension = math::clamp(_tension, 0.0, 1.0); this->dataPtr->segDistance.clear(); auto first = _waypoints.begin(); @@ -523,7 +545,7 @@ void TrajectoryInfo::SetWaypoints( animName << this->AnimIndex() << "_" << this->Id(); std::shared_ptr anim = std::make_shared(animName.str(), - std::chrono::duration(this->Duration()).count(), false); + std::chrono::duration(this->Duration()).count(), false, _tension); auto prevPose = first->second.Pos(); for (auto pIter = _waypoints.begin(); pIter != _waypoints.end(); ++pIter) diff --git a/graphics/src/Animation_TEST.cc b/graphics/src/Animation_TEST.cc index 4c1eccece..3ad3c5496 100644 --- a/graphics/src/Animation_TEST.cc +++ b/graphics/src/Animation_TEST.cc @@ -87,6 +87,30 @@ TEST_F(AnimationTest, PoseAnimation) math::Quaterniond(0.0302776, 0.0785971, 0.109824)); } +///////////////////////////////////////////////// +TEST_F(AnimationTest, PoseAnimationTension) +{ + // Test using a tension value of 1.0, which should cause the pose + // animation to hit the key frames. + + common::PoseAnimation animTension("test-tension", 10.0, false, 1.0); + common::PoseKeyFrame *keyTension = animTension.CreateKeyFrame(0.0); + keyTension->Translation(math::Vector3d(0, 0, 0)); + keyTension->Rotation(math::Quaterniond(0, 0, 0)); + + animTension.AddTime(5.0); + keyTension->Translation(math::Vector3d(10, 20, 30)); + keyTension->Rotation(math::Quaterniond(0.1, 0.2, 0.3)); + animTension.Time(4.0); + + common::PoseKeyFrame interpolatedKeyTension(-1.0); + animTension.InterpolatedKeyFrame(interpolatedKeyTension); + EXPECT_TRUE(interpolatedKeyTension.Translation() == + math::Vector3d(10, 20, 30)); + EXPECT_TRUE(interpolatedKeyTension.Rotation() == + math::Quaterniond(0.1, 0.2, 0.3)); +} + ///////////////////////////////////////////////// TEST_F(AnimationTest, NumericAnimation) { From 51c8e067ccacc8717186a763a8df76de22242ae1 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 15 Oct 2021 12:46:25 -0500 Subject: [PATCH 08/10] Prepartions for 4.4.0 (#261) Signed-off-by: Michael Carroll --- Changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Changelog.md b/Changelog.md index ae54009ff..0f1b996c8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,10 @@ ## Ignition Common 4.x +## Ignition Common 4.4.0 (2021-10-15) + +1. Add support for animation tension + * [Pull request #256](https://github.com/ignitionrobotics/ign-common/pull/256) + ## Ignition Common 4.3.0 (2021-09-27) 1. Remove ign-utils from TempDirectory From 8ba9b71526b63193159adc577976949e83a089ee Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 15 Oct 2021 13:24:48 -0500 Subject: [PATCH 09/10] Bump CMakeLists version Signed-off-by: Michael Carroll --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fa9d0b56a..be87aca8e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.10.2 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(ignition-common4 VERSION 4.3.0) +project(ignition-common4 VERSION 4.4.0) #============================================================================ # Find ignition-cmake From b337f4e0af725cd6dc1b3942c2114eaf311a9817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Tue, 9 Nov 2021 06:39:33 +0100 Subject: [PATCH 10/10] Added method to remove meshes from the MeshManager (#222) Signed-off-by: ahcorde Signed-off-by: Nate Koenig Co-authored-by: Louise Poubel Co-authored-by: Michael Carroll Co-authored-by: Nate Koenig Co-authored-by: Nate Koenig --- .../include/ignition/common/MeshManager.hh | 9 +++++++ graphics/src/MeshManager.cc | 27 +++++++++++++++++++ graphics/src/MeshManager_TEST.cc | 22 +++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/graphics/include/ignition/common/MeshManager.hh b/graphics/include/ignition/common/MeshManager.hh index e7d33f51f..f7dc59ff5 100644 --- a/graphics/include/ignition/common/MeshManager.hh +++ b/graphics/include/ignition/common/MeshManager.hh @@ -100,6 +100,15 @@ namespace ignition /// \param[in] the mesh to add. public: void AddMesh(Mesh *_mesh); + /// \brief Remove a mesh based on a name. + /// \param[in] _name Name of the mesh to remove. + /// \return True if the mesh was removed, false if the mesh with the + /// provided name could not be found. + public: bool RemoveMesh(const std::string &_name); + + /// \brief Remove all meshes. + public: void RemoveAll(); + /// \brief Get a mesh by name. /// \param[in] _name the name of the mesh to look for /// \return the mesh or nullptr if not found diff --git a/graphics/src/MeshManager.cc b/graphics/src/MeshManager.cc index 8a53efc92..037ad5afc 100644 --- a/graphics/src/MeshManager.cc +++ b/graphics/src/MeshManager.cc @@ -241,6 +241,33 @@ const Mesh *MeshManager::MeshByName(const std::string &_name) const return nullptr; } +////////////////////////////////////////////////// +void MeshManager::RemoveAll() +{ + std::lock_guard lock(this->dataPtr->mutex); + for (auto m : this->dataPtr->meshes) + { + delete m.second; + } + this->dataPtr->meshes.clear(); +} + +////////////////////////////////////////////////// +bool MeshManager::RemoveMesh(const std::string &_name) +{ + std::lock_guard lock(this->dataPtr->mutex); + + auto iter = this->dataPtr->meshes.find(_name); + if (iter != this->dataPtr->meshes.end()) + { + delete iter->second; + this->dataPtr->meshes.erase(iter); + return true; + } + + return false; +} + ////////////////////////////////////////////////// bool MeshManager::HasMesh(const std::string &_name) const { diff --git a/graphics/src/MeshManager_TEST.cc b/graphics/src/MeshManager_TEST.cc index fe75c0aff..76b198d3d 100644 --- a/graphics/src/MeshManager_TEST.cc +++ b/graphics/src/MeshManager_TEST.cc @@ -267,6 +267,28 @@ TEST_F(MeshManager, CreateExtrudedPolylineInvalid) EXPECT_TRUE(!common::MeshManager::Instance()->HasMesh(meshName)); } +///////////////////////////////////////////////// +TEST_F(MeshManager, Remove) +{ + auto mgr = common::MeshManager::Instance(); + + EXPECT_FALSE(mgr->HasMesh("box")); + mgr->CreateBox("box", + ignition::math::Vector3d(1, 1, 1), + ignition::math::Vector2d(0, 0)); + EXPECT_TRUE(mgr->HasMesh("box")); + + mgr->CreateSphere("sphere", 1.0, 1, 1); + EXPECT_TRUE(mgr->HasMesh("sphere")); + + EXPECT_TRUE(mgr->RemoveMesh("box")); + EXPECT_FALSE(mgr->HasMesh("box")); + EXPECT_TRUE(mgr->HasMesh("sphere")); + + mgr->RemoveAll(); + EXPECT_FALSE(mgr->HasMesh("sphere")); +} + ///////////////////////////////////////////////// int main(int argc, char **argv) {