From 2868aa5961b05ff40b74b6dcd1784b0237921cc6 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Thu, 3 Oct 2019 20:46:54 +0200 Subject: [PATCH 1/5] define $EB_SCRIPT_PATH in 'eb' wrapper script, and consider it before location of 'eb' determined via $PATH in get_paths_for function --- easybuild/framework/easyconfig/tools.py | 20 +++++++++++++++++--- easybuild/tools/filetools.py | 2 +- easybuild/tools/options.py | 2 +- eb | 2 ++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index bc82e67acd..edf6c34d28 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -255,18 +255,32 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None): path_list.extend(sys.path) # figure out installation prefix, e.g. distutils install path for easyconfigs - eb_path = which('eb') + + # prefer using path specified in $EB_SCRIPT_PATH (if defined), which is set by 'eb' wrapper script + eb_path = os.getenv('EB_SCRIPT_PATH') + if eb_path is None: + # try to determine location of 'eb' script via $PATH, as fallback mechanism + eb_path = which('eb') + if eb_path is None: warning_msg = "'eb' not found in $PATH, failed to determine installation prefix!" _log.warning(warning_msg) print_warning(warning_msg) else: - # real location to 'eb' should be /bin/eb - eb_path = resolve_path(eb_path) + # eb_path is location to 'eb' wrapper script, e.g. /bin/eb + # so installation prefix is usually two levels up install_prefix = os.path.dirname(os.path.dirname(eb_path)) path_list.append(install_prefix) _log.debug("Also considering installation prefix %s..." % install_prefix) + # also consider fully resolved location to 'eb' wrapper + # see https://github.com/easybuilders/easybuild-framework/pull/2248 + resolved_eb_path = resolve_path(eb_path) + if eb_path != resolved_eb_path: + install_prefix = os.path.dirname(os.path.dirname(resolved_eb_path)) + path_list.append(install_prefix) + _log.debug("Also considering installation prefix %s (via resolved path to 'eb' script)..." % install_prefix) + # look for desired subdirs for path in path_list: path = os.path.join(path, "easybuild", subdir) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 241f654334..727e2f72b8 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -703,7 +703,7 @@ def find_eb_script(script_name): prev_script_loc = script_loc # fallback mechanism: check in location relative to location of 'eb' - eb_path = which('eb') + eb_path = os.getenv('EB_SCRIPT_PATH') or which('eb') if eb_path is None: _log.warning("'eb' not found in $PATH, failed to determine installation prefix") else: diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 43443f3dda..55a4a1106e 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -1476,7 +1476,7 @@ def parse_external_modules_metadata(cfgs): topdirs = [os.path.dirname(os.path.dirname(os.path.dirname(__file__)))] # etc/ could also be located next to bin/ - eb_cmd = which('eb') + eb_cmd = os.getenv('EB_SCRIPT_PATH') or which('eb') if eb_cmd: topdirs.append(os.path.dirname(os.path.dirname(eb_cmd))) diff --git a/eb b/eb index bc1139f36d..8fdce779aa 100755 --- a/eb +++ b/eb @@ -90,5 +90,7 @@ then export FANCYLOGGER_IGNORE_MPI4PY=1 fi +export EB_SCRIPT_PATH=$0 + verbose "$PYTHON -m easybuild.main `echo \"$@\"`" $PYTHON -m easybuild.main "$@" From e020c0b0b672ac0291abe7dc8a60a8df17a7dff2 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Thu, 3 Oct 2019 20:58:01 +0200 Subject: [PATCH 2/5] enhance test for get_paths_for --- test/framework/easyconfig.py | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 3fd4550d19..5970a69b3f 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2562,8 +2562,12 @@ def test_get_paths_for(self): test_ecs = os.path.join(top_dir, 'easyconfigs') symlink(test_ecs, os.path.join(self.test_prefix, 'easybuild', 'easyconfigs')) + # temporarily mock stderr to avoid printed warning (because 'eb' is not available via $PATH) + self.mock_stderr(True) + # locations listed in 'robot_path' named argument are taken into account res = get_paths_for(subdir='easyconfigs', robot_path=[self.test_prefix]) + self.mock_stderr(False) self.assertTrue(os.path.samefile(test_ecs, res[0])) # easyconfigs location can also be derived from location of 'eb' @@ -2588,6 +2592,42 @@ def test_get_paths_for(self): res = get_paths_for(subdir='easyconfigs', robot_path=None) self.assertTrue(os.path.samefile(test_ecs, res[0])) + # put mock 'eb' back in $PATH + os.environ['PATH'] = '%s:%s' % (os.path.join(self.test_prefix, 'bin'), orig_path) + + # if $EB_SCRIPT_PATH is specified, this is picked up to determine location to easyconfigs + someprefix = os.path.join(self.test_prefix, 'someprefix') + test_easyconfigs_dir = os.path.join(someprefix, 'easybuild', 'easyconfigs') + mkdir(test_easyconfigs_dir, parents=True) + + # put symlink in place, both original path and resolved path should be considered + symlinked_prefix = os.path.join(self.test_prefix, 'symlinked_prefix') + symlink(someprefix, symlinked_prefix) + + os.environ['EB_SCRIPT_PATH'] = os.path.join(symlinked_prefix, 'bin', 'eb') + + res = get_paths_for(subdir='easyconfigs', robot_path=None) + + # 2nd to last path is original symlinked path + self.assertEqual(res[-2], os.path.join(symlinked_prefix, 'easybuild', 'easyconfigs')) + # last path is same path, but fully resolved + # (take into account that self.test_prefix itself can also be a symlink...) + self.assertTrue(os.path.samefile(test_easyconfigs_dir, res[-1])) + + # wipe sys.path. then only paths found via $EB_SCRIPT_PATH are found + sys.path = [] + res = get_paths_for(subdir='easyconfigs', robot_path=None) + self.assertEqual(len(res), 2) + self.assertEqual(res[0], os.path.join(symlinked_prefix, 'easybuild', 'easyconfigs')) + self.assertTrue(os.path.samefile(res[1], test_easyconfigs_dir)) + + # if $EB_SCRIPT_PATH is not defined, then paths determined via 'eb' found through $PATH are picked up + del os.environ['EB_SCRIPT_PATH'] + + res = get_paths_for(subdir='easyconfigs', robot_path=None) + expected = os.path.join(self.test_prefix, 'easybuild', 'easyconfigs') + self.assertTrue(os.path.samefile(res[-1], expected)) + def test_is_generic_easyblock(self): """Test for is_generic_easyblock function.""" From 1a7c4fd2641994752abc4067276f5ac7f3dfcf03 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 4 Oct 2019 14:55:37 +0200 Subject: [PATCH 3/5] only consider resolved path to 'eb' script if desired subdir is not found relative to 'eb' script location in get_paths_for --- easybuild/framework/easyconfig/tools.py | 27 ++++++++++++++------- test/framework/easyconfig.py | 31 +++++++++++++++++-------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index edf6c34d28..a36eca54dc 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -261,6 +261,9 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None): if eb_path is None: # try to determine location of 'eb' script via $PATH, as fallback mechanism eb_path = which('eb') + _log.info("Location to 'eb' script (found via $PATH): %s", eb_path) + else: + _log.info("Found location to 'eb' script via $EB_SCRIPT_PATH: %s", eb_path) if eb_path is None: warning_msg = "'eb' not found in $PATH, failed to determine installation prefix!" @@ -270,16 +273,22 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None): # eb_path is location to 'eb' wrapper script, e.g. /bin/eb # so installation prefix is usually two levels up install_prefix = os.path.dirname(os.path.dirname(eb_path)) - path_list.append(install_prefix) - _log.debug("Also considering installation prefix %s..." % install_prefix) - - # also consider fully resolved location to 'eb' wrapper - # see https://github.com/easybuilders/easybuild-framework/pull/2248 - resolved_eb_path = resolve_path(eb_path) - if eb_path != resolved_eb_path: - install_prefix = os.path.dirname(os.path.dirname(resolved_eb_path)) + + # only consider resolved path to 'eb' script if desired subdir is not found relative to 'eb' script location + if os.path.exists(os.path.join(install_prefix, 'easybuild', subdir)): path_list.append(install_prefix) - _log.debug("Also considering installation prefix %s (via resolved path to 'eb' script)..." % install_prefix) + _log.info("Also considering installation prefix %s (determined via path to 'eb' script)...", install_prefix) + else: + _log.info("Not considering %s (no easybuild/%s subdir found)", install_prefix, subdir) + + # also consider fully resolved location to 'eb' wrapper + # see https://github.com/easybuilders/easybuild-framework/pull/2248 + resolved_eb_path = resolve_path(eb_path) + if eb_path != resolved_eb_path: + install_prefix = os.path.dirname(os.path.dirname(resolved_eb_path)) + path_list.append(install_prefix) + _log.info("Also considering installation prefix %s (via resolved path to 'eb' script)...", + install_prefix) # look for desired subdirs for path in path_list: diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 5970a69b3f..2428dc077f 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -63,8 +63,8 @@ from easybuild.tools.config import module_classes from easybuild.tools.configobj import ConfigObj from easybuild.tools.docs import avail_easyconfig_constants, avail_easyconfig_templates -from easybuild.tools.filetools import adjust_permissions, change_dir, copy_file, mkdir, read_file, remove_file -from easybuild.tools.filetools import symlink, write_file +from easybuild.tools.filetools import adjust_permissions, change_dir, copy_file, mkdir, move_file +from easybuild.tools.filetools import read_file, remove_file, symlink, write_file from easybuild.tools.module_naming_scheme.toolchain import det_toolchain_compilers, det_toolchain_mpi from easybuild.tools.module_naming_scheme.utilities import det_full_ec_version from easybuild.tools.options import parse_external_modules_metadata @@ -2599,6 +2599,7 @@ def test_get_paths_for(self): someprefix = os.path.join(self.test_prefix, 'someprefix') test_easyconfigs_dir = os.path.join(someprefix, 'easybuild', 'easyconfigs') mkdir(test_easyconfigs_dir, parents=True) + write_file(os.path.join(someprefix, 'bin', 'eb'), '') # put symlink in place, both original path and resolved path should be considered symlinked_prefix = os.path.join(self.test_prefix, 'symlinked_prefix') @@ -2608,18 +2609,14 @@ def test_get_paths_for(self): res = get_paths_for(subdir='easyconfigs', robot_path=None) - # 2nd to last path is original symlinked path - self.assertEqual(res[-2], os.path.join(symlinked_prefix, 'easybuild', 'easyconfigs')) - # last path is same path, but fully resolved - # (take into account that self.test_prefix itself can also be a symlink...) - self.assertTrue(os.path.samefile(test_easyconfigs_dir, res[-1])) + # last path is symlinked path + self.assertEqual(res[-1], os.path.join(symlinked_prefix, 'easybuild', 'easyconfigs')) - # wipe sys.path. then only paths found via $EB_SCRIPT_PATH are found + # wipe sys.path. then only path found via $EB_SCRIPT_PATH is found sys.path = [] res = get_paths_for(subdir='easyconfigs', robot_path=None) - self.assertEqual(len(res), 2) + self.assertEqual(len(res), 1) self.assertEqual(res[0], os.path.join(symlinked_prefix, 'easybuild', 'easyconfigs')) - self.assertTrue(os.path.samefile(res[1], test_easyconfigs_dir)) # if $EB_SCRIPT_PATH is not defined, then paths determined via 'eb' found through $PATH are picked up del os.environ['EB_SCRIPT_PATH'] @@ -2628,6 +2625,20 @@ def test_get_paths_for(self): expected = os.path.join(self.test_prefix, 'easybuild', 'easyconfigs') self.assertTrue(os.path.samefile(res[-1], expected)) + # also check with $EB_SCRIPT_PATH set to a symlink which doesn't allow + # directly deriving path to easybuild/easyconfigs dir, but resolved symlink does + # cfr. https://github.com/easybuilders/easybuild-framework/pull/2248 + eb_symlink = os.path.join(self.test_prefix, 'eb') + symlink(os.path.join(someprefix, 'bin', 'eb'), eb_symlink) + os.environ['EB_SCRIPT_PATH'] = eb_symlink + + res = get_paths_for(subdir='easyconfigs', robot_path=None) + # not an exact match (since it's a symlinked path in a deeper subdir), + # but fully resolved path to correct directory + self.assertTrue(os.path.exists(res[0])) + self.assertFalse(res[0] == os.path.join(someprefix, 'easybuild', 'easyconfigs')) + self.assertTrue(os.path.samefile(res[0], os.path.join(someprefix, 'easybuild', 'easyconfigs'))) + def test_is_generic_easyblock(self): """Test for is_generic_easyblock function.""" From 9fbed28c8d19e2ee1f24f3dd502c8792adaad393 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 4 Oct 2019 14:58:56 +0200 Subject: [PATCH 4/5] appease the Hound --- easybuild/framework/easyconfig/tools.py | 3 +-- test/framework/easyconfig.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index a36eca54dc..d6762ca32b 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -287,8 +287,7 @@ def get_paths_for(subdir=EASYCONFIGS_PKG_SUBDIR, robot_path=None): if eb_path != resolved_eb_path: install_prefix = os.path.dirname(os.path.dirname(resolved_eb_path)) path_list.append(install_prefix) - _log.info("Also considering installation prefix %s (via resolved path to 'eb' script)...", - install_prefix) + _log.info("Also considering installation prefix %s (via resolved path to 'eb')...", install_prefix) # look for desired subdirs for path in path_list: diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index 2428dc077f..b268acdce6 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -63,8 +63,8 @@ from easybuild.tools.config import module_classes from easybuild.tools.configobj import ConfigObj from easybuild.tools.docs import avail_easyconfig_constants, avail_easyconfig_templates -from easybuild.tools.filetools import adjust_permissions, change_dir, copy_file, mkdir, move_file -from easybuild.tools.filetools import read_file, remove_file, symlink, write_file +from easybuild.tools.filetools import adjust_permissions, change_dir, copy_file, mkdir, read_file, remove_file +from easybuild.tools.filetools import symlink, write_file from easybuild.tools.module_naming_scheme.toolchain import det_toolchain_compilers, det_toolchain_mpi from easybuild.tools.module_naming_scheme.utilities import det_full_ec_version from easybuild.tools.options import parse_external_modules_metadata From b63f805d0491f655aebc8df1085479bb2351b023 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Fri, 4 Oct 2019 15:47:45 +0200 Subject: [PATCH 5/5] fix broken test for get_paths_for --- test/framework/easyconfig.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index b268acdce6..c900df0f82 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -2633,10 +2633,7 @@ def test_get_paths_for(self): os.environ['EB_SCRIPT_PATH'] = eb_symlink res = get_paths_for(subdir='easyconfigs', robot_path=None) - # not an exact match (since it's a symlinked path in a deeper subdir), - # but fully resolved path to correct directory self.assertTrue(os.path.exists(res[0])) - self.assertFalse(res[0] == os.path.join(someprefix, 'easybuild', 'easyconfigs')) self.assertTrue(os.path.samefile(res[0], os.path.join(someprefix, 'easybuild', 'easyconfigs'))) def test_is_generic_easyblock(self):