From 6533bc45af1df0956f6c02416b15e2136e01d519 Mon Sep 17 00:00:00 2001 From: tgreier Date: Mon, 21 Dec 2020 13:40:09 -0500 Subject: [PATCH 1/4] Logging interface (#41) * Add in a new rcl_logging_interface package. This has just the header files that the implementations need to provide, and that the callers can call. Also port log4cxx, noop, and spdlog backends to use this new interface. Signed-off-by: Chris Lalancette Signed-off-by: tgreier --- rcl_logging_interface/CMakeLists.txt | 38 ++++ rcl_logging_interface/QUALITY_DECLARATION.md | 185 ++++++++++++++++++ .../rcl_logging_interface.h | 77 ++++---- .../visibility_control.h | 57 ++++++ rcl_logging_interface/package.xml | 18 ++ rcl_logging_log4cxx/CMakeLists.txt | 27 +-- .../rcl_logging_log4cxx/visibility_control.h | 54 ----- rcl_logging_log4cxx/package.xml | 1 + .../rcl_logging_log4cxx.cpp | 17 +- rcl_logging_noop/CMakeLists.txt | 18 +- rcl_logging_noop/package.xml | 1 + .../src/rcl_logging_noop/rcl_logging_noop.cpp | 8 +- rcl_logging_spdlog/CMakeLists.txt | 16 +- .../rcl_logging_spdlog/logging_interface.h | 84 -------- .../rcl_logging_spdlog/visibility_control.h | 54 ----- rcl_logging_spdlog/package.xml | 1 + rcl_logging_spdlog/src/rcl_logging_spdlog.cpp | 13 +- .../benchmark/benchmark_logging_interface.cpp | 2 +- .../test/test_logging_interface.cpp | 22 +-- 19 files changed, 381 insertions(+), 312 deletions(-) create mode 100644 rcl_logging_interface/CMakeLists.txt create mode 100644 rcl_logging_interface/QUALITY_DECLARATION.md rename rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h => rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h (51%) create mode 100644 rcl_logging_interface/include/rcl_logging_interface/visibility_control.h create mode 100644 rcl_logging_interface/package.xml delete mode 100644 rcl_logging_log4cxx/include/rcl_logging_log4cxx/visibility_control.h delete mode 100644 rcl_logging_spdlog/include/rcl_logging_spdlog/logging_interface.h delete mode 100644 rcl_logging_spdlog/include/rcl_logging_spdlog/visibility_control.h diff --git a/rcl_logging_interface/CMakeLists.txt b/rcl_logging_interface/CMakeLists.txt new file mode 100644 index 0000000..1dd9204 --- /dev/null +++ b/rcl_logging_interface/CMakeLists.txt @@ -0,0 +1,38 @@ +cmake_minimum_required(VERSION 3.5) + +project(rcl_logging_interface) + +# Default to C11 +if(NOT CMAKE_C_STANDARD) + set(CMAKE_C_STANDARD 11) +endif() +# Default to C++14 +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 14) +endif() + +if(NOT WIN32) + add_compile_options(-Wall -Wextra -Wpedantic) +endif() + +find_package(ament_cmake_ros REQUIRED) +find_package(rcutils REQUIRED) + +add_library(${PROJECT_NAME} INTERFACE) +target_include_directories(${PROJECT_NAME} INTERFACE + "$") +ament_target_dependencies(${PROJECT_NAME} INTERFACE rcutils) + +install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME} + ARCHIVE DESTINATION lib + LIBRARY DESTINATION lib + RUNTIME DESTINATION bin) + +install(DIRECTORY include/${PROJECT_NAME}/ + DESTINATION include/${PROJECT_NAME} +) + +ament_export_include_directories(include) +ament_export_targets(${PROJECT_NAME}) +ament_export_dependencies(rcutils) +ament_package() diff --git a/rcl_logging_interface/QUALITY_DECLARATION.md b/rcl_logging_interface/QUALITY_DECLARATION.md new file mode 100644 index 0000000..9b7ff36 --- /dev/null +++ b/rcl_logging_interface/QUALITY_DECLARATION.md @@ -0,0 +1,185 @@ +This document is a declaration of software quality for the `rcl_logging_interface` package, based on the guidelines in [REP-2004](https://github.com/ros-infrastructure/rep/blob/rep-2004/rep-2004.rst). + +# `rcl_logging_interface` Quality Declaration + +The package `rcl_logging_interface` claims to be in the **Quality Level 4** category. + +Below are the rationales, notes, and caveats for this claim, organized by each requirement listed in the [Package Quality Categories in REP-2004](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories) of the ROS2 developer guide. + +## Version Policy [1] + +### Version Scheme [1.i] + +`rcl_logging_interface` uses `semver` according to the recommendation for ROS Core packages in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#versioning). + +### Version Stability [1.ii] + +Currently this package is at or above a stable version, i.e. `>= 1.0.0`. + +### Public API Declaration [1.iii] + +All symbols in the installed headers are considered part of the public API. + +All installed headers are in the [include](./include/rcl_logging_interface) directory of the package. + +### API Stability Policy [1.iv] + +`rcl_logging_interface` will not break public API within a released ROS distribution, i.e. no major releases once the ROS distribution is released. + +### ABI Stability Policy [1.v] + +`rcl_logging_interface` contains only header files, and thus ABI Stability does not apply to it. + +### ABI and ABI Stability Within a Released ROS Distribution [1.vi] + +`rcl_logging_interface` will not break API nor ABI within a released ROS distribution, i.e. no major releases once the ROS distribution is released. + +## Change Control Process [2] + +`rcl_logging_interface` follows the recommended guidelines for ROS Core packages in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#change-control-process). + +### Change Requests [2.i] + +All changes will occur through a pull request, check [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#change-control-process) for additional information. + +### Contributor Origin [2.ii] + +This package uses DCO as its confirmation of contributor origin policy. More information can be found in [CONTRIBUTING](../CONTRIBUTING.md) + +### Peer Review Policy [2.iii] + +Following the recommended guidelines in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#change-control-process) all pull requests must have at least 1 peer review. + +### Continuous Integration [2.iv] + +All pull requests must pass CI on all [tier 1 platforms](https://www.ros.org/reps/rep-2000.html#support-tiers) + +Currently nightly results can be seen here: +* [linux-aarch64_release](https://ci.ros2.org/view/nightly/job/nightly_linux-aarch64_release/lastBuild/testReport/rcl_logging_interface/) +* [linux_release](https://ci.ros2.org/view/nightly/job/nightly_linux_release/lastBuild/testReport/rcl_logging_interface`/) +* [mac_osx_release](https://ci.ros2.org/view/nightly/job/nightly_osx_release/lastBuild/testReport/rcl_logging_interface/) +* [windows_release](https://ci.ros2.org/view/nightly/job/nightly_win_rel/lastBuild/testReport/rcl_logging_interface/) + +### Documentation Policy [2.v] + +All pull requests must resolve related documentation changes before merging. + +## Documentation [3] + +### Feature Documentation [3.i] + +`rcl_logging_interface` does not have feature documentation. + +### Public API Documentation [3.ii] + +`rcl_logging_interface` does not have public API documentation. + +### License [3.iii] + +The license for `rcl_logging_interface` is Apache 2.0, and a summary is in each source file, the type is declared in the [`package.xml`](./package.xml) manifest file, and a full copy of the license is in the [`LICENSE`](../LICENSE) file. + +There is an automated test which runs a linter that ensures each file has a license statement. [Here](https://ci.ros2.org/view/nightly/job/nightly_linux_release/lastBuild/testReport/rcl_logging_interface/) can be found a list with the latest results of the various linters being run on the package. + +### Copyright Statements [3.iv] + +The copyright holders each provide a statement of copyright in each source code file in `rcl_logging_interface`. + +There is an automated test which runs a linter that ensures each file has at least one copyright statement. Latest linter result report can be seen [here](https://ci.ros2.org/view/nightly/job/nightly_linux_release/lastBuild/testReport/rcl_logging_interface/copyright/). + +## Testing [4] + +### Feature Testing [4.i] + +`rcl_logging_interface` does not include feature testing. + +### Public API Testing [4.ii] + +`rcl_logging_interface` does not include Public API testing. + +### Coverage [4.iii] + +`rcl_logging_interface` does not include tests, so coverage is not provided. + +### Performance [4.iv] + +`rcl_logging_interface` does not conduct performance tests. + +### Linters and Static Analysis [4.v] + +`rcl_logging_interface` uses and passes all the standard linters and static analysis tools for a C package as described in the [ROS 2 Developer Guide](https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#linters-and-static-analysis). Passing implies there are no linter/static errors when testing against CI of supported platforms. + +Currently nightly results can be seen here: +* [linux-aarch64_release](https://ci.ros2.org/view/nightly/job/nightly_linux-aarch64_release/lastBuild/testReport/rcl_logging_interface/) +* [linux_release](https://ci.ros2.org/view/nightly/job/nightly_linux_release/lastBuild/testReport/rcl_logging_interface/) +* [mac_osx_release](https://ci.ros2.org/view/nightly/job/nightly_osx_release/lastBuild/testReport/rcl_logging_interface/) +* [windows_release](https://ci.ros2.org/view/nightly/job/nightly_win_rel/lastBuild/testReport/rcl_logging_interface/) + +## Dependencies [5] + +Below are evaluations of each of `rcl_logging_interface`'s run-time and build-time dependencies that have been determined to influence the quality. + +`rcl_logging_interface` depends on the ROS package `rcutils`. + +`rcutils` was declared to be Quality Level 4 [here](https://github.com/ros2/rcutils/blob/master/QUALITY_DECLARATION.md). + +### Optional Direct Runtime ROS Dependencies [5.ii] + +`rcl_logging_interface` has no optional Direct Runtime ROS dependencies that need to be considered for this declaration. + +### Direct Runtime non-ROS Dependency [5.iii] + +`rcl_logging_spdlog` has no Direct Runtime non-ROS dependencies that need to be considered for this declaration. + +## Platform Support [6] + +`rcl_logging_interface` supports all of the tier 1 platforms as described in [REP-2000](https://www.ros.org/reps/rep-2000.html#support-tiers), and tests each change against all of them. + +## Security [7] + +### Vulnerability Disclosure Policy [7.i] + +This package conforms to the Vulnerability Disclosure Policy in [REP-2006](https://www.ros.org/reps/rep-2006.html). + +# Current status Summary + +The chart below compares the requirements in the REP-2004 with the current state of the `rcl` package. + +|Number| Requirement| Current state | +|--|--|--| +|1| **Version policy** |---| +|1.i|Version Policy available | ✓ | +|1.ii|Stable version | ☓ | +|1.iii|Declared public API | ✓ | +|1.iv|API stability policy | ✓ | +|1.v|ABI stability policy | ✓ | +|1.vi_|API/ABI stable within ros distribution | ✓ | +|2| **Change control process** |---| +|2.i| All changes occur on change request | ✓ | +|2.ii| Contributor origin (DCO, CLA, etc) | ✓ | +|2.iii| Peer review policy | ✓ | +|2.iv| CI policy for change requests | ✓ | +|2.v| Documentation policy for change requests | ✓ | +|3| **Documentation** | --- | +|3.i| Per feature documentation | ☓ | +|3.ii| Per public API item documentation | ☓ | +|3.iii| Declared License(s) | ✓ | +|3.iv| Copyright in source files| ✓ | +|3.v.a| Quality declaration linked to README | ✓ | +|3.v.b| Centralized declaration available for peer review | ✓ | +|4| Testing | --- | +|4.i| Feature items tests | ☓ | +|4.ii| Public API tests | ☓ | +|4.iii.a| Using coverage | ☓ | +|4.iii.a| Coverage policy | ☓ | +|4.iv.a| Performance tests (if applicable) | ☓ | +|4.iv.b| Performance tests policy| ✓ | +|4.v.a| Code style enforcement (linters)| ✓ | +|4.v.b| Use of static analysis tools | ✓ | +|5| Dependencies | --- | +|5.i| Must not have ROS lower level dependencies | ✓ | +|5.ii| Optional ROS lower level dependencies | ✓ | +|5.iii| Justifies quality use of non-ROS dependencies | ✓ | +|6| Platform support | --- | +|6.i| Support targets Tier1 ROS platforms | ✓ | +|7| Security | --- | +|7.i| Vulnerability Disclosure Policy | ✓ | diff --git a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h b/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h similarity index 51% rename from rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h rename to rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h index c50c09b..235942f 100644 --- a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/logging_interface.h +++ b/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h @@ -1,4 +1,4 @@ -// Copyright 2018 Open Source Robotics Foundation, Inc. +// Copyright 2020 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,54 +12,68 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef RCL_LOGGING_LOG4CXX__LOGGING_INTERFACE_H_ -#define RCL_LOGGING_LOG4CXX__LOGGING_INTERFACE_H_ +#ifndef RCL_LOGGING_INTERFACE__RCL_LOGGING_INTERFACE_H_ +#define RCL_LOGGING_INTERFACE__RCL_LOGGING_INTERFACE_H_ -#include "rcl_logging_log4cxx/visibility_control.h" +#include "rcl_logging_interface/visibility_control.h" #include "rcutils/allocator.h" #ifdef __cplusplus extern "C" { #endif -typedef int rcl_logging_ret_t; +typedef enum +{ + RCL_LOGGING_RET_OK = 0, + RCL_LOGGING_RET_ERROR = 2, + RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST = 21, + RCL_LOGGING_RET_CONFIG_FILE_INVALID = 22, +} rcl_logging_ret_t; -/// Initialize log4cxx logging library. -/* +/// Initialize the external logging library. +/** * \param[in] config_file The location of a config file that the external * logging library should use to configure itself. - * If no config file is provided this will be set to an empty string. - * Must be a NULL terminated c string. - * \param[in] allocator The allocator to use for memory allocation. This is + * If provided, it must be a null terminated C string. + * If set to NULL or the empty string, the logging library will use defaults. + * \param[in] allocator The allocator to use for memory allocation. This is * an rcutils_allocator_t rather than an rcl_allocator_t to ensure that the * rcl_logging_* packages don't have a circular dependency back to rcl. - * \return RCL_LOGGING_RET_OK if initialized successfully, or + * \return RCL_LOGGING_RET_OK if initialized successfully. * \return RCL_LOGGING_RET_ERROR if an unspecified error occurs. + * \return RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST if a config_file is provided + * but the file doesn't exist. + * \return RCL_LOGGING_RET_CONFIG_FILE_INVALID if a config_file is provided but + * the logging backend doesn't understand it. */ -RCL_LOGGING_PUBLIC -rcl_logging_ret_t rcl_logging_external_initialize( - const char * config_file, - rcutils_allocator_t allocator); +RCL_LOGGING_INTERFACE_PUBLIC +RCUTILS_WARN_UNUSED +rcl_logging_ret_t +rcl_logging_external_initialize(const char * config_file, rcutils_allocator_t allocator); -/// Free the resources allocated for the log4cxx external logging system. +/// Free the resources allocated for the external logging system. /** * This puts the system into a state equivalent to being uninitialized. * - * \return always RCL_LOGGING_RET_OK. + * \return RCL_LOGGING_RET_OK if successfully shutdown, or + * \return RCL_LOGGING_RET_ERROR if an unspecified error occurs. */ -RCL_LOGGING_PUBLIC -rcl_logging_ret_t rcl_logging_external_shutdown(); +RCL_LOGGING_INTERFACE_PUBLIC +RCUTILS_WARN_UNUSED +rcl_logging_ret_t +rcl_logging_external_shutdown(); /// Log a message. /** * \param[in] severity The severity level of the message being logged. * \param[in] name The name of the logger, must either be a null terminated - * c string or NULL. + * C string or NULL. * If NULL or empty the root logger will be used. - * \param[in] msg The message to be logged. Must be a null terminated c string. + * \param[in] msg The message to be logged. Must be a null terminated C string. */ -RCL_LOGGING_PUBLIC -void rcl_logging_external_log(int severity, const char * name, const char * msg); +RCL_LOGGING_INTERFACE_PUBLIC +void +rcl_logging_external_log(int severity, const char * name, const char * msg); /// Set the severity level for a logger. /** @@ -68,20 +82,17 @@ void rcl_logging_external_log(int severity, const char * name, const char * msg) * the root logger. * * \param[in] name The name of the logger. - * Must be a NULL terminated c string or NULL. - * \param[in] level The severity level to be used for the specified logger. Values: - * RCUTILS_LOG_SEVERITY_DEBUG, RCUTILS_LOG_SEVERITY_UNSET, RCUTILS_LOG_SEVERITY_DEBUG, - * RCUTILS_LOG_SEVERITY_INFO, RCUTILS_LOG_SEVERITY_WARN, RCUTILS_LOG_SEVERITY_ERROR, - * RCUTILS_LOG_SEVERITY_FATAL - * \return always RCL_LOGGING_RET_OK. + * Must be a null terminated C string or NULL. + * \param[in] level The severity level to be used for the specified logger. + * \return RCL_LOGGING_RET_OK if set successfully, or + * \return RCL_LOGGING_RET_ERROR if an unspecified error occurs. */ -RCL_LOGGING_PUBLIC +RCL_LOGGING_INTERFACE_PUBLIC RCUTILS_WARN_UNUSED rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int level); - #ifdef __cplusplus -} /* extern "C" */ +} #endif -#endif // RCL_LOGGING_LOG4CXX__LOGGING_INTERFACE_H_ +#endif // RCL_LOGGING_INTERFACE__RCL_LOGGING_INTERFACE_H_ diff --git a/rcl_logging_interface/include/rcl_logging_interface/visibility_control.h b/rcl_logging_interface/include/rcl_logging_interface/visibility_control.h new file mode 100644 index 0000000..ddb436c --- /dev/null +++ b/rcl_logging_interface/include/rcl_logging_interface/visibility_control.h @@ -0,0 +1,57 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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 RCL_LOGGING_INTERFACE__VISIBILITY_CONTROL_H_ +#define RCL_LOGGING_INTERFACE__VISIBILITY_CONTROL_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +// This logic was borrowed (then namespaced) from the examples on the gcc wiki: +// https://gcc.gnu.org/wiki/Visibility + +#if defined _WIN32 || defined __CYGWIN__ + #ifdef __GNUC__ + #define RCL_LOGGING_INTERFACE_EXPORT __attribute__ ((dllexport)) + #define RCL_LOGGING_INTERFACE_IMPORT __attribute__ ((dllimport)) + #else + #define RCL_LOGGING_INTERFACE_EXPORT __declspec(dllexport) + #define RCL_LOGGING_INTERFACE_IMPORT __declspec(dllimport) + #endif + #ifdef RCL_LOGGING_INTERFACE_BUILDING_DLL + #define RCL_LOGGING_INTERFACE_PUBLIC RCL_LOGGING_INTERFACE_EXPORT + #else + #define RCL_LOGGING_INTERFACE_PUBLIC RCL_LOGGING_INTERFACE_IMPORT + #endif + #define RCL_LOGGING_INTERFACE_PUBLIC_TYPE RCL_LOGGING_INTERFACE_PUBLIC + #define RCL_LOGGING_INTERFACE_LOCAL +#else + #define RCL_LOGGING_INTERFACE_EXPORT __attribute__ ((visibility("default"))) + #define RCL_LOGGING_INTERFACE_IMPORT + #if __GNUC__ >= 4 + #define RCL_LOGGING_INTERFACE_PUBLIC __attribute__ ((visibility("default"))) + #define RCL_LOGGING_INTERFACE_LOCAL __attribute__ ((visibility("hidden"))) + #else + #define RCL_LOGGING_INTERFACE_PUBLIC + #define RCL_LOGGING_INTERFACE_LOCAL + #endif + #define RCL_LOGGING_INTERFACE_PUBLIC_TYPE +#endif + +#ifdef __cplusplus +} +#endif + +#endif // RCL_LOGGING_INTERFACE__VISIBILITY_CONTROL_H_ diff --git a/rcl_logging_interface/package.xml b/rcl_logging_interface/package.xml new file mode 100644 index 0000000..8522a32 --- /dev/null +++ b/rcl_logging_interface/package.xml @@ -0,0 +1,18 @@ + + + + rcl_logging_interface + 1.0.0 + Interface that rcl_logging backends needs to implement. + Chris Lalancette + Apache License 2.0 + Chris Lalancette + + ament_cmake_ros + + rcutils + + + ament_cmake + + diff --git a/rcl_logging_log4cxx/CMakeLists.txt b/rcl_logging_log4cxx/CMakeLists.txt index 0b903fd..afd1f83 100644 --- a/rcl_logging_log4cxx/CMakeLists.txt +++ b/rcl_logging_log4cxx/CMakeLists.txt @@ -14,11 +14,9 @@ endif() find_package(ament_cmake_ros REQUIRED) list(INSERT CMAKE_MODULE_PATH 0 "${CMAKE_CURRENT_SOURCE_DIR}/cmake") find_package(Log4cxx REQUIRED) +find_package(rcl_logging_interface REQUIRED) find_package(rcutils REQUIRED) -include_directories(include) - - if(NOT WIN32) add_compile_options(-Wall -Wextra -Wpedantic) else() @@ -29,22 +27,15 @@ else() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4275 /wd4251") endif() -set(${PROJECT_NAME}_sources - src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp -) - -if(NOT WIN32) - add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources}) -else() - add_library(${PROJECT_NAME} STATIC ${${PROJECT_NAME}_sources}) -endif() +add_library(${PROJECT_NAME} src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp) ament_target_dependencies(${PROJECT_NAME} Log4cxx + rcl_logging_interface rcutils ) -target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_BUILDING_DLL") +target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_INTERFACE_BUILDING_DLL") if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) @@ -53,16 +44,12 @@ if(BUILD_TESTING) ament_lint_auto_find_test_dependencies() endif() -install(TARGETS ${PROJECT_NAME} +install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib RUNTIME DESTINATION bin) -install(DIRECTORY include/${PROJECT_NAME}/ - DESTINATION include/${PROJECT_NAME} -) - -ament_export_include_directories(include) -ament_export_dependencies(ament_cmake) +ament_export_dependencies(ament_cmake rcl_logging_interface rcutils) ament_export_libraries(${PROJECT_NAME} log4cxx) +ament_export_targets(${PROJECT_NAME}) ament_package() diff --git a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/visibility_control.h b/rcl_logging_log4cxx/include/rcl_logging_log4cxx/visibility_control.h deleted file mode 100644 index 2816f24..0000000 --- a/rcl_logging_log4cxx/include/rcl_logging_log4cxx/visibility_control.h +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2018 Open Source Robotics Foundation, Inc. -// -// 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 RCL_LOGGING_LOG4CXX__VISIBILITY_CONTROL_H_ -#define RCL_LOGGING_LOG4CXX__VISIBILITY_CONTROL_H_ - -#ifdef __cplusplus -extern "C" { -#endif - -#if defined _WIN32 || defined __CYGWIN__ - #ifdef __GNUC__ - #define RCL_LOGGING_EXPORT __attribute__ ((dllexport)) - #define RCL_LOGGING_IMPORT __attribute__ ((dllimport)) - #else - #define RCL_LOGGING_EXPORT __declspec(dllexport) - #define RCL_LOGGING_IMPORT __declspec(dllimport) - #endif - #ifdef RCL_LOGGING_BUILDING_DLL - #define RCL_LOGGING_PUBLIC RCL_LOGGING_EXPORT - #else - #define RCL_LOGGING_PUBLIC RCL_LOGGING_IMPORT - #endif - #define RCL_LOGGING_PUBLIC_TYPE RCL_LOGGING_PUBLIC - #define RCL_LOGGING_LOCAL -#else - #define RCL_LOGGING_EXPORT __attribute__ ((visibility("default"))) - #define RCL_LOGGING_IMPORT - #if __GNUC__ >= 4 - #define RCL_LOGGING_PUBLIC __attribute__ ((visibility("default"))) - #define RCL_LOGGING_LOCAL __attribute__ ((visibility("hidden"))) - #else - #define RCL_LOGGING_PUBLIC - #define RCL_LOGGING_LOCAL - #endif - #define RCL_LOGGING_PUBLIC_TYPE -#endif - -#ifdef __cplusplus -} /* extern "C" */ -#endif - -#endif // RCL_LOGGING_LOG4CXX__VISIBILITY_CONTROL_H_ diff --git a/rcl_logging_log4cxx/package.xml b/rcl_logging_log4cxx/package.xml index 024292c..8c3c0c7 100644 --- a/rcl_logging_log4cxx/package.xml +++ b/rcl_logging_log4cxx/package.xml @@ -12,6 +12,7 @@ python3-empy log4cxx + rcl_logging_interface rcutils ament_cmake_gmock diff --git a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp index ba69f52..3adf80c 100644 --- a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp +++ b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include + #include #include #include @@ -62,17 +64,6 @@ static const log4cxx::LevelPtr map_external_log_level_to_library_level(int exter return level; } -#ifdef __cplusplus -extern "C" { -#endif - -#include "rcl_logging_log4cxx/logging_interface.h" - -#define RCL_LOGGING_RET_OK (0) -#define RCL_LOGGING_RET_ERROR (2) -#define RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST (21) -#define RCL_LOGGING_RET_CONFIG_FILE_INVALID (22) - rcl_logging_ret_t rcl_logging_external_initialize( const char * config_file, rcutils_allocator_t allocator) @@ -177,7 +168,3 @@ rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int l logger->setLevel(map_external_log_level_to_library_level(level)); return RCL_LOGGING_RET_OK; } - -#ifdef __cplusplus -} /* extern "C" */ -#endif diff --git a/rcl_logging_noop/CMakeLists.txt b/rcl_logging_noop/CMakeLists.txt index 37ef1fe..71c184b 100644 --- a/rcl_logging_noop/CMakeLists.txt +++ b/rcl_logging_noop/CMakeLists.txt @@ -12,6 +12,7 @@ if(NOT CMAKE_CXX_STANDARD) endif() find_package(ament_cmake_ros REQUIRED) +find_package(rcl_logging_interface REQUIRED) find_package(rcutils REQUIRED) if(NOT WIN32) @@ -22,30 +23,27 @@ set(${PROJECT_NAME}_sources src/rcl_logging_noop/rcl_logging_noop.cpp ) -if(NOT WIN32) - add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources}) -else() - # TODO(nburek): There is an issue with build on Windows where it is looking for a static lib - # and can't find it when trying to call find_package() for this package. This explicitly creates - # it. - add_library(${PROJECT_NAME} STATIC ${${PROJECT_NAME}_sources}) -endif() +add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources}) ament_target_dependencies(${PROJECT_NAME} + rcl_logging_interface rcutils ) +target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_INTERFACE_BUILDING_DLL") + if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) ament_lint_auto_find_test_dependencies() endif() -install(TARGETS ${PROJECT_NAME} +install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib RUNTIME DESTINATION bin) -ament_export_dependencies(ament_cmake) +ament_export_dependencies(ament_cmake rcl_logging_interface rcutils) ament_export_libraries(${PROJECT_NAME}) +ament_export_targets(${PROJECT_NAME}) ament_package() diff --git a/rcl_logging_noop/package.xml b/rcl_logging_noop/package.xml index 491d24a..bff5f08 100644 --- a/rcl_logging_noop/package.xml +++ b/rcl_logging_noop/package.xml @@ -11,6 +11,7 @@ ament_cmake_ros python3-empy + rcl_logging_interface rcutils ament_cmake_gmock diff --git a/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp b/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp index 25a9086..0530e14 100644 --- a/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp +++ b/rcl_logging_noop/src/rcl_logging_noop/rcl_logging_noop.cpp @@ -12,13 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include -extern "C" { - -typedef int rcl_logging_ret_t; -#define RCL_LOGGING_RET_OK (0) - rcl_logging_ret_t rcl_logging_external_initialize( const char * config_file, rcutils_allocator_t allocator) @@ -46,5 +42,3 @@ rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int l (void) level; return RCL_LOGGING_RET_OK; } - -} /* extern "C" */ diff --git a/rcl_logging_spdlog/CMakeLists.txt b/rcl_logging_spdlog/CMakeLists.txt index d3e25c5..0ae2837 100644 --- a/rcl_logging_spdlog/CMakeLists.txt +++ b/rcl_logging_spdlog/CMakeLists.txt @@ -12,8 +12,9 @@ if(NOT CMAKE_CXX_STANDARD) endif() find_package(ament_cmake_ros REQUIRED) +find_package(rcl_logging_interface REQUIRED) find_package(rcutils REQUIRED) -find_package(spdlog_vendor REQUIRED) # Provides spdlog 1.3.1 on platforms without it. +find_package(spdlog_vendor REQUIRED) # Provides spdlog on platforms without it. find_package(spdlog REQUIRED) if(NOT WIN32) @@ -21,27 +22,21 @@ if(NOT WIN32) endif() add_library(${PROJECT_NAME} src/rcl_logging_spdlog.cpp) -target_include_directories(${PROJECT_NAME} PUBLIC - "$" - "$") target_link_libraries(${PROJECT_NAME} spdlog::spdlog) ament_target_dependencies(${PROJECT_NAME} + rcl_logging_interface rcutils spdlog ) -target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_BUILDING_DLL") +target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_INTERFACE_BUILDING_DLL") install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib RUNTIME DESTINATION bin) -install(DIRECTORY include/${PROJECT_NAME}/ - DESTINATION include/${PROJECT_NAME} -) - if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) ament_lint_auto_find_test_dependencies() @@ -65,8 +60,7 @@ if(BUILD_TESTING) endif() endif() -ament_export_include_directories(include) -ament_export_dependencies(ament_cmake rcutils spdlog_vendor spdlog) +ament_export_dependencies(ament_cmake rcl_logging_interface rcutils spdlog_vendor spdlog) ament_export_libraries(${PROJECT_NAME}) ament_export_targets(${PROJECT_NAME}) ament_package() diff --git a/rcl_logging_spdlog/include/rcl_logging_spdlog/logging_interface.h b/rcl_logging_spdlog/include/rcl_logging_spdlog/logging_interface.h deleted file mode 100644 index b1c08d0..0000000 --- a/rcl_logging_spdlog/include/rcl_logging_spdlog/logging_interface.h +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2019 Open Source Robotics Foundation, Inc. -// -// 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 RCL_LOGGING_SPDLOG__LOGGING_INTERFACE_H_ -#define RCL_LOGGING_SPDLOG__LOGGING_INTERFACE_H_ - -#include "rcl_logging_spdlog/visibility_control.h" -#include "rcutils/allocator.h" - -#ifdef __cplusplus -extern "C" { -#endif - -typedef int rcl_logging_ret_t; - -/// Initialize spdlog logging library. -/* - * \param[in] config_file The location of a config file that the external - * logging library should use to configure itself. - * If no config file is provided this will be set to an empty string. - * Must be a NULL terminated c string. - * \param[in] allocator The allocator to use for memory allocation. This is - * an rcutils_allocator_t rather than an rcl_allocator_t to ensure that the - * rcl_logging_* packages don't have a circular dependency back to rcl. - * \return RCL_LOGGING_RET_OK if initialized successfully, or - * \return RCL_LOGGING_RET_ERROR if an unspecified error occurs. - */ -RCL_LOGGING_PUBLIC -rcl_logging_ret_t rcl_logging_external_initialize( - const char * config_file, - rcutils_allocator_t allocator); - -/// Free the resources allocated for the spdlog external logging system. -/** - * This puts the system into a state equivalent to being uninitialized. - * - * \return always RCL_LOGGING_RET_OK. - */ -RCL_LOGGING_PUBLIC -rcl_logging_ret_t rcl_logging_external_shutdown(); - -/// Log a message. -/** - * \param[in] severity The severity level of the message being logged. - * \param[in] name The name of the logger (unused). - * \param[in] msg The message to be logged. Must be a null terminated c string. - */ -RCL_LOGGING_PUBLIC -void rcl_logging_external_log(int severity, const char * name, const char * msg); - -/// Set the severity level for a logger. -/** - * This function sets the severity level for the specified logger. - * If the name provided is an empty string or NULL it will change the level of - * the root logger. - * - * \param[in] name The name of the logger (unused). - * \param[in] level The severity level to be used for the specified logger. Values: - * RCUTILS_LOG_SEVERITY_DEBUG, RCUTILS_LOG_SEVERITY_UNSET, RCUTILS_LOG_SEVERITY_DEBUG, - * RCUTILS_LOG_SEVERITY_INFO, RCUTILS_LOG_SEVERITY_WARN, RCUTILS_LOG_SEVERITY_ERROR, - * RCUTILS_LOG_SEVERITY_FATAL - * \return always RCL_LOGGING_RET_OK. - */ -RCL_LOGGING_PUBLIC -RCUTILS_WARN_UNUSED -rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int level); - - -#ifdef __cplusplus -} /* extern "C" */ -#endif - -#endif // RCL_LOGGING_SPDLOG__LOGGING_INTERFACE_H_ diff --git a/rcl_logging_spdlog/include/rcl_logging_spdlog/visibility_control.h b/rcl_logging_spdlog/include/rcl_logging_spdlog/visibility_control.h deleted file mode 100644 index 2095ba8..0000000 --- a/rcl_logging_spdlog/include/rcl_logging_spdlog/visibility_control.h +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2019 Open Source Robotics Foundation, Inc. -// -// 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 RCL_LOGGING_SPDLOG__VISIBILITY_CONTROL_H_ -#define RCL_LOGGING_SPDLOG__VISIBILITY_CONTROL_H_ - -#ifdef __cplusplus -extern "C" { -#endif - -#if defined _WIN32 || defined __CYGWIN__ - #ifdef __GNUC__ - #define RCL_LOGGING_EXPORT __attribute__ ((dllexport)) - #define RCL_LOGGING_IMPORT __attribute__ ((dllimport)) - #else - #define RCL_LOGGING_EXPORT __declspec(dllexport) - #define RCL_LOGGING_IMPORT __declspec(dllimport) - #endif - #ifdef RCL_LOGGING_BUILDING_DLL - #define RCL_LOGGING_PUBLIC RCL_LOGGING_EXPORT - #else - #define RCL_LOGGING_PUBLIC RCL_LOGGING_IMPORT - #endif - #define RCL_LOGGING_PUBLIC_TYPE RCL_LOGGING_PUBLIC - #define RCL_LOGGING_LOCAL -#else - #define RCL_LOGGING_EXPORT __attribute__ ((visibility("default"))) - #define RCL_LOGGING_IMPORT - #if __GNUC__ >= 4 - #define RCL_LOGGING_PUBLIC __attribute__ ((visibility("default"))) - #define RCL_LOGGING_LOCAL __attribute__ ((visibility("hidden"))) - #else - #define RCL_LOGGING_PUBLIC - #define RCL_LOGGING_LOCAL - #endif - #define RCL_LOGGING_PUBLIC_TYPE -#endif - -#ifdef __cplusplus -} /* extern "C" */ -#endif - -#endif // RCL_LOGGING_SPDLOG__VISIBILITY_CONTROL_H_ diff --git a/rcl_logging_spdlog/package.xml b/rcl_logging_spdlog/package.xml index cb4d0f9..184f739 100644 --- a/rcl_logging_spdlog/package.xml +++ b/rcl_logging_spdlog/package.xml @@ -13,6 +13,7 @@ spdlog_vendor spdlog + rcl_logging_interface rcutils spdlog_vendor diff --git a/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp b/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp index 86089e9..daa4cc3 100644 --- a/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp +++ b/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp @@ -29,14 +29,7 @@ #include "spdlog/spdlog.h" #include "spdlog/sinks/basic_file_sink.h" -#include "rcl_logging_spdlog/logging_interface.h" - -#define RCL_LOGGING_RET_OK (0) -#define RCL_LOGGING_RET_ERROR (2) - -#ifdef __cplusplus -extern "C" { -#endif +#include "rcl_logging_interface/rcl_logging_interface.h" static std::mutex g_logger_mutex; static std::unique_ptr g_root_logger = nullptr; @@ -174,7 +167,3 @@ rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int l return RCL_LOGGING_RET_OK; } - -#ifdef __cplusplus -} /* extern "C" */ -#endif diff --git a/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp b/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp index 348d3a6..743b0bc 100644 --- a/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp +++ b/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp @@ -16,7 +16,7 @@ #include #include -#include +#include #include diff --git a/rcl_logging_spdlog/test/test_logging_interface.cpp b/rcl_logging_spdlog/test/test_logging_interface.cpp index c68b9b8..0773490 100644 --- a/rcl_logging_spdlog/test/test_logging_interface.cpp +++ b/rcl_logging_spdlog/test/test_logging_interface.cpp @@ -26,7 +26,7 @@ #include "fixtures.hpp" #include "gtest/gtest.h" -#include "rcl_logging_spdlog/logging_interface.h" +#include "rcl_logging_interface/rcl_logging_interface.h" #define RCL_LOGGING_RET_OK (0) #define RCL_LOGGING_RET_ERROR (2) @@ -90,11 +90,11 @@ static fs::path current_path() TEST_F(LoggingTest, init_invalid) { // Config files are not supported by spdlog - EXPECT_EQ(2, rcl_logging_external_initialize("anything", allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize("anything", allocator)); rcutils_reset_error(); - EXPECT_EQ(2, rcl_logging_external_initialize(nullptr, bad_allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, bad_allocator)); rcutils_reset_error(); - EXPECT_EQ(2, rcl_logging_external_initialize(nullptr, invalid_allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, invalid_allocator)); rcutils_reset_error(); } @@ -106,7 +106,7 @@ TEST_F(LoggingTest, init_failure) // No home directory to write log to ASSERT_EQ(true, rcutils_set_env("HOME", nullptr)); ASSERT_EQ(true, rcutils_set_env("USERPROFILE", nullptr)); - EXPECT_EQ(2, rcl_logging_external_initialize(nullptr, allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); rcutils_reset_error(); // Force failure to create directories @@ -117,14 +117,14 @@ TEST_F(LoggingTest, init_failure) // ...fail to create .ros dir fs::path ros_dir = fake_home / ".ros"; std::fstream(ros_dir.string(), std::ios_base::out).close(); - EXPECT_EQ(2, rcl_logging_external_initialize(nullptr, allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); ASSERT_TRUE(fs::remove(ros_dir)); // ...fail to create .ros/log dir ASSERT_TRUE(fs::create_directories(ros_dir)); fs::path ros_log_dir = ros_dir / "log"; std::fstream(ros_log_dir.string(), std::ios_base::out).close(); - EXPECT_EQ(2, rcl_logging_external_initialize(nullptr, allocator)); + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); ASSERT_TRUE(fs::remove(ros_log_dir)); ASSERT_TRUE(fs::remove(ros_dir)); @@ -133,14 +133,14 @@ TEST_F(LoggingTest, init_failure) TEST_F(LoggingTest, full_cycle) { - ASSERT_EQ(0, rcl_logging_external_initialize(nullptr, allocator)); + ASSERT_EQ(RCL_LOGGING_RET_OK, rcl_logging_external_initialize(nullptr, allocator)); // Make sure we can call initialize more than once - ASSERT_EQ(0, rcl_logging_external_initialize(nullptr, allocator)); + ASSERT_EQ(RCL_LOGGING_RET_OK, rcl_logging_external_initialize(nullptr, allocator)); std::stringstream expected_log; for (int level : logger_levels) { - EXPECT_EQ(0, rcl_logging_external_set_logger_level(nullptr, level)); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_external_set_logger_level(nullptr, level)); for (int severity : logger_levels) { std::stringstream ss; @@ -156,7 +156,7 @@ TEST_F(LoggingTest, full_cycle) } } - EXPECT_EQ(0, rcl_logging_external_shutdown()); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_external_shutdown()); std::string log_file_path = find_single_log().string(); std::ifstream log_file(log_file_path); From df294d7bfdfbca6499076f41cdeaa722fb7fcc6e Mon Sep 17 00:00:00 2001 From: tgreier Date: Mon, 21 Dec 2020 14:02:58 -0500 Subject: [PATCH 2/4] Allow configuring logging directory through environment variables (#53) Signed-off-by: Christophe Bedard Signed-off-by: tgreier --- rcl_logging_interface/CMakeLists.txt | 34 +++- .../rcl_logging_interface.h | 25 +++ rcl_logging_interface/package.xml | 4 + rcl_logging_interface/src/logging_dir.c | 107 ++++++++++ .../test/test_get_logging_directory.cpp | 183 ++++++++++++++++++ .../rcl_logging_log4cxx.cpp | 19 +- rcl_logging_spdlog/CMakeLists.txt | 3 +- rcl_logging_spdlog/package.xml | 2 +- rcl_logging_spdlog/src/rcl_logging_spdlog.cpp | 51 ++--- 9 files changed, 380 insertions(+), 48 deletions(-) create mode 100644 rcl_logging_interface/src/logging_dir.c create mode 100644 rcl_logging_interface/test/test_get_logging_directory.cpp diff --git a/rcl_logging_interface/CMakeLists.txt b/rcl_logging_interface/CMakeLists.txt index 1dd9204..b1002a6 100644 --- a/rcl_logging_interface/CMakeLists.txt +++ b/rcl_logging_interface/CMakeLists.txt @@ -18,21 +18,41 @@ endif() find_package(ament_cmake_ros REQUIRED) find_package(rcutils REQUIRED) -add_library(${PROJECT_NAME} INTERFACE) -target_include_directories(${PROJECT_NAME} INTERFACE +set(${PROJECT_NAME}_sources + "src/logging_dir.c" +) +add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources}) +target_include_directories(${PROJECT_NAME} PUBLIC + "$" "$") -ament_target_dependencies(${PROJECT_NAME} INTERFACE rcutils) +ament_target_dependencies(${PROJECT_NAME} rcutils) +target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_LOGGING_INTERFACE_BUILDING_DLL") +install( + DIRECTORY include/ + DESTINATION include +) install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib RUNTIME DESTINATION bin) -install(DIRECTORY include/${PROJECT_NAME}/ - DESTINATION include/${PROJECT_NAME} -) - ament_export_include_directories(include) ament_export_targets(${PROJECT_NAME}) ament_export_dependencies(rcutils) + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() + + find_package(ament_cmake_gtest REQUIRED) + find_package(rcpputils REQUIRED) + ament_add_gtest(test_get_logging_directory test/test_get_logging_directory.cpp) + if(TARGET test_get_logging_directory) + target_link_libraries(test_get_logging_directory ${PROJECT_NAME}) + target_include_directories(test_get_logging_directory PRIVATE include) + ament_target_dependencies(test_get_logging_directory rcutils rcpputils) + endif() +endif() + ament_package() diff --git a/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h b/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h index 235942f..074f86a 100644 --- a/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h +++ b/rcl_logging_interface/include/rcl_logging_interface/rcl_logging_interface.h @@ -26,6 +26,7 @@ typedef enum { RCL_LOGGING_RET_OK = 0, RCL_LOGGING_RET_ERROR = 2, + RCL_LOGGING_RET_INVALID_ARGUMENT = 11, RCL_LOGGING_RET_CONFIG_FILE_DOESNT_EXIST = 21, RCL_LOGGING_RET_CONFIG_FILE_INVALID = 22, } rcl_logging_ret_t; @@ -91,6 +92,30 @@ RCL_LOGGING_INTERFACE_PUBLIC RCUTILS_WARN_UNUSED rcl_logging_ret_t rcl_logging_external_set_logger_level(const char * name, int level); +/// Get the logging directory. +/** + * Uses various environment variables to construct a logging directory path. + * + * Use $ROS_LOG_DIR if ROS_LOG_DIR is set and not empty. + * Otherwise, use $ROS_HOME/log, using ~/.ros for ROS_HOME if not set or if empty. + * + * It also expands an initial '~' to the current user's home directory, + * and converts the path separator if necessary. + * + * If successful, the directory C string should be deallocated using the given allocator when it is + * no longer needed. + * + * \param[in] allocator The allocator to use for memory allocation. + * \param[out] directory The C string pointer at which to write the directory path. + * Only meaningful if the call is successful. Must not be nullptr and must point to nullptr. + * \return RCL_LOGGING_RET_OK if successful, or + * \return RCL_LOGGING_RET_INVALID_ARGUMENT if any arguments are invalid, or + * \return RCL_LOGGING_RET_ERROR if an unspecified error occurs. + */ +RCL_LOGGING_INTERFACE_PUBLIC +rcl_logging_ret_t +rcl_logging_get_logging_directory(rcutils_allocator_t allocator, char ** directory); + #ifdef __cplusplus } #endif diff --git a/rcl_logging_interface/package.xml b/rcl_logging_interface/package.xml index 8522a32..eb3a058 100644 --- a/rcl_logging_interface/package.xml +++ b/rcl_logging_interface/package.xml @@ -12,6 +12,10 @@ rcutils + ament_lint_auto + ament_lint_common + rcpputils + ament_cmake diff --git a/rcl_logging_interface/src/logging_dir.c b/rcl_logging_interface/src/logging_dir.c new file mode 100644 index 0000000..23d0f03 --- /dev/null +++ b/rcl_logging_interface/src/logging_dir.c @@ -0,0 +1,107 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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 +#include +#include +#include + +#include "rcl_logging_interface/rcl_logging_interface.h" + +rcl_logging_ret_t +rcl_logging_get_logging_directory(rcutils_allocator_t allocator, char ** directory) +{ + if (NULL == directory) { + RCUTILS_SET_ERROR_MSG("directory argument must not be null"); + return RCL_LOGGING_RET_INVALID_ARGUMENT; + } + if (NULL != *directory) { + RCUTILS_SET_ERROR_MSG("directory argument must point to null"); + return RCL_LOGGING_RET_INVALID_ARGUMENT; + } + + const char * log_dir_env; + const char * err = rcutils_get_env("ROS_LOG_DIR", &log_dir_env); + if (NULL != err) { + RCUTILS_SET_ERROR_MSG("rcutils_get_env failed"); + return RCL_LOGGING_RET_ERROR; + } + if ('\0' != *log_dir_env) { + *directory = rcutils_strdup(log_dir_env, allocator); + if (NULL == *directory) { + RCUTILS_SET_ERROR_MSG("rcutils_strdup failed"); + return RCL_LOGGING_RET_ERROR; + } + } else { + const char * ros_home_dir_env; + err = rcutils_get_env("ROS_HOME", &ros_home_dir_env); + if (NULL != err) { + RCUTILS_SET_ERROR_MSG("rcutils_get_env failed"); + return RCL_LOGGING_RET_ERROR; + } + char * ros_home_dir; + if ('\0' == *ros_home_dir_env) { + ros_home_dir = rcutils_join_path("~", ".ros", allocator); + if (NULL == ros_home_dir) { + RCUTILS_SET_ERROR_MSG("rcutils_join_path failed"); + return RCL_LOGGING_RET_ERROR; + } + } else { + ros_home_dir = rcutils_strdup(ros_home_dir_env, allocator); + if (NULL == ros_home_dir) { + RCUTILS_SET_ERROR_MSG("rcutils_strdup failed"); + return RCL_LOGGING_RET_ERROR; + } + } + *directory = rcutils_join_path(ros_home_dir, "log", allocator); + allocator.deallocate(ros_home_dir, allocator.state); + if (NULL == *directory) { + RCUTILS_SET_ERROR_MSG("rcutils_join_path failed"); + return RCL_LOGGING_RET_ERROR; + } + } + + // Expand home directory + if ('~' == (*directory)[0]) { + const char * homedir = rcutils_get_home_dir(); + if (NULL == homedir) { + allocator.deallocate(*directory, allocator.state); + RCUTILS_SET_ERROR_MSG("failed to get the home directory"); + return RCL_LOGGING_RET_ERROR; + } + char * directory_not_expanded = *directory; + *directory = rcutils_format_string_limit( + allocator, + strlen(homedir) + strlen(directory_not_expanded), + "%s%s", + homedir, + directory_not_expanded + 1); + allocator.deallocate(directory_not_expanded, allocator.state); + if (NULL == *directory) { + RCUTILS_SET_ERROR_MSG("rcutils_format_string failed"); + return RCL_LOGGING_RET_ERROR; + } + } + + char * directory_maybe_not_native = *directory; + *directory = rcutils_to_native_path(directory_maybe_not_native, allocator); + allocator.deallocate(directory_maybe_not_native, allocator.state); + if (NULL == *directory) { + RCUTILS_SET_ERROR_MSG("rcutils_to_native_path failed"); + return RCL_LOGGING_RET_ERROR; + } + return RCL_LOGGING_RET_OK; +} diff --git a/rcl_logging_interface/test/test_get_logging_directory.cpp b/rcl_logging_interface/test/test_get_logging_directory.cpp new file mode 100644 index 0000000..2c9ed8e --- /dev/null +++ b/rcl_logging_interface/test/test_get_logging_directory.cpp @@ -0,0 +1,183 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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 +#include +#include + +#include +#include + +#include "gtest/gtest.h" +#include "rcl_logging_interface/rcl_logging_interface.h" + +// This is a helper class that resets an environment +// variable when leaving scope +class RestoreEnvVar +{ +public: + explicit RestoreEnvVar(const std::string & name) + : name_(name), + value_(rcpputils::get_env_var(name.c_str())) + { + } + + ~RestoreEnvVar() + { + if (!rcutils_set_env(name_.c_str(), value_.c_str())) { + std::cerr << "Failed to restore value of environment variable: " << name_ << std::endl; + } + } + +private: + const std::string name_; + const std::string value_; +}; + +TEST(test_logging_directory, directory) +{ + RestoreEnvVar home_var("HOME"); + RestoreEnvVar userprofile_var("USERPROFILE"); + ASSERT_EQ(true, rcutils_set_env("HOME", nullptr)); + ASSERT_EQ(true, rcutils_set_env("USERPROFILE", nullptr)); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr)); + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); + + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + + // Invalid argument if given a nullptr + EXPECT_EQ( + RCL_LOGGING_RET_INVALID_ARGUMENT, rcl_logging_get_logging_directory(allocator, nullptr)); + EXPECT_TRUE(rcutils_error_is_set()); + rcutils_reset_error(); + // Invalid argument if the C string is not nullptr + char * could_leak = const_cast("/could/be/leaked"); + EXPECT_EQ( + RCL_LOGGING_RET_INVALID_ARGUMENT, rcl_logging_get_logging_directory(allocator, &could_leak)); + EXPECT_TRUE(rcutils_error_is_set()); + rcutils_reset_error(); + + // Fails without any relevant env vars at all (HOME included) + char * directory = nullptr; + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_TRUE(rcutils_error_is_set()); + rcutils_reset_error(); + directory = nullptr; + + // Default case without ROS_LOG_DIR or ROS_HOME being set (but with HOME) + rcpputils::fs::path fake_home("/fake_home_dir"); + ASSERT_EQ(true, rcutils_set_env("HOME", fake_home.string().c_str())); + ASSERT_EQ(true, rcutils_set_env("USERPROFILE", fake_home.string().c_str())); + rcpputils::fs::path default_dir = fake_home / ".ros" / "log"; + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + + // Use $ROS_LOG_DIR if it is set + const char * my_log_dir_raw = "/my/ros_log_dir"; + rcpputils::fs::path my_log_dir(my_log_dir_raw); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", my_log_dir.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure it converts path separators when necessary + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", my_log_dir_raw)); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Setting ROS_HOME won't change anything since ROS_LOG_DIR is used first + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "/this/wont/be/used")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); + // Empty is considered unset + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure '~' is expanded to the home directory + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~/logdir")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path fake_log_dir = fake_home / "logdir"; + EXPECT_STREQ(directory, fake_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // But it should only be expanded if it's at the beginning + rcpputils::fs::path prefixed_fake_log_dir("/prefix/~/logdir"); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", prefixed_fake_log_dir.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, prefixed_fake_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, fake_home.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + rcpputils::fs::path home_trailing_slash(fake_home.string() + "/"); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~/")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, home_trailing_slash.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr)); + + // Without ROS_LOG_DIR, use $ROS_HOME/log + rcpputils::fs::path fake_ros_home = fake_home / ".fakeroshome"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", fake_ros_home.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path fake_ros_home_log_dir = fake_ros_home / "log"; + EXPECT_STREQ(directory, fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure it converts path separators when necessary + const char * my_ros_home_raw = "/my/ros/home"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", my_ros_home_raw)); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path my_ros_home_log_dir = rcpputils::fs::path(my_ros_home_raw) / "log"; + EXPECT_STREQ(directory, my_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Empty is considered unset + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure '~' is expanded to the home directory + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "~/.fakeroshome")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // But it should only be expanded if it's at the beginning + rcpputils::fs::path prefixed_fake_ros_home("/prefix/~/.fakeroshome"); + rcpputils::fs::path prefixed_fake_ros_home_log_dir = prefixed_fake_ros_home / "log"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", prefixed_fake_ros_home.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, prefixed_fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); +} diff --git a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp index 3adf80c..11a8afb 100644 --- a/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp +++ b/rcl_logging_log4cxx/src/rcl_logging_log4cxx/rcl_logging_log4cxx.cpp @@ -102,19 +102,20 @@ rcl_logging_ret_t rcl_logging_external_initialize( // To be compatible with ROS 1, we want to construct a default filename of // the form ~/.ros/log/__.log - // First get the home directory. - const char * homedir = rcutils_get_home_dir(); - if (homedir == nullptr) { - // We couldn't get the home directory; it is not really going to be - // possible to do logging properly, so get out of here without setting - // up logging. - return RCL_LOGGING_RET_ERROR; + char * logdir = nullptr; + rcl_logging_ret_t dir_ret = rcl_logging_get_logging_directory(allocator, &logdir); + if (RCL_LOGGING_RET_OK != dir_ret) { + // We couldn't get the log directory, so get out of here without setting up + // logging. + RCUTILS_SET_ERROR_MSG("Failed to get logging directory"); + return dir_ret; } // Now get the milliseconds since the epoch in the local timezone. rcutils_time_point_value_t now; rcutils_ret_t ret = rcutils_system_time_now(&now); if (ret != RCUTILS_RET_OK) { + allocator.deallocate(logdir, allocator.state); // We couldn't get the system time, so get out of here without setting up // logging. return RCL_LOGGING_RET_ERROR; @@ -124,6 +125,7 @@ rcl_logging_ret_t rcl_logging_external_initialize( // Get the program name. char * executable_name = rcutils_get_executable_name(allocator); if (executable_name == nullptr) { + allocator.deallocate(logdir, allocator.state); // We couldn't get the program name, so get out of here without setting up // logging. return RCL_LOGGING_RET_ERROR; @@ -132,8 +134,9 @@ rcl_logging_ret_t rcl_logging_external_initialize( char log_name_buffer[512] = {0}; int print_ret = rcutils_snprintf( log_name_buffer, sizeof(log_name_buffer), - "%s/.ros/log/%s_%i_%" PRId64 ".log", homedir, executable_name, + "%s/%s_%i_%" PRId64 ".log", logdir, executable_name, rcutils_get_pid(), ms_since_epoch); + allocator.deallocate(logdir, allocator.state); allocator.deallocate(executable_name, allocator.state); if (print_ret < 0) { RCUTILS_SET_ERROR_MSG("Failed to create log file name string"); diff --git a/rcl_logging_spdlog/CMakeLists.txt b/rcl_logging_spdlog/CMakeLists.txt index 0ae2837..46cfc8b 100644 --- a/rcl_logging_spdlog/CMakeLists.txt +++ b/rcl_logging_spdlog/CMakeLists.txt @@ -13,6 +13,7 @@ endif() find_package(ament_cmake_ros REQUIRED) find_package(rcl_logging_interface REQUIRED) +find_package(rcpputils REQUIRED) find_package(rcutils REQUIRED) find_package(spdlog_vendor REQUIRED) # Provides spdlog on platforms without it. find_package(spdlog REQUIRED) @@ -26,6 +27,7 @@ target_link_libraries(${PROJECT_NAME} spdlog::spdlog) ament_target_dependencies(${PROJECT_NAME} rcl_logging_interface + rcpputils rcutils spdlog ) @@ -47,7 +49,6 @@ if(BUILD_TESTING) performance_test_fixture::performance_test_fixture INTERFACE_INCLUDE_DIRECTORIES) find_package(ament_cmake_gtest REQUIRED) - find_package(rcpputils REQUIRED) ament_add_gtest(test_logging_interface test/test_logging_interface.cpp) if(TARGET test_logging_interface) target_link_libraries(test_logging_interface ${PROJECT_NAME}) diff --git a/rcl_logging_spdlog/package.xml b/rcl_logging_spdlog/package.xml index 184f739..c2e50f6 100644 --- a/rcl_logging_spdlog/package.xml +++ b/rcl_logging_spdlog/package.xml @@ -14,6 +14,7 @@ spdlog rcl_logging_interface + rcpputils rcutils spdlog_vendor @@ -22,7 +23,6 @@ ament_lint_auto ament_lint_common performance_test_fixture - rcpputils rcl_logging_packages diff --git a/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp b/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp index daa4cc3..ba400db 100644 --- a/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp +++ b/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include -#include #include #include #include @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "spdlog/spdlog.h" @@ -75,36 +76,20 @@ rcl_logging_ret_t rcl_logging_external_initialize( // To be compatible with ROS 1, we construct a default filename of // the form ~/.ros/log/__.log - // First get the home directory. - const char * homedir = rcutils_get_home_dir(); - if (homedir == nullptr) { - // We couldn't get the home directory; it is not really going to be - // possible to do logging properly, so get out of here without setting - // up logging. - RCUTILS_SET_ERROR_MSG("Failed to get users home directory"); - return RCL_LOGGING_RET_ERROR; - } - - // SPDLOG doesn't automatically create the log directories, so make them - // by hand here. - char name_buffer[4096] = {0}; - int print_ret = rcutils_snprintf(name_buffer, sizeof(name_buffer), "%s/.ros", homedir); - if (print_ret < 0) { - RCUTILS_SET_ERROR_MSG("Failed to create home directory string"); - return RCL_LOGGING_RET_ERROR; - } - if (!rcutils_mkdir(name_buffer)) { - RCUTILS_SET_ERROR_MSG("Failed to create user .ros directory"); - return RCL_LOGGING_RET_ERROR; + char * logdir = nullptr; + rcl_logging_ret_t dir_ret = rcl_logging_get_logging_directory(allocator, &logdir); + if (RCL_LOGGING_RET_OK != dir_ret) { + // We couldn't get the log directory, so get out of here without setting up + // logging. + RCUTILS_SET_ERROR_MSG("Failed to get logging directory"); + return dir_ret; } - print_ret = rcutils_snprintf(name_buffer, sizeof(name_buffer), "%s/.ros/log", homedir); - if (print_ret < 0) { - RCUTILS_SET_ERROR_MSG("Failed to create log directory string"); - return RCL_LOGGING_RET_ERROR; - } - if (!rcutils_mkdir(name_buffer)) { - RCUTILS_SET_ERROR_MSG("Failed to create user log directory"); + // SPDLOG doesn't automatically create the log directories, so create them + rcpputils::fs::path logdir_path(logdir); + if (!rcpputils::fs::create_directories(logdir_path)) { + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("Failed to create log directory: %s", logdir); + allocator.deallocate(logdir, allocator.state); return RCL_LOGGING_RET_ERROR; } @@ -112,6 +97,7 @@ rcl_logging_ret_t rcl_logging_external_initialize( rcutils_time_point_value_t now; rcutils_ret_t ret = rcutils_system_time_now(&now); if (ret != RCUTILS_RET_OK) { + allocator.deallocate(logdir, allocator.state); // We couldn't get the system time, so get out of here without setting up // logging. We don't need to call RCUTILS_SET_ERROR_MSG either since // rcutils_system_time_now() already did it. @@ -122,16 +108,19 @@ rcl_logging_ret_t rcl_logging_external_initialize( // Get the program name. char * basec = rcutils_get_executable_name(allocator); if (basec == nullptr) { + allocator.deallocate(logdir, allocator.state); // We couldn't get the program name, so get out of here without setting up // logging. RCUTILS_SET_ERROR_MSG("Failed to get the executable name"); return RCL_LOGGING_RET_ERROR; } - print_ret = rcutils_snprintf( + char name_buffer[4096] = {0}; + int print_ret = rcutils_snprintf( name_buffer, sizeof(name_buffer), - "%s/.ros/log/%s_%i_%" PRId64 ".log", homedir, + "%s/%s_%i_%" PRId64 ".log", logdir, basec, rcutils_get_pid(), ms_since_epoch); + allocator.deallocate(logdir, allocator.state); allocator.deallocate(basec, allocator.state); if (print_ret < 0) { RCUTILS_SET_ERROR_MSG("Failed to create log file name string"); From c7352e40516b47e878f2dd23862e1e2d725e12d6 Mon Sep 17 00:00:00 2001 From: tgreier Date: Mon, 21 Dec 2020 14:03:43 -0500 Subject: [PATCH 3/4] Used current_path() function from rcpputils (#51) * Used current_path function from rcpputils Signed-off-by: ahcorde * Removed fs namespace Signed-off-by: ahcorde Signed-off-by: tgreier --- .../test/test_logging_interface.cpp | 44 ++++--------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/rcl_logging_spdlog/test/test_logging_interface.cpp b/rcl_logging_spdlog/test/test_logging_interface.cpp index 0773490..6f9021a 100644 --- a/rcl_logging_spdlog/test/test_logging_interface.cpp +++ b/rcl_logging_spdlog/test/test_logging_interface.cpp @@ -28,11 +28,6 @@ #include "gtest/gtest.h" #include "rcl_logging_interface/rcl_logging_interface.h" -#define RCL_LOGGING_RET_OK (0) -#define RCL_LOGGING_RET_ERROR (2) - -namespace fs = rcpputils::fs; - const int logger_levels[] = { RCUTILS_LOG_SEVERITY_UNSET, @@ -66,27 +61,6 @@ class RestoreEnvVar const std::string value_; }; -// TODO(cottsay): Remove when ros2/rcpputils#63 is resolved -static fs::path current_path() -{ -#ifdef _WIN32 -#ifdef UNICODE -#error "rcpputils::fs does not support Unicode paths" -#endif - char cwd[MAX_PATH]; - if (nullptr == _getcwd(cwd, MAX_PATH)) { -#else - char cwd[PATH_MAX]; - if (nullptr == getcwd(cwd, PATH_MAX)) { -#endif - std::error_code ec{errno, std::system_category()}; - errno = 0; - throw std::system_error{ec, "cannot get current working directory"}; - } - - return fs::path(cwd); -} - TEST_F(LoggingTest, init_invalid) { // Config files are not supported by spdlog @@ -110,25 +84,25 @@ TEST_F(LoggingTest, init_failure) rcutils_reset_error(); // Force failure to create directories - fs::path fake_home = current_path() / "fake_home_dir"; - ASSERT_TRUE(fs::create_directories(fake_home)); + rcpputils::fs::path fake_home = rcpputils::fs::current_path() / "fake_home_dir"; + ASSERT_TRUE(rcpputils::fs::create_directories(fake_home)); ASSERT_EQ(true, rcutils_set_env("HOME", fake_home.string().c_str())); // ...fail to create .ros dir - fs::path ros_dir = fake_home / ".ros"; + rcpputils::fs::path ros_dir = fake_home / ".ros"; std::fstream(ros_dir.string(), std::ios_base::out).close(); EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); - ASSERT_TRUE(fs::remove(ros_dir)); + ASSERT_TRUE(rcpputils::fs::remove(ros_dir)); // ...fail to create .ros/log dir - ASSERT_TRUE(fs::create_directories(ros_dir)); - fs::path ros_log_dir = ros_dir / "log"; + ASSERT_TRUE(rcpputils::fs::create_directories(ros_dir)); + rcpputils::fs::path ros_log_dir = ros_dir / "log"; std::fstream(ros_log_dir.string(), std::ios_base::out).close(); EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); - ASSERT_TRUE(fs::remove(ros_log_dir)); - ASSERT_TRUE(fs::remove(ros_dir)); + ASSERT_TRUE(rcpputils::fs::remove(ros_log_dir)); + ASSERT_TRUE(rcpputils::fs::remove(ros_dir)); - ASSERT_TRUE(fs::remove(fake_home)); + ASSERT_TRUE(rcpputils::fs::remove(fake_home)); } TEST_F(LoggingTest, full_cycle) From b52ac1932bb107808b178326bac017651a005920 Mon Sep 17 00:00:00 2001 From: tgreier Date: Mon, 21 Dec 2020 15:23:08 -0500 Subject: [PATCH 4/4] Added benchmark test to rcl_logging_spdlog (#52) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alejandro Hernández Cordero Signed-off-by: tgreier --- rcl_logging_spdlog/package.xml | 1 + .../benchmark/benchmark_logging_interface.cpp | 3 - .../test/test_logging_interface.cpp | 144 +++++++++++++++--- 3 files changed, 124 insertions(+), 24 deletions(-) diff --git a/rcl_logging_spdlog/package.xml b/rcl_logging_spdlog/package.xml index c2e50f6..905e663 100644 --- a/rcl_logging_spdlog/package.xml +++ b/rcl_logging_spdlog/package.xml @@ -23,6 +23,7 @@ ament_lint_auto ament_lint_common performance_test_fixture + rcpputils rcl_logging_packages diff --git a/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp b/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp index 743b0bc..8abe0b4 100644 --- a/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp +++ b/rcl_logging_spdlog/test/benchmark/benchmark_logging_interface.cpp @@ -22,9 +22,6 @@ #include "performance_test_fixture/performance_test_fixture.hpp" -#define RCL_LOGGING_RET_OK (0) -#define RCL_LOGGING_RET_ERROR (2) - using performance_test_fixture::PerformanceTest; namespace diff --git a/rcl_logging_spdlog/test/test_logging_interface.cpp b/rcl_logging_spdlog/test/test_logging_interface.cpp index 6f9021a..f4dd53b 100644 --- a/rcl_logging_spdlog/test/test_logging_interface.cpp +++ b/rcl_logging_spdlog/test/test_logging_interface.cpp @@ -76,33 +76,135 @@ TEST_F(LoggingTest, init_failure) { RestoreEnvVar home_var("HOME"); RestoreEnvVar userprofile_var("USERPROFILE"); - - // No home directory to write log to ASSERT_EQ(true, rcutils_set_env("HOME", nullptr)); ASSERT_EQ(true, rcutils_set_env("USERPROFILE", nullptr)); - EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr)); + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); + + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + + // Invalid argument if given a nullptr + EXPECT_EQ( + RCL_LOGGING_RET_INVALID_ARGUMENT, rcl_logging_get_logging_directory(allocator, nullptr)); + EXPECT_TRUE(rcutils_error_is_set()); + rcutils_reset_error(); + // Invalid argument if the C string is not nullptr + char * could_leak = const_cast("/could/be/leaked"); + EXPECT_EQ( + RCL_LOGGING_RET_INVALID_ARGUMENT, rcl_logging_get_logging_directory(allocator, &could_leak)); + EXPECT_TRUE(rcutils_error_is_set()); rcutils_reset_error(); - // Force failure to create directories - rcpputils::fs::path fake_home = rcpputils::fs::current_path() / "fake_home_dir"; - ASSERT_TRUE(rcpputils::fs::create_directories(fake_home)); + // Fails without any relevant env vars at all (HOME included) + char * directory = nullptr; + EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_TRUE(rcutils_error_is_set()); + rcutils_reset_error(); + directory = nullptr; + + // Default case without ROS_LOG_DIR or ROS_HOME being set (but with HOME) + rcpputils::fs::path fake_home("/fake_home_dir"); ASSERT_EQ(true, rcutils_set_env("HOME", fake_home.string().c_str())); + ASSERT_EQ(true, rcutils_set_env("USERPROFILE", fake_home.string().c_str())); + rcpputils::fs::path default_dir = fake_home / ".ros" / "log"; + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + + // Use $ROS_LOG_DIR if it is set + const char * my_log_dir_raw = "/my/ros_log_dir"; + rcpputils::fs::path my_log_dir(my_log_dir_raw); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", my_log_dir.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure it converts path separators when necessary + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", my_log_dir_raw)); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Setting ROS_HOME won't change anything since ROS_LOG_DIR is used first + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "/this/wont/be/used")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, my_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); + // Empty is considered unset + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure '~' is expanded to the home directory + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~/logdir")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path fake_log_dir = fake_home / "logdir"; + EXPECT_STREQ(directory, fake_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // But it should only be expanded if it's at the beginning + rcpputils::fs::path prefixed_fake_log_dir("/prefix/~/logdir"); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", prefixed_fake_log_dir.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, prefixed_fake_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, fake_home.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + rcpputils::fs::path home_trailing_slash(fake_home.string() + "/"); + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", "~/")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, home_trailing_slash.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + + ASSERT_EQ(true, rcutils_set_env("ROS_LOG_DIR", nullptr)); + + // Without ROS_LOG_DIR, use $ROS_HOME/log + rcpputils::fs::path fake_ros_home = fake_home / ".fakeroshome"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", fake_ros_home.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path fake_ros_home_log_dir = fake_ros_home / "log"; + EXPECT_STREQ(directory, fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure it converts path separators when necessary + const char * my_ros_home_raw = "/my/ros/home"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", my_ros_home_raw)); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + rcpputils::fs::path my_ros_home_log_dir = rcpputils::fs::path(my_ros_home_raw) / "log"; + EXPECT_STREQ(directory, my_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Empty is considered unset + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, default_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // Make sure '~' is expanded to the home directory + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", "~/.fakeroshome")); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; + // But it should only be expanded if it's at the beginning + rcpputils::fs::path prefixed_fake_ros_home("/prefix/~/.fakeroshome"); + rcpputils::fs::path prefixed_fake_ros_home_log_dir = prefixed_fake_ros_home / "log"; + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", prefixed_fake_ros_home.string().c_str())); + EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_get_logging_directory(allocator, &directory)); + EXPECT_STREQ(directory, prefixed_fake_ros_home_log_dir.string().c_str()); + allocator.deallocate(directory, allocator.state); + directory = nullptr; - // ...fail to create .ros dir - rcpputils::fs::path ros_dir = fake_home / ".ros"; - std::fstream(ros_dir.string(), std::ios_base::out).close(); - EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); - ASSERT_TRUE(rcpputils::fs::remove(ros_dir)); - - // ...fail to create .ros/log dir - ASSERT_TRUE(rcpputils::fs::create_directories(ros_dir)); - rcpputils::fs::path ros_log_dir = ros_dir / "log"; - std::fstream(ros_log_dir.string(), std::ios_base::out).close(); - EXPECT_EQ(RCL_LOGGING_RET_ERROR, rcl_logging_external_initialize(nullptr, allocator)); - ASSERT_TRUE(rcpputils::fs::remove(ros_log_dir)); - ASSERT_TRUE(rcpputils::fs::remove(ros_dir)); - - ASSERT_TRUE(rcpputils::fs::remove(fake_home)); + ASSERT_EQ(true, rcutils_set_env("ROS_HOME", nullptr)); } TEST_F(LoggingTest, full_cycle)