From bdba1c2f85bccc9888a2f0fcb9a3546e1f0cfcb0 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 12 Apr 2024 17:12:33 +0200 Subject: [PATCH 1/3] Improve behavior when using extension with 'nosource:True' --- easybuild/framework/extensioneasyblock.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 69c824fe7e..86ea151cbb 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -123,7 +123,9 @@ def _set_start_dir(self): self.log.debug("Using extension start dir: %s", ext_start_dir) self.cfg['start_dir'] = ext_start_dir self.cfg.template_values['start_dir'] = ext_start_dir - elif ext_start_dir is None: + return ext_start_dir + + if 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: @@ -137,7 +139,10 @@ def run(self, unpack_src=False): """Common operations for extensions: unpacking sources, patching, ...""" # unpack file if desired - if unpack_src: + if self.options.get('nosource', False): + # If no source wanted use the start_dir from the main EC + self.ext_dir = self.master.start_dir + elif unpack_src: targetdir = os.path.join(self.master.builddir, remove_unwanted_chars(self.name)) self.ext_dir = extract_file(self.src, targetdir, extra_options=self.unpack_options, change_into_dir=False, cmd=self.src_extract_cmd) @@ -146,10 +151,9 @@ def run(self, unpack_src=False): # because start_dir value is usually a relative path (if it is set) change_dir(self.ext_dir) - self._set_start_dir() + start_dir = self._set_start_dir() + if start_dir: change_dir(self.start_dir) - else: - self._set_start_dir() # patch if needed EasyBlock.patch_step(self, beginpath=self.ext_dir) From 138d5bea9a4df2143fa1110f16d63a2619543708 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 12 Apr 2024 18:17:48 +0200 Subject: [PATCH 2/3] Throw error if non-existant startdir was provided Improve the current error that we couldn't change into that directory. --- easybuild/framework/extensioneasyblock.py | 16 ++++++++-------- test/framework/easyblock.py | 5 +---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 86ea151cbb..efd7c349f7 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -123,15 +123,15 @@ def _set_start_dir(self): self.log.debug("Using extension start dir: %s", ext_start_dir) self.cfg['start_dir'] = ext_start_dir self.cfg.template_values['start_dir'] = ext_start_dir - return ext_start_dir - - if ext_start_dir is None: + 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: + elif self.start_dir: # 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, - ext_start_dir) + raise EasyBuildError("Provided start dir (%s) for extension %s does not exist: %s", + self.start_dir, self.name, ext_start_dir) + else: + warn_msg = 'Failed to determine start dir for extension %s: %s' % (self.name, ext_start_dir) self.log.warning(warn_msg) print_warning(warn_msg, silent=build_option('silent')) @@ -151,8 +151,8 @@ def run(self, unpack_src=False): # because start_dir value is usually a relative path (if it is set) change_dir(self.ext_dir) - start_dir = self._set_start_dir() - if start_dir: + self._set_start_dir() + if self.start_dir: change_dir(self.start_dir) # patch if needed diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index e6b54e0bc8..6ccd971141 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -2257,11 +2257,8 @@ def check_ext_start_dir(expected_start_dir, unpack_src=True): 'start_dir': 'nonexistingdir'}), ] with self.mocked_stdout_stderr(): - err_pattern = "Failed to change from .*barbar/barbar-0.0 to nonexistingdir.*" + err_pattern = r"Provided start dir \(nonexistingdir\) for extension barbar does not exist:.*" 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) # No error when using relative path in non-extracted source for some reason ec['ec']['exts_list'] = [ From 948ee8d661bff7d2f28ba111e5706a08fa845a13 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 12 Apr 2024 18:19:35 +0200 Subject: [PATCH 3/3] Fix tests for changed behavior and add test for nosource --- test/framework/easyblock.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index 6ccd971141..7ab5674b97 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -2200,11 +2200,16 @@ def test_extension_set_start_dir(self): cwd = os.getcwd() self.assertExists(cwd) - def check_ext_start_dir(expected_start_dir, unpack_src=True): + def check_ext_start_dir(expected_start_dir, unpack_src=True, parent_startdir=None): """Check start dir.""" # make sure we're in an existing directory at the start change_dir(cwd) + eb = EasyBlock(ec['ec']) + if not os.path.exists(eb.builddir): + eb.make_builddir() # Required to exist for samefile + eb.cfg['start_dir'] = parent_startdir + eb.extensions_step(fetch=True, install=False) # extract sources of the extension ext = eb.ext_instances[-1] @@ -2212,6 +2217,8 @@ def check_ext_start_dir(expected_start_dir, unpack_src=True): if expected_start_dir is None: self.assertIsNone(ext.start_dir) + # Without a start dir we don't change the CWD + self.assertEqual(os.getcwd(), cwd) else: self.assertTrue(os.path.isabs(ext.start_dir)) if ext.start_dir != os.sep: @@ -2221,14 +2228,8 @@ def check_ext_start_dir(expected_start_dir, unpack_src=True): 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' @@ -2288,6 +2289,15 @@ def check_ext_start_dir(expected_start_dir, unpack_src=True): check_ext_start_dir(os.sep, unpack_src=False) self.assertFalse(self.get_stderr()) + # Go to ECs start dir if nosource is used + ec['ec']['exts_list'] = [ + ('barbar', '0.0', { + 'nosource': True}), + ] + with self.mocked_stdout_stderr(): + check_ext_start_dir(self.test_prefix, parent_startdir=self.test_prefix) + self.assertFalse(self.get_stderr()) + def test_prepare_step(self): """Test prepare step (setting up build environment).""" test_easyconfigs = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs', 'test_ecs')