-
Notifications
You must be signed in to change notification settings - Fork 41
Allow configuring logging directory through environment variables #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ivanpauno
merged 30 commits into
ros2:master
from
christophebedard:allow-configuring-logging-dir-through-env-vars
Oct 13, 2020
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
07dfba4
Allow configuring logging directory through environment variables
christophebedard b6013cb
Re-order ament_export_*() lines
christophebedard 0705dd3
Fix function signature/param order and update doc
christophebedard ba70da8
Remove comment
christophebedard cd68e6b
Cleanup path init
christophebedard 3d5bfb3
Validate directory argument, set error msg on error and update doc
christophebedard b50d081
Fix leaks
christophebedard a97d633
Use 'nullptr' instead of 'null' in doc
christophebedard 2a519a8
Switch to passing allocator by value
christophebedard 2db8e30
Fix signature
christophebedard 653e38c
Update tests
christophebedard 71bfafc
Fix and document ownership of returned pointer
christophebedard 4114f12
Return error if rcutils_get_env fails
christophebedard 269e901
Move literals to the left hand side for conditions
christophebedard 18b8448
Use ROS_* env vars instead of ROS2_*
christophebedard 35cc88a
Turn into lib and extract implementation
christophebedard a885295
Add test that sets both env vars
christophebedard 47bc3e4
Return dir_ret on failure directly
christophebedard de0667c
Remove unnecessary #ifdef __cplusplus in .c file
christophebedard d0ebff7
Only expand '~' if it is at the beginning
christophebedard c2735e6
Simplify comparison
christophebedard 4576b12
Handle home directory expansion better
christophebedard 93ad695
Fix and simplify home directory expansion
christophebedard 49fdcd8
Switch to rcutils_format_string_limit
christophebedard 0dd91f4
Fix memory leak with home directory env var
christophebedard c8aae9e
Convert path separators in env var values if necessary
christophebedard 1a4f0e0
Fix visibility control for Windows
christophebedard 49e71a0
Include iostream for test
christophebedard 8e6661a
Set error message on failure in init functions
christophebedard 97f6824
Fix more leaks
christophebedard File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <rcutils/allocator.h> | ||
| #include <rcutils/error_handling.h> | ||
| #include <rcutils/filesystem.h> | ||
| #include <rcutils/format_string.h> | ||
| #include <rcutils/get_env.h> | ||
| #include <rcutils/strdup.h> | ||
|
|
||
| #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"); | ||
clalancette marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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"); | ||
clalancette marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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; | ||
| } | ||
183 changes: 183 additions & 0 deletions
183
rcl_logging_interface/test/test_get_logging_directory.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <rcpputils/filesystem_helper.hpp> | ||
| #include <rcpputils/get_env.hpp> | ||
| #include <rcutils/allocator.h> | ||
| #include <rcutils/env.h> | ||
| #include <rcutils/error_handling.h> | ||
|
|
||
| #include <iostream> | ||
| #include <string> | ||
|
|
||
| #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; | ||
clalancette marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| 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<char *>("/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()); | ||
wjwwood marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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)); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.