improve handling of start_dir and add tests for cases where either ext_dir or initial start_dir or both are unset or None

easybuild/framework/extensioneasyblock.py

@@ -103,22 +103,30 @@ def __init__(self, *args, **kwargs):
     def _set_start_dir(self):
         """Set absolute path of self.start_dir similarly to EasyBlock.guess_start_dir
 
-        Uses existing value of self.start_dir if it is already set, an absolute path and exists
-        otherwise use self.ext_dir (path to extracted source) as base dir if that is set and exists.
+        Uses existing value of self.start_dir defaulting to self.ext_dir.
+        If self.ext_dir (path to extracted source) is set, it is used as the base dir for relative paths.
+        Otherwise otherwise self.builddir is used as the base.
+        When neither start_dir nor ext_dir are set or when the computed start_dir does not exist
+        the start dir is not changed.
+        The computed start dir will not end in path separators
         """
-        ext_start_dir = ''
-
-        if self.start_dir:
-            ext_start_dir = self.start_dir
-
-        if not os.path.isabs(ext_start_dir) and self.ext_dir:
-            # start dir is either empty or a _relative_ path provided by user through self.start_dir
-            # generate absolute path from ext_dir
-            ext_start_dir = os.path.join(self.ext_dir, ext_start_dir)
-
-        if os.path.isdir(ext_start_dir):
+        ext_start_dir = self.start_dir
+        if self.ext_dir:
+            if not os.path.isabs(self.ext_dir):
+                raise EasyBuildError("ext_dir must be an absolute path. Is: '%s'", self.ext_dir)
+            ext_start_dir = os.path.join(self.ext_dir, ext_start_dir or '')
+        elif ext_start_dir is not None:
+            if not os.path.isabs(self.builddir):
+                raise EasyBuildError("builddir must be an absolute path. Is: '%s'", self.builddir)
+            ext_start_dir = os.path.join(self.builddir, ext_start_dir)
+
+        if ext_start_dir and os.path.isdir(ext_start_dir):
+            ext_start_dir = ext_start_dir.rstrip(os.sep) or os.sep
             self.cfg['start_dir'] = ext_start_dir
             self.log.debug("Using extension start dir: %s", ext_start_dir)
+        elif ext_start_dir is None:
+            # This may be on purpose, e.g. for Python WHL files which do not get extracted
+            self.log.debug("Start dir is not set.")
         else:
             # non-existing start dir means wrong input from user
             warn_msg = "Provided start dir (%s) for extension %s does not exist: %s" % (self.start_dir, self.name,

test/framework/easyblock.py

@@ -46,7 +46,7 @@
 from easybuild.tools import LooseVersion, config
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.config import get_module_syntax, update_build_option
-from easybuild.tools.filetools import change_dir, copy_dir, copy_file, mkdir, read_file, remove_file
+from easybuild.tools.filetools import change_dir, copy_dir, copy_file, mkdir, read_file, remove_dir, remove_file
 from easybuild.tools.filetools import verify_checksum, write_file
 from easybuild.tools.module_generator import module_generator
 from easybuild.tools.modules import EnvironmentModules, Lmod, reset_module_caches
@@ -2172,46 +2172,96 @@ def test_extension_set_start_dir(self):
         cwd = os.getcwd()
         self.assertTrue(os.path.exists(cwd))
 
-        def check_ext_start_dir(expected_start_dir):
+        def check_ext_start_dir(expected_start_dir, unpack_src=True):
             """Check start dir."""
             # make sure we're in an existing directory at the start
             change_dir(cwd)
             eb = EasyBlock(ec['ec'])
             eb.extensions_step(fetch=True, install=False)
             # extract sources of the extension
             ext = eb.ext_instances[-1]
-            ext.run(unpack_src=True)
-            abs_expected_start_dir = os.path.join(eb.builddir, expected_start_dir)
-            self.assertTrue(os.path.samefile(ext.cfg['start_dir'], abs_expected_start_dir))
-            self.assertTrue(os.path.samefile(os.getcwd(), abs_expected_start_dir))
+            ext.run(unpack_src=unpack_src)
+
+            if expected_start_dir is None:
+                self.assertIsNone(ext.start_dir)
+            else:
+                self.assertTrue(os.path.isabs(ext.start_dir))
+                if ext.start_dir != os.sep:
+                    self.assertFalse(ext.start_dir.endswith(os.sep))
+                if os.path.isabs(expected_start_dir):
+                    abs_expected_start_dir = expected_start_dir
+                else:
+                    abs_expected_start_dir = os.path.join(eb.builddir, expected_start_dir)
+                self.assertEqual(ext.start_dir, abs_expected_start_dir)
+                if not os.path.exists(eb.builddir):
+                    eb.make_builddir()  # Required to exist for samefile
+                self.assertTrue(os.path.samefile(ext.start_dir, abs_expected_start_dir))
+            if unpack_src:
+                self.assertTrue(os.path.samefile(os.getcwd(), abs_expected_start_dir))
+            else:
+                # When not unpacking we don't change the CWD
+                self.assertEqual(os.getcwd(), cwd)
+            remove_dir(eb.builddir)
 
         ec['ec']['exts_defaultclass'] = 'DummyExtension'
 
         # default (no start_dir specified): use unpacked dir as start dir
         ec['ec']['exts_list'] = [
             ('barbar', '0.0', {}),
         ]
-        check_ext_start_dir('barbar/barbar-0.0')
+        with self.mocked_stdout_stderr():
+            check_ext_start_dir('barbar/barbar-0.0')
+            check_ext_start_dir(None, unpack_src=False)
+            self.assertFalse(self.get_stderr())
 
         # use start dir defined in extension
         ec['ec']['exts_list'] = [
             ('barbar', '0.0', {
                 'start_dir': 'src'}),
         ]
-        check_ext_start_dir('barbar/barbar-0.0/src')
+        with self.mocked_stdout_stderr():
+            check_ext_start_dir('barbar/barbar-0.0/src')
+            self.assertFalse(self.get_stderr())
 
         # clean error when specified start dir does not exist
         ec['ec']['exts_list'] = [
             ('barbar', '0.0', {
                 'start_dir': 'nonexistingdir'}),
         ]
-        self.mock_stderr(True)
-        err_pattern = "Failed to change from .*barbar/barbar-0.0 to nonexistingdir.*"
-        self.assertErrorRegex(EasyBuildError, err_pattern, check_ext_start_dir, 'whatever')
-        stderr = self.get_stderr()
+        with self.mocked_stdout_stderr():
+            err_pattern = "Failed to change from .*barbar/barbar-0.0 to nonexistingdir.*"
+            self.assertErrorRegex(EasyBuildError, err_pattern, check_ext_start_dir, 'whatever')
+            stderr = self.get_stderr()
         warning_pattern = "WARNING: Provided start dir (nonexistingdir) for extension barbar does not exist"
         self.assertIn(warning_pattern, stderr)
-        self.mock_stderr(False)
+
+        # No error when using relative path in non-extracted source for some reason
+        ec['ec']['exts_list'] = [
+            ('barbar', '0.0', {
+                'start_dir': '.'}),  # The build directory which does exist
+        ]
+        with self.mocked_stdout_stderr():
+            check_ext_start_dir('.', unpack_src=False)
+            self.assertFalse(self.get_stderr())
+
+        # Keep absolute path in start_dir
+        assert os.path.isabs(self.test_prefix)
+        ec['ec']['exts_list'] = [
+            ('barbar', '0.0', {
+                'start_dir': self.test_prefix}),
+        ]
+        with self.mocked_stdout_stderr():
+            check_ext_start_dir(self.test_prefix, unpack_src=False)
+            self.assertFalse(self.get_stderr())
+
+        # Support / (absolute path) if explicitely requested
+        ec['ec']['exts_list'] = [
+            ('barbar', '0.0', {
+                'start_dir': os.sep}),
+        ]
+        with self.mocked_stdout_stderr():
+            check_ext_start_dir(os.sep, unpack_src=False)
+            self.assertFalse(self.get_stderr())
 
     def test_prepare_step(self):
         """Test prepare step (setting up build environment)."""