From e9417e404be644864ace20e2bd2d7ebea3a72acf Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Wed, 25 Jan 2023 01:08:38 +0100 Subject: [PATCH 1/7] enforce absolute paths as start dir of extensions --- easybuild/framework/extensioneasyblock.py | 30 ++++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index c9aa1eeacb..ef9035efb5 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -100,18 +100,30 @@ def __init__(self, *args, **kwargs): self.ext_dir = None # dir where extension source was unpacked def _set_start_dir(self): - """Set value for self.start_dir + """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 and exists - otherwise self.ext_dir (path to extracted source) if that is set and exists, similar to 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. """ - possible_dirs = (self.start_dir, self.ext_dir) - for possible_dir in possible_dirs: - if possible_dir and os.path.isdir(possible_dir): - self.cfg['start_dir'] = possible_dir - self.log.debug("Using start_dir: %s", self.start_dir) + start_dir = '' + + # Use provided start dir if it is an absolute path + if self.start_dir: + start_dir = self.start_dir + if os.path.isabs(start_dir) and os.path.isdir(start_dir): + self.log.info("Using user provided start dir: %s", start_dir) + return + + # Generate absolute start dir from ext_dir + if self.ext_dir: + ext_start_dir = os.path.join(self.ext_dir, start_dir) + if os.path.isdir(ext_start_dir): + self.cfg['start_dir'] = ext_start_dir + self.log.debug("Using start dir: %s", ext_start_dir) return - self.log.debug("Unable to determine start_dir as none of these paths is set and exists: %s", possible_dirs) + + tested_dirs = ', '.join([d for d in [start_dir, ext_start_dir] if d]) + self.log.debug("Unable to determine extension start dir as none of the tentative dirs exist: %s" % tested_dirs) def run(self, unpack_src=False): """Common operations for extensions: unpacking sources, patching, ...""" From a7304ff15d2712e3423ff06f9d98e1b56b0b3cd7 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Wed, 25 Jan 2023 16:05:33 +0100 Subject: [PATCH 2/7] add unit test test_extension_set_start_dir --- test/framework/easyblock.py | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/framework/easyblock.py b/test/framework/easyblock.py index a8cd59375e..9bda723b41 100644 --- a/test/framework/easyblock.py +++ b/test/framework/easyblock.py @@ -2164,6 +2164,50 @@ def check_start_dir(expected_start_dir): err_pattern = "Specified start dir .*/toy-0.0/thisstartdirisnotthere does not exist" self.assertErrorRegex(EasyBuildError, err_pattern, check_start_dir, 'whatever') + def test_extension_set_start_dir(self): + """Test start dir with extensions.""" + test_easyconfigs = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs', 'test_ecs') + ec = process_easyconfig(os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0.eb'))[0] + + cwd = os.getcwd() + self.assertTrue(os.path.exists(cwd)) + + def check_ext_start_dir(expected_start_dir): + """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)) + + 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') + + # 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') + + # clean error when specified start dir does not exist + ec['ec']['exts_list'] = [ + ('barbar', '0.0', { + 'start_dir': 'nonexistingdir'}), + ] + err_pattern = "Failed to change from .*barbar/barbar-0.0 to nonexistingdir.*" + self.assertErrorRegex(EasyBuildError, err_pattern, check_ext_start_dir, 'whatever') + 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') From 54501a8767b3ea2f7689260bdf524afa783cd118 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 26 Jan 2023 15:39:04 +0100 Subject: [PATCH 3/7] inititalise ext_start_dir in ExtensionEasyBlock._set_start_dir --- easybuild/framework/extensioneasyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index ef9035efb5..ccd60fbc12 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -105,7 +105,7 @@ def _set_start_dir(self): 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. """ - start_dir = '' + start_dir, ext_start_dir = '', '' # Use provided start dir if it is an absolute path if self.start_dir: @@ -122,7 +122,7 @@ def _set_start_dir(self): self.log.debug("Using start dir: %s", ext_start_dir) return - tested_dirs = ', '.join([d for d in [start_dir, ext_start_dir] if d]) + tested_dirs = ', '.join([d for d in (start_dir, ext_start_dir) if d]) self.log.debug("Unable to determine extension start dir as none of the tentative dirs exist: %s" % tested_dirs) def run(self, unpack_src=False): From a00af7f964c5df2289978f6343229dab18089088 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 26 Jan 2023 15:42:50 +0100 Subject: [PATCH 4/7] clarify comment about joining ext_dir and start_dir in ExtensionEasyBlock Co-authored-by: ocaisa --- easybuild/framework/extensioneasyblock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index ccd60fbc12..c0ac881847 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -116,6 +116,7 @@ def _set_start_dir(self): # Generate absolute start dir from ext_dir if self.ext_dir: + # User may have provided a _relative_ path for self.start_dir (now stored in start_dir) ext_start_dir = os.path.join(self.ext_dir, start_dir) if os.path.isdir(ext_start_dir): self.cfg['start_dir'] = ext_start_dir From c02a5a44e040d009c1bc9ef6370333c0de141edb Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 26 Jan 2023 16:18:22 +0100 Subject: [PATCH 5/7] move comment about using provided absolute start dir path in ExtensionEasyBlock Co-authored-by: ocaisa --- easybuild/framework/extensioneasyblock.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index c0ac881847..1ef7263d4b 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -107,9 +107,9 @@ def _set_start_dir(self): """ start_dir, ext_start_dir = '', '' - # Use provided start dir if it is an absolute path if self.start_dir: start_dir = self.start_dir + # Use provided start dir if it is an absolute path if os.path.isabs(start_dir) and os.path.isdir(start_dir): self.log.info("Using user provided start dir: %s", start_dir) return From 699f7cbd701660377a223c00673f5a536367d64f Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 27 Jan 2023 00:33:00 +0100 Subject: [PATCH 6/7] refactor ExtensionEasyBlock._set_start_dir without early returns --- easybuild/framework/extensioneasyblock.py | 32 ++++++++++------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 1ef7263d4b..32a316470d 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -105,27 +105,23 @@ def _set_start_dir(self): 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. """ - start_dir, ext_start_dir = '', '' + ext_start_dir = '' if self.start_dir: - start_dir = self.start_dir - # Use provided start dir if it is an absolute path - if os.path.isabs(start_dir) and os.path.isdir(start_dir): - self.log.info("Using user provided start dir: %s", start_dir) - return - - # Generate absolute start dir from ext_dir - if self.ext_dir: - # User may have provided a _relative_ path for self.start_dir (now stored in start_dir) - ext_start_dir = os.path.join(self.ext_dir, start_dir) - if os.path.isdir(ext_start_dir): - self.cfg['start_dir'] = ext_start_dir - self.log.debug("Using start dir: %s", ext_start_dir) - return - - tested_dirs = ', '.join([d for d in (start_dir, ext_start_dir) if d]) - self.log.debug("Unable to determine extension start dir as none of the tentative dirs exist: %s" % tested_dirs) + 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): + self.cfg['start_dir'] = ext_start_dir + self.log.debug("Using extension start dir: %s", ext_start_dir) + else: + # non-existing start dir means wrong input from user + self.log.debug("Provided start dir for extension does not exist: %s" % ext_start_dir) + def run(self, unpack_src=False): """Common operations for extensions: unpacking sources, patching, ...""" From 183d0365a7657efda1ff3d5d5d755fc15a7e4252 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 27 Jan 2023 00:35:22 +0100 Subject: [PATCH 7/7] fix codestyle issues in ExtensionEasyBlock._set_start_dir --- easybuild/framework/extensioneasyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/extensioneasyblock.py b/easybuild/framework/extensioneasyblock.py index 32a316470d..db75ad0c77 100644 --- a/easybuild/framework/extensioneasyblock.py +++ b/easybuild/framework/extensioneasyblock.py @@ -120,8 +120,8 @@ def _set_start_dir(self): self.log.debug("Using extension start dir: %s", ext_start_dir) else: # non-existing start dir means wrong input from user - self.log.debug("Provided start dir for extension does not exist: %s" % ext_start_dir) - + self.log.debug("Provided start dir for extension does not exist: %s", ext_start_dir) + def run(self, unpack_src=False): """Common operations for extensions: unpacking sources, patching, ..."""