From a45663c5fab7bd91282be631a56452e137040bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Nordmoen?= Date: Fri, 26 May 2017 13:09:00 +0200 Subject: [PATCH 1/2] Added logging output when `roslogging` cannot change permissions Added better differentiated logging output to `roslogging` so that problems with permission is made clear to the user. Accompanying test have also been added. --- tools/rosgraph/src/rosgraph/roslogging.py | 8 ++- .../rosgraph/test/test_logging_dir_root/empty | 0 tools/rosgraph/test/test_roslogging.py | 55 +++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 tools/rosgraph/test/test_logging_dir_root/empty create mode 100644 tools/rosgraph/test/test_roslogging.py diff --git a/tools/rosgraph/src/rosgraph/roslogging.py b/tools/rosgraph/src/rosgraph/roslogging.py index e4cfafef9b..841a7942f1 100644 --- a/tools/rosgraph/src/rosgraph/roslogging.py +++ b/tools/rosgraph/src/rosgraph/roslogging.py @@ -83,7 +83,13 @@ def configure_logging(logname, level=logging.INFO, filename=None, env=None): makedirs_with_parent_perms(logfile_dir) except OSError: # cannot print to screen because command-line tools with output use this - sys.stderr.write("WARNING: cannot create log directory [%s]. Please set %s to a writable location.\n"%(logfile_dir, ROS_LOG_DIR)) + if os.path.exists(logfile_dir): + # We successfully created the logging folder, but could not change + # permissions of the new folder to the same as the parent folder + sys.stderr.write("Could not change permissions for folder [{!s}], make sure that the parent folder has correct permissions.\n".format(logfile_dir)) + else: + # Could not create folder + sys.stderr.write("WARNING: cannot create log directory [%s]. Please set %s to a writable location.\n"%(logfile_dir, ROS_LOG_DIR)) return None elif os.path.isfile(logfile_dir): raise LoggingException("Cannot save log files: file [%s] is in the way"%logfile_dir) diff --git a/tools/rosgraph/test/test_logging_dir_root/empty b/tools/rosgraph/test/test_logging_dir_root/empty new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools/rosgraph/test/test_roslogging.py b/tools/rosgraph/test/test_roslogging.py new file mode 100644 index 0000000000..813f9d51c5 --- /dev/null +++ b/tools/rosgraph/test/test_roslogging.py @@ -0,0 +1,55 @@ +# Software License Agreement (BSD License) +# +# Copyright (c) 2009, Willow Garage, Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided +# with the distribution. +# * Neither the name of Willow Garage, Inc. nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import os + +from rosgraph import roslogging + +def test_configure_logging_makedirs_fails_wrong_grp(): + """ + Test proper error output if folder is wrong group. + + Test that proper error output is given if `makedirs_with_parent_perms` + fails when ownership of folder cannot be changed. + """ + test_folder = os.path.join( + os.path.dirname(os.path.abspath(__file__)), + "test_logging_dir_root") + test_log = os.path.join( + test_folder, + "test_subfolder", + "test.log") + test_env = {"ROS_LOG_DIR": test_folder} + log_filename = roslogging.configure_logging("test", filename=test_log, env=test_env) + assert log_filename is None + assert os.path.isdir(test_folder) + os.rmdir(os.path.join(test_folder, "test_subfolder")) From 2ea1826c80535d552b19f1d4cc5572b18b9f8e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Nordmoen?= Date: Wed, 31 May 2017 11:13:56 +0200 Subject: [PATCH 2/2] Removed testing, updated warning message and fixed formatting Removed testing since test folder should not be stored together with tests. Since testing group permission requires intervention outside the test harness the test it self is also removed. Updated the warning message to include `WARNING` and updated to using `%` formatting. --- tools/rosgraph/src/rosgraph/roslogging.py | 2 +- .../rosgraph/test/test_logging_dir_root/empty | 0 tools/rosgraph/test/test_roslogging.py | 55 ------------------- 3 files changed, 1 insertion(+), 56 deletions(-) delete mode 100644 tools/rosgraph/test/test_logging_dir_root/empty delete mode 100644 tools/rosgraph/test/test_roslogging.py diff --git a/tools/rosgraph/src/rosgraph/roslogging.py b/tools/rosgraph/src/rosgraph/roslogging.py index 841a7942f1..20c4cb789f 100644 --- a/tools/rosgraph/src/rosgraph/roslogging.py +++ b/tools/rosgraph/src/rosgraph/roslogging.py @@ -86,7 +86,7 @@ def configure_logging(logname, level=logging.INFO, filename=None, env=None): if os.path.exists(logfile_dir): # We successfully created the logging folder, but could not change # permissions of the new folder to the same as the parent folder - sys.stderr.write("Could not change permissions for folder [{!s}], make sure that the parent folder has correct permissions.\n".format(logfile_dir)) + sys.stderr.write("WARNING: Could not change permissions for folder [%s], make sure that the parent folder has correct permissions.\n"%logfile_dir) else: # Could not create folder sys.stderr.write("WARNING: cannot create log directory [%s]. Please set %s to a writable location.\n"%(logfile_dir, ROS_LOG_DIR)) diff --git a/tools/rosgraph/test/test_logging_dir_root/empty b/tools/rosgraph/test/test_logging_dir_root/empty deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tools/rosgraph/test/test_roslogging.py b/tools/rosgraph/test/test_roslogging.py deleted file mode 100644 index 813f9d51c5..0000000000 --- a/tools/rosgraph/test/test_roslogging.py +++ /dev/null @@ -1,55 +0,0 @@ -# Software License Agreement (BSD License) -# -# Copyright (c) 2009, Willow Garage, Inc. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions -# are met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following -# disclaimer in the documentation and/or other materials provided -# with the distribution. -# * Neither the name of Willow Garage, Inc. nor the names of its -# contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS -# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE -# COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, -# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, -# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; -# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT -# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN -# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -# POSSIBILITY OF SUCH DAMAGE. - -import os - -from rosgraph import roslogging - -def test_configure_logging_makedirs_fails_wrong_grp(): - """ - Test proper error output if folder is wrong group. - - Test that proper error output is given if `makedirs_with_parent_perms` - fails when ownership of folder cannot be changed. - """ - test_folder = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - "test_logging_dir_root") - test_log = os.path.join( - test_folder, - "test_subfolder", - "test.log") - test_env = {"ROS_LOG_DIR": test_folder} - log_filename = roslogging.configure_logging("test", filename=test_log, env=test_env) - assert log_filename is None - assert os.path.isdir(test_folder) - os.rmdir(os.path.join(test_folder, "test_subfolder"))