From 496a305dc5e91f193bb12c3f2134a9bab8129ded Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Wed, 16 Mar 2022 02:00:34 +0000 Subject: [PATCH 01/10] Implemented "default_version" for TCL --- docs/getting_started/user-guide.rst | 2 +- shpc/main/modules/__init__.py | 25 ++++++++++++++----- .../modules/templates/default_version.lua | 0 .../modules/templates/no_default_version.tcl | 2 ++ shpc/settings.yml | 2 +- 5 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 shpc/main/modules/templates/default_version.lua create mode 100644 shpc/main/modules/templates/no_default_version.tcl diff --git a/docs/getting_started/user-guide.rst b/docs/getting_started/user-guide.rst index 304c2a927..dd69ef8fc 100644 --- a/docs/getting_started/user-guide.rst +++ b/docs/getting_started/user-guide.rst @@ -178,7 +178,7 @@ variable replacement. A summary table of variables is included below, and then f - a timestamp to keep track of when you last saved - never * - default_version - - A boolean to indicate generating a .version file (LMOD or lua modules only) + - A boolean to indicate whether a default version will be arbitrarily chosen, when multiple versions are available, and none is explicitly requested - true * - singularity_module - if defined, add to module script to load this Singularity module first diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index c07886ee2..04a5daed2 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -11,7 +11,6 @@ from datetime import datetime import os -from pathlib import Path import shutil import subprocess import sys @@ -273,6 +272,23 @@ def check(self, module_name): config = self._load_container(module_name.rsplit(":", 1)[0]) return self.container.check(module_name, config) + def write_version_file(self, version_dir): + """ + Create the .version file, if there is a template for it. + + Note that we don't actually change the content of the template: + it is copied as is. + """ + version_template = 'default_version.' + self.module_extension + if not self.settings.default_version: + version_template = 'no_' + version_template + template_file = os.path.join(here, "templates", version_template) + if os.path.exists(template_file): + version_file = os.path.join(version_dir, ".version") + if not os.path.exists(version_file): + version_content = shpc.utils.read_file(template_file) + shpc.utils.write_file(version_file, version_content) + def install(self, name, tag=None, **kwargs): """ Given a unique resource identifier, install a recipe. @@ -304,11 +320,8 @@ def install(self, name, tag=None, **kwargs): shpc.utils.mkdirp([module_dir, container_dir]) # Add a .version file to indicate the level of versioning (not for tcl) - if self.module_extension != "tcl" and self.settings.default_version == True: - version_dir = os.path.join(self.settings.module_base, uri) - version_file = os.path.join(version_dir, ".version") - if not os.path.exists(version_file): - Path(version_file).touch() + version_dir = os.path.join(self.settings.module_base, uri) + self.write_version_file(version_dir) # For Singularity this is a path, podman is a uri. If None is returned # there was an error and we cleanup diff --git a/shpc/main/modules/templates/default_version.lua b/shpc/main/modules/templates/default_version.lua new file mode 100644 index 000000000..e69de29bb diff --git a/shpc/main/modules/templates/no_default_version.tcl b/shpc/main/modules/templates/no_default_version.tcl new file mode 100644 index 000000000..1c52dfc7c --- /dev/null +++ b/shpc/main/modules/templates/no_default_version.tcl @@ -0,0 +1,2 @@ +#%Module +set ModulesVersion "please_specify_a_version_number" diff --git a/shpc/settings.yml b/shpc/settings.yml index 1681ec771..6cf0c8a1d 100644 --- a/shpc/settings.yml +++ b/shpc/settings.yml @@ -27,7 +27,7 @@ module_base: $root_dir/modules # This is where you might add a prefix to your module names, if desired. module_name: '{{ parsed_name.tool }}' -# Create a .version file for LMOD in the module folder +# When multiple versions are available and none requested, allow module picking one iself default_version: true # store containers separately from module files From 2ea12ac98727fb003dc6e3f2a8e7385f65fce99b Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sat, 19 Mar 2022 23:41:30 +0000 Subject: [PATCH 02/10] Use write_version_file for the symlink tree too --- shpc/main/modules/__init__.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index be84f3937..c6310e195 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -197,11 +197,8 @@ def create_symlink(self, module_dir): # Create the symbolic link! os.symlink(module_dir, symlink_path) - # If we don't have a version file in root, create it - if self.module_extension != "tcl" and self.settings.default_version == True: - version_file = os.path.join(os.path.dirname(symlink_path), ".version") - if not os.path.exists(version_file): - Path(version_file).touch() + # Create .version + self.write_version_file(os.path.dirname(symlink_path)) def check_symlink(self, module_dir): """ From f469cb7613694d639f41908929db43ad3e6f6ae7 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Wed, 9 Mar 2022 02:19:32 +0000 Subject: [PATCH 03/10] Skip `module.tcl` in the symlinks This is done by symlinking `/` itself to `///module.tcl`. For the directory of the wrapper scripts to be correctly found, the symlink has to be resolved, but TCL's `file normalize` won't normalise the filename. So, we need to use `file readlink` instead, but only on real symlinks because it raises an error. --- shpc/main/modules/__init__.py | 9 +++++++-- shpc/main/modules/templates/docker.tcl | 4 ++-- shpc/main/modules/templates/singularity.tcl | 4 ++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index c6310e195..f9b3be16b 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -187,15 +187,20 @@ def create_symlink(self, module_dir): symlink_path = self.get_symlink_path(module_dir) if os.path.exists(symlink_path): os.unlink(symlink_path) - logger.info("Creating link %s -> %s" %(module_dir, symlink_path)) symlink_dir = os.path.dirname(symlink_path) # If the parent directory doesn't exist, make it if not os.path.exists(symlink_dir): utils.mkdirp([symlink_dir]) + if self.module_extension == "lua": + symlink_target = module_dir + else: + symlink_target = os.path.join(module_dir, self.modulefile) + logger.info("Creating link %s -> %s" % (symlink_target, symlink_path)) + # Create the symbolic link! - os.symlink(module_dir, symlink_path) + os.symlink(symlink_target, symlink_path) # Create .version self.write_version_file(os.path.dirname(symlink_path)) diff --git a/shpc/main/modules/templates/docker.tcl b/shpc/main/modules/templates/docker.tcl index ade20ceb4..93a25781c 100644 --- a/shpc/main/modules/templates/docker.tcl +++ b/shpc/main/modules/templates/docker.tcl @@ -55,8 +55,8 @@ set helpcommand "This module is a {{ docker }} container wrapper for {{ name }} {% if labels %}{% for key, value in labels.items() %}set {{ key }} "{{ value }}" {% endfor %}{% endif %} -# directory containing this modulefile (dynamically defined) -set moduleDir "[file dirname ${ModulesCurrentModulefile}]" +# directory containing this modulefile, once symlinks resolved (dynamically defined) +set moduleDir [file dirname [expr { [string equal [file type ${ModulesCurrentModulefile}] "link"] ? [file readlink ${ModulesCurrentModulefile}] : ${ModulesCurrentModulefile} }]] # conflict with modules with the same alias name conflict {{ parsed_name.tool }} diff --git a/shpc/main/modules/templates/singularity.tcl b/shpc/main/modules/templates/singularity.tcl index dad790d99..b2e078dd5 100644 --- a/shpc/main/modules/templates/singularity.tcl +++ b/shpc/main/modules/templates/singularity.tcl @@ -59,8 +59,8 @@ set helpcommand "This module is a singularity container wrapper for {{ name }} v {% if labels %}{% for key, value in labels.items() %}set {{ key }} "{{ value }}" {% endfor %}{% endif %} -# directory containing this modulefile (dynamically defined) -set moduleDir "[file dirname ${ModulesCurrentModulefile}]" +# directory containing this modulefile, once symlinks resolved (dynamically defined) +set moduleDir [file dirname [expr { [string equal [file type ${ModulesCurrentModulefile}] "link"] ? [file readlink ${ModulesCurrentModulefile}] : ${ModulesCurrentModulefile} }]] # conflict with modules with the same alias name conflict {{ parsed_name.tool }} From c67172367dbcdb3b7a27e5bc3c23269c26c97e39 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sat, 19 Mar 2022 17:53:42 +0000 Subject: [PATCH 04/10] Symlink to module.lua when possible which is when default_version==True (default_version==False can't be made to work with symlinks). --- shpc/main/modules/__init__.py | 13 +++++++++++-- shpc/main/modules/templates/docker.lua | 4 ++-- shpc/main/modules/templates/singularity.lua | 4 ++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index f9b3be16b..b0581afc7 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -178,7 +178,15 @@ def get_symlink_path(self, module_dir): """ if not self.settings.symlink_base: return - return os.path.join(self.settings.symlink_base, *module_dir.split(os.sep)[-2:]) + + symlink_base_name = os.path.join(self.settings.symlink_base, *module_dir.split(os.sep)[-2:]) + + # With Lmod and default_version==True, the symlinks points to module.lua itself, + # and its name needs to end with `.lua` too + if self.module_extension == "lua" and self.settings.default_version == True: + return symlink_base_name + ".lua" + else: + return symlink_base_name def create_symlink(self, module_dir): """ @@ -193,7 +201,8 @@ def create_symlink(self, module_dir): if not os.path.exists(symlink_dir): utils.mkdirp([symlink_dir]) - if self.module_extension == "lua": + # With Lmod, default_version==False can't be made to work with symlinks at the module.lua level + if self.module_extension == "lua" and self.settings.default_version == False: symlink_target = module_dir else: symlink_target = os.path.join(module_dir, self.modulefile) diff --git a/shpc/main/modules/templates/docker.lua b/shpc/main/modules/templates/docker.lua index 780311678..a4b6b50a8 100644 --- a/shpc/main/modules/templates/docker.lua +++ b/shpc/main/modules/templates/docker.lua @@ -40,8 +40,8 @@ For each of the above, you can export: if not os.getenv("PODMAN_OPTS") then setenv ("PODMAN_OPTS", "") end if not os.getenv("PODMAN_COMMAND_OPTS") then setenv ("PODMAN_COMMAND_OPTS", "") end --- directory containing this modulefile (dynamically defined) -local moduleDir = myFileName():match("(.*[/])") or "." +-- directory containing this modulefile, once symlinks resolved (dynamically defined) +local moduleDir = subprocess("realpath " .. myFileName()):match("(.*[/])") or "." -- interactive shell to any container, plus exec for aliases local containerPath = '{{ image }}' diff --git a/shpc/main/modules/templates/singularity.lua b/shpc/main/modules/templates/singularity.lua index f50c7ec4f..84bbe1f6c 100644 --- a/shpc/main/modules/templates/singularity.lua +++ b/shpc/main/modules/templates/singularity.lua @@ -38,8 +38,8 @@ For each of the above, you can export: {% if settings.singularity_module %}load("{{ settings.singularity_module }}"){% endif %} --- directory containing this modulefile (dynamically defined) -local moduleDir = myFileName():match("(.*[/])") or "." +-- directory containing this modulefile, once symlinks resolved (dynamically defined) +local moduleDir = subprocess("realpath " .. myFileName()):match("(.*[/])") or "." -- singularity environment variable to set shell setenv("SINGULARITY_SHELL", "{{ settings.singularity_shell }}") From 94cf3584fd8be311507efaf6acb9b7b7daa66eab Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sat, 26 Mar 2022 12:28:40 +0000 Subject: [PATCH 05/10] Added a `--force` option to `shpc install` to force overwriting existing symlinks The name `--force` is generic, so that other things could be forced through it, not just overwriting symlinks. Also added an info message if a symlink is overwritten, which can be hidden with the `--quiet` flag. --- shpc/client/__init__.py | 8 ++++++++ shpc/client/install.py | 2 +- shpc/main/modules/__init__.py | 17 ++++++++++------- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index 39dcaa02c..e286341c2 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -107,6 +107,14 @@ def get_parser(): default=False, action="store_true", ) + install.add_argument( + "--force", + "-f", + dest="force", + help="replace existing symlinks", + default=False, + action="store_true", + ) # List installed modules listing = subparsers.add_parser("list", description="list installed modules.") diff --git a/shpc/client/install.py b/shpc/client/install.py index 839f52de8..07e0b7398 100644 --- a/shpc/client/install.py +++ b/shpc/client/install.py @@ -21,4 +21,4 @@ def main(args, parser, extra, subparser): cli.settings.update_params(args.config_params) # And do the install - cli.install(args.install_recipe, symlink=args.symlink) + cli.install(args.install_recipe, symlink=args.symlink, force=args.force) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index d58842a33..1cf886e8f 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -219,17 +219,20 @@ def create_symlink(self, module_dir): # Create .version self.write_version_file(os.path.dirname(symlink_path)) - def check_symlink(self, module_dir): + def check_symlink(self, module_dir, force): """ Given an install command, if --symlink-tree is provided make sure we don't already have this symlink in the tree. """ # Get the symlink path - does it exist? symlink_path = self.get_symlink_path(module_dir) - if os.path.exists(symlink_path) and not utils.confirm_action( - "%s already exists, are you sure you want to overwrite?" % symlink_path - ): - sys.exit(0) + if os.path.exists(symlink_path): + if force: + logger.info("Overwriting %s, as requested") + elif not utils.confirm_action( + "%s already exists, are you sure you want to overwrite?" % symlink_path + ): + sys.exit(0) def _cleanup_symlink(self, module_dir): """ @@ -371,7 +374,7 @@ def write_version_file(self, version_dir): version_content = shpc.utils.read_file(template_file) shpc.utils.write_file(version_file, version_content) - def install(self, name, tag=None, symlink=False, **kwargs): + def install(self, name, tag=None, symlink=False, force=False, **kwargs): """ Given a unique resource identifier, install a recipe. @@ -405,7 +408,7 @@ def install(self, name, tag=None, symlink=False, **kwargs): if symlink: # Cut out early if symlink desired and already exists - self.check_symlink(module_dir) + self.check_symlink(module_dir, force) shpc.utils.mkdirp([module_dir, container_dir]) # Add a .version file to indicate the level of versioning (not for tcl) From df82913fa950aa50c92714af4da412437ab43fac Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 27 Mar 2022 00:07:13 +0000 Subject: [PATCH 06/10] Made `force` optional Co-authored-by: Vanessasaurus <814322+vsoch@users.noreply.github.com> --- shpc/main/modules/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index 1cf886e8f..561cb1f25 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -219,7 +219,7 @@ def create_symlink(self, module_dir): # Create .version self.write_version_file(os.path.dirname(symlink_path)) - def check_symlink(self, module_dir, force): + def check_symlink(self, module_dir, force=False): """ Given an install command, if --symlink-tree is provided make sure we don't already have this symlink in the tree. From 069010b8403b6d773ee39c5279efadfb40d8fcaf Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 27 Mar 2022 00:06:51 +0000 Subject: [PATCH 07/10] Forgot the variable for substitution --- shpc/main/modules/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index 561cb1f25..c9546d2af 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -228,7 +228,7 @@ def check_symlink(self, module_dir, force=False): symlink_path = self.get_symlink_path(module_dir) if os.path.exists(symlink_path): if force: - logger.info("Overwriting %s, as requested") + logger.info("Overwriting %s, as requested" % module_dir) elif not utils.confirm_action( "%s already exists, are you sure you want to overwrite?" % symlink_path ): From daf78d72854c2c8539a3e491d44ed48a0d7abdbe Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 27 Mar 2022 04:20:19 +0100 Subject: [PATCH 08/10] The "delete" command was superseded by "uninstall" in #6 --- shpc/client/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index e286341c2..a32c12f09 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -370,8 +370,6 @@ def help(return_code=0): from .docgen import main elif args.command == "get": from .get import main - elif args.command == "delete": - from .delete import main elif args.command == "install": from .install import main elif args.command == "inspect": From 5496b005c97b3d2208acad8f52ccf7649f123f41 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 27 Mar 2022 04:43:07 +0100 Subject: [PATCH 09/10] Added `--no-symlink-tree` to override the config file `--symlink-tree` now also overrides the config file --- shpc/client/__init__.py | 10 ++++++++-- shpc/main/modules/__init__.py | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/shpc/client/__init__.py b/shpc/client/__init__.py index a32c12f09..109821f5f 100644 --- a/shpc/client/__init__.py +++ b/shpc/client/__init__.py @@ -103,10 +103,16 @@ def get_parser(): install.add_argument( "--symlink-tree", dest="symlink", - help="install to symlink tree too.", - default=False, + help="install to symlink tree too (overrides settings.yml).", + default=None, action="store_true", ) + install.add_argument( + "--no-symlink-tree", + dest="symlink", + help="skip installing to symlink tree (in case set in settings.yml).", + action="store_false", + ) install.add_argument( "--force", "-f", diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index c9546d2af..5eb10f39c 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -374,7 +374,7 @@ def write_version_file(self, version_dir): version_content = shpc.utils.read_file(template_file) shpc.utils.write_file(version_file, version_content) - def install(self, name, tag=None, symlink=False, force=False, **kwargs): + def install(self, name, tag=None, symlink=None, force=False, **kwargs): """ Given a unique resource identifier, install a recipe. @@ -403,8 +403,9 @@ def install(self, name, tag=None, symlink=False, force=False, **kwargs): subfolder = os.path.join(uri, tag.name) container_dir = self.container.container_dir(subfolder) - # Global override to arg - symlink = self.settings.symlink_tree is True or symlink + # Default to global setting + if symlink is None: + symlink = self.settings.symlink_tree if symlink: # Cut out early if symlink desired and already exists From 7265c956a61dde6ff491561523997793a4ff0d82 Mon Sep 17 00:00:00 2001 From: Matthieu Muffato Date: Sun, 27 Mar 2022 04:47:32 +0100 Subject: [PATCH 10/10] Make it explicit we are expecting yes or no --- shpc/main/modules/__init__.py | 2 +- shpc/utils/terminal.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shpc/main/modules/__init__.py b/shpc/main/modules/__init__.py index 5eb10f39c..642782541 100644 --- a/shpc/main/modules/__init__.py +++ b/shpc/main/modules/__init__.py @@ -230,7 +230,7 @@ def check_symlink(self, module_dir, force=False): if force: logger.info("Overwriting %s, as requested" % module_dir) elif not utils.confirm_action( - "%s already exists, are you sure you want to overwrite?" % symlink_path + "%s already exists, are you sure you want to overwrite" % symlink_path ): sys.exit(0) diff --git a/shpc/utils/terminal.py b/shpc/utils/terminal.py index 1fad12226..b309f46d2 100644 --- a/shpc/utils/terminal.py +++ b/shpc/utils/terminal.py @@ -95,7 +95,7 @@ def confirm_action(question, force=False): if force is True: return True - response = input(question) + response = input(question + " (yes/no)?") while len(response) < 1 or response[0].lower().strip() not in "ynyesno": response = input("Please answer yes or no: ")