From e561a302a021e08d84d029e4c11b888a5d6840b4 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 14 May 2020 10:55:01 -0700 Subject: [PATCH 1/5] Explicitly add DLL directories for Windows before importing New in Python 3.8, we should call os.add_dll_directory for directories containing the DLLs we intend to import as well as their recursive dependencies. Signed-off-by: Jacob Perron --- rclpy/rclpy/impl/__init__.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/rclpy/rclpy/impl/__init__.py b/rclpy/rclpy/impl/__init__.py index 92926f7e6..5a325904c 100644 --- a/rclpy/rclpy/impl/__init__.py +++ b/rclpy/rclpy/impl/__init__.py @@ -17,8 +17,20 @@ def _import(name): + dll_dir_handles = [] try: - return importlib.import_module(name, package='rclpy') + # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories + # containing DLLs we depend on. + # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew + if os.name == 'nt': + path_env = os.environ['PATH'].split(';') + for prefix_path in path_env: + if os.path.exists(prefix_path): + dll_dir_handles.append(os.add_dll_directory(prefix_path)) + + imported_module = importlib.import_module(name, package='rclpy') + + return imported_module except ImportError as e: if e.path is not None and os.path.isfile(e.path): e.msg += \ @@ -27,3 +39,6 @@ def _import(name): (e.path, 'https://index.ros.org/doc/ros2/Troubleshooting/' '#import-failing-even-with-library-present-on-the-system') raise + finally: + for handle in dll_dir_handles: + handle.close() From 241a02c98fb18d54bfec6f08f004274cd52c253c Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 14 May 2020 13:40:08 -0700 Subject: [PATCH 2/5] Refactor patch and add check for os.add_dll_directory Signed-off-by: Jacob Perron --- rclpy/rclpy/impl/__init__.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/rclpy/rclpy/impl/__init__.py b/rclpy/rclpy/impl/__init__.py index 5a325904c..4683418fd 100644 --- a/rclpy/rclpy/impl/__init__.py +++ b/rclpy/rclpy/impl/__init__.py @@ -17,20 +17,17 @@ def _import(name): + # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories + # containing DLLs we depend on. + # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew dll_dir_handles = [] + if os.name == 'nt' and hasattr(os, 'add_dll_directory'): + path_env = os.environ['PATH'].split(';') + for prefix_path in path_env: + if os.path.exists(prefix_path): + dll_dir_handles.append(os.add_dll_directory(prefix_path)) try: - # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories - # containing DLLs we depend on. - # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew - if os.name == 'nt': - path_env = os.environ['PATH'].split(';') - for prefix_path in path_env: - if os.path.exists(prefix_path): - dll_dir_handles.append(os.add_dll_directory(prefix_path)) - - imported_module = importlib.import_module(name, package='rclpy') - - return imported_module + return importlib.import_module(name, package='rclpy') except ImportError as e: if e.path is not None and os.path.isfile(e.path): e.msg += \ From 4f1676cc836bb08db14b3dcb841ee9490fa0527b Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Thu, 14 May 2020 14:46:48 -0700 Subject: [PATCH 3/5] Make same patch to test_rclpy module import Signed-off-by: Jacob Perron --- rclpy/test/__init__.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/rclpy/test/__init__.py b/rclpy/test/__init__.py index 8f1a3098a..78f41c48a 100644 --- a/rclpy/test/__init__.py +++ b/rclpy/test/__init__.py @@ -13,6 +13,7 @@ # limitations under the License. import importlib +import os import sys assert 'rclpy' not in sys.modules, 'rclpy should not have been imported before running tests' @@ -23,7 +24,19 @@ def _custom_import(name): - return importlib.import_module(name, package='test_rclpy') + # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories + # containing DLLs we depend on. + # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew + dll_dir_handles = [] + if os.name == 'nt' and hasattr(os, 'add_dll_directory'): + path_env = os.environ['PATH'].split(';') + for prefix_path in path_env: + if os.path.exists(prefix_path): + dll_dir_handles.append(os.add_dll_directory(prefix_path)) + import_module = importlib.import_module(name, package='test_rclpy') + for handle in dll_dir_handles: + handle.close() + return import_module rclpy.impl._import = _custom_import From a7953043e9f832b69a1d0b77d0aebe27189fb41f Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 15 May 2020 16:54:45 -0700 Subject: [PATCH 4/5] Move logic to common package rpyutils Signed-off-by: Jacob Perron --- rclpy/package.xml | 1 + rclpy/rclpy/impl/__init__.py | 20 +++++++------------- rclpy/test/__init__.py | 21 +++++++-------------- 3 files changed, 15 insertions(+), 27 deletions(-) diff --git a/rclpy/package.xml b/rclpy/package.xml index 19e731d18..ca8356543 100644 --- a/rclpy/package.xml +++ b/rclpy/package.xml @@ -26,6 +26,7 @@ ament_index_python builtin_interfaces rcl_interfaces + rpyutils rosgraph_msgs ament_cmake_gtest diff --git a/rclpy/rclpy/impl/__init__.py b/rclpy/rclpy/impl/__init__.py index 4683418fd..8fa489133 100644 --- a/rclpy/rclpy/impl/__init__.py +++ b/rclpy/rclpy/impl/__init__.py @@ -15,19 +15,16 @@ import importlib import os +from rpyutils import add_dll_directories_from_env + def _import(name): - # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories - # containing DLLs we depend on. - # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew - dll_dir_handles = [] - if os.name == 'nt' and hasattr(os, 'add_dll_directory'): - path_env = os.environ['PATH'].split(';') - for prefix_path in path_env: - if os.path.exists(prefix_path): - dll_dir_handles.append(os.add_dll_directory(prefix_path)) try: - return importlib.import_module(name, package='rclpy') + # Since Python 3.8, on Windows we should ensure DLL directories are explicitly added + # to the search path. + # See https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew + with add_dll_directories_from_env('PATH'): + return importlib.import_module(name, package='rclpy') except ImportError as e: if e.path is not None and os.path.isfile(e.path): e.msg += \ @@ -36,6 +33,3 @@ def _import(name): (e.path, 'https://index.ros.org/doc/ros2/Troubleshooting/' '#import-failing-even-with-library-present-on-the-system') raise - finally: - for handle in dll_dir_handles: - handle.close() diff --git a/rclpy/test/__init__.py b/rclpy/test/__init__.py index 78f41c48a..d1a5be1e2 100644 --- a/rclpy/test/__init__.py +++ b/rclpy/test/__init__.py @@ -13,9 +13,10 @@ # limitations under the License. import importlib -import os import sys +from rpyutils import add_dll_directories_from_env + assert 'rclpy' not in sys.modules, 'rclpy should not have been imported before running tests' # this will make the extensions load from the build folder @@ -24,19 +25,11 @@ def _custom_import(name): - # New in Python 3.8: on Windows we should call 'add_dll_directory()' for directories - # containing DLLs we depend on. - # https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew - dll_dir_handles = [] - if os.name == 'nt' and hasattr(os, 'add_dll_directory'): - path_env = os.environ['PATH'].split(';') - for prefix_path in path_env: - if os.path.exists(prefix_path): - dll_dir_handles.append(os.add_dll_directory(prefix_path)) - import_module = importlib.import_module(name, package='test_rclpy') - for handle in dll_dir_handles: - handle.close() - return import_module + # Since Python 3.8, on Windows we should ensure DLL directories are explicitly added + # to the search path. + # See https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew + with add_dll_directories_from_env('PATH'): + return importlib.import_module(name, package='test_rclpy') rclpy.impl._import = _custom_import From 4f5cc275eba113f31d8782b81a468414bd4951a1 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 15 May 2020 17:09:52 -0700 Subject: [PATCH 5/5] alphabetize --- rclpy/package.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclpy/package.xml b/rclpy/package.xml index ca8356543..7b98159e7 100644 --- a/rclpy/package.xml +++ b/rclpy/package.xml @@ -26,8 +26,8 @@ ament_index_python builtin_interfaces rcl_interfaces - rpyutils rosgraph_msgs + rpyutils ament_cmake_gtest ament_cmake_pytest