From d5ed1fd87966230c3f93f11305c75439d8b8c4e6 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 8 Jun 2018 11:52:32 +0200 Subject: [PATCH 1/4] Fix importing module from python file --- reframe/utility/__init__.py | 2 +- unittests/test_utility.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index d1f603ad63..e8a07d1a33 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -44,7 +44,7 @@ def import_module_from_file(filename): if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') - if filename.startswith('..'): + if filename.startswith('..') or os.path.isfile(filename): filename = os.path.abspath(filename) module_name = _get_module_name(filename) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 4cb2576c57..74af29abd5 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -294,6 +294,12 @@ def test_load_file_relative(self): self.assertEqual('reframe', module.__name__) self.assertIs(module, sys.modules.get('reframe')) + def test_load_python_file(self): + with os_ext.change_dir('reframe/utility'): + module = util.import_module_from_file('os_ext.py') + self.assertEqual('os_ext', module.__name__) + self.assertIs(module, sys.modules.get('os_ext')) + def test_load_twice(self): filename = os.path.abspath('reframe/__init__.py') module1 = util.import_module_from_file(filename) From ea8c3c49ba307db5c78ccc6612d01af70c7a98f7 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 8 Jun 2018 13:35:24 +0200 Subject: [PATCH 2/4] Use _do_import_module_from_file for files --- reframe/utility/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index e8a07d1a33..6bfa56a86a 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -44,11 +44,11 @@ def import_module_from_file(filename): if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') - if filename.startswith('..') or os.path.isfile(filename): + if filename.startswith('..'): filename = os.path.abspath(filename) module_name = _get_module_name(filename) - if os.path.isabs(filename): + if os.path.isabs(filename) or os.path.isfile(filename): return _do_import_module_from_file(filename, module_name) return importlib.import_module(module_name) From 7a19e7073f4775ad58378dd23bc84a984085fd39 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 8 Jun 2018 13:58:41 +0200 Subject: [PATCH 3/4] Fix bug --- reframe/utility/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 6bfa56a86a..822bad2817 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -48,7 +48,7 @@ def import_module_from_file(filename): filename = os.path.abspath(filename) module_name = _get_module_name(filename) - if os.path.isabs(filename) or os.path.isfile(filename): + if os.path.isabs(filename) or os.path.basename(filename) == filename: return _do_import_module_from_file(filename, module_name) return importlib.import_module(module_name) From 8a2aed86cfec3d8316688b5904efaf644e063449 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 8 Jun 2018 20:12:17 +0200 Subject: [PATCH 4/4] Fix import from file code. The current implementation first resolves the module file relative to ReFrame. If it is inside ReFrame's top-level dir, it uses the standard Python mechanism (from importlib) to import the module. If not it loads it directly from the file assigning it a module name derived from the file's basename. --- reframe/utility/__init__.py | 24 ++++++++++++++---------- unittests/test_utility.py | 24 +++++++++++++++++------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 822bad2817..7f575ac614 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -14,11 +14,13 @@ def _get_module_name(filename): barename = os.path.dirname(filename) if os.path.isabs(barename): - module_name = os.path.basename(barename) - else: - module_name = barename.replace(os.sep, '.') + raise AssertionError('BUG: _get_module_name() ' + 'accepts relative paths only') - return module_name + if filename.startswith('..'): + return os.path.basename(barename) + else: + return barename.replace(os.sep, '.') def _do_import_module_from_file(filename, module_name=None): @@ -40,15 +42,17 @@ def _do_import_module_from_file(filename, module_name=None): def import_module_from_file(filename): """Import module from file.""" - filename = os.path.normpath(os.path.expandvars(filename)) + # Expand and sanitize filename + filename = os.path.abspath(os.path.expandvars(filename)) if os.path.isdir(filename): filename = os.path.join(filename, '__init__.py') - if filename.startswith('..'): - filename = os.path.abspath(filename) - - module_name = _get_module_name(filename) - if os.path.isabs(filename) or os.path.basename(filename) == filename: + # Express filename relative to reframe + rel_filename = os.path.relpath(filename, sys.path[0]) + module_name = _get_module_name(rel_filename) + if rel_filename.startswith('..'): + # We cannot use the standard Python import mechanism here, because the + # module to import is outside the top-level package return _do_import_module_from_file(filename, module_name) return importlib.import_module(module_name) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 74af29abd5..4f833a16e0 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -287,21 +287,31 @@ def test_load_directory_relative(self): self.assertEqual('reframe', module.__name__) self.assertIs(module, sys.modules.get('reframe')) - def test_load_file_relative(self): + def test_load_relative(self): with os_ext.change_dir('reframe'): + # Load a module from a directory up module = util.import_module_from_file('../reframe/__init__.py') self.assertEqual(reframe.VERSION, module.VERSION) self.assertEqual('reframe', module.__name__) self.assertIs(module, sys.modules.get('reframe')) - def test_load_python_file(self): - with os_ext.change_dir('reframe/utility'): - module = util.import_module_from_file('os_ext.py') - self.assertEqual('os_ext', module.__name__) - self.assertIs(module, sys.modules.get('os_ext')) + # Load a module from the current directory + module = util.import_module_from_file('utility/os_ext.py') + self.assertEqual('reframe.utility.os_ext', module.__name__) + self.assertIs(module, sys.modules.get('reframe.utility.os_ext')) + + def test_load_outside_pkg(self): + module = util.import_module_from_file(os.path.__file__) + + # os imports the OS-specific path libraries under the name `path`. Our + # importer will import the actual file, thus the module name should be + # the real one. + self.assertTrue(module is sys.modules.get('posixpath') or + module is sys.modules.get('ntpath') or + module is sys.modules.get('macpath')) def test_load_twice(self): - filename = os.path.abspath('reframe/__init__.py') + filename = os.path.abspath('reframe') module1 = util.import_module_from_file(filename) module2 = util.import_module_from_file(filename) self.assertIs(module1, module2)