Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip module in the symlinks #511

Merged
merged 14 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/getting_started/user-guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,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
Expand Down
18 changes: 15 additions & 3 deletions shpc/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,21 @@ def get_parser():
install.add_argument(
"--symlink-tree",
dest="symlink",
help="install to symlink tree too.",
help="install to symlink tree too (overrides settings.yml).",
default=None,
Copy link
Member

@vsoch vsoch Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh this is neat I’ve never seen it before! So if —symlink-tree is set, the None is True and this overrides the default False of no symlink tree (should that be None too)? And then if no symlink tree is True the None is ignored? What happens if both flags are provided?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should use store_const here to make the three cases more clear? https://stackoverflow.com/a/34750557

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this overrides the default False of no symlink tree (should that be None too)

It works as it is: None if no option given, and True / False for --symlink-tree / --no-symlink-tree

What happens if both flags are provided?

If both are given, the last one wins

Perhaps we should use store_const here to make the three cases more clear? https://stackoverflow.com/a/34750557

A matter of choice :) I find store_true and store_false very explicit :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I’m cool with that :)

will probably finish review tomorrow - I want to look everything over once more with a clear head, and I’m rather tired and still a bit on edge from the day. So stay tuned for tomorrow! I think the default version PR looks great (and can update the PR here) so probably I’ll merge that one first.

goodnight from the inferno! lol

F1103B3A-C907-4872-91DA-C70F9447F509

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",
dest="force",
help="replace existing symlinks",
default=False,
action="store_true",
)
Expand Down Expand Up @@ -362,8 +376,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":
Expand Down
2 changes: 1 addition & 1 deletion shpc/client/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
75 changes: 52 additions & 23 deletions shpc/main/modules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

from datetime import datetime
import os
from pathlib import Path
import shutil
import subprocess
import sys
Expand Down Expand Up @@ -114,6 +113,7 @@ def _uninstall(self, module_dir, name, force=False):
msg = "%s, and all content below it? " % name
if not utils.confirm_uninstall(msg, force):
return
self._cleanup_symlink(module_dir)
self._cleanup(module_dir)
logger.info("%s and all subdirectories have been removed." % name)
else:
Expand Down Expand Up @@ -183,7 +183,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):
"""
Expand All @@ -192,33 +200,39 @@ 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])

# 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)
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)

# 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):
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.
"""
# 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" % module_dir)
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):
"""
Expand Down Expand Up @@ -343,7 +357,24 @@ def check(self, module_name):
config = self._load_container(module_name.rsplit(":", 1)[0])
return self.container.check(module_name, config)

def install(self, name, tag=None, symlink=False, **kwargs):
def write_version_file(self, version_dir):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this section old from before? I guess we need to merge the version changes before this one!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because I had done a merge of the tcl_default_version :/
I can probably remove this merge commit to make this PR clearer ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - let's update this PR with the changes in current main and then I can give another review!

"""
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, symlink=None, force=False, **kwargs):
"""
Given a unique resource identifier, install a recipe.

Expand Down Expand Up @@ -372,20 +403,18 @@ def install(self, name, tag=None, symlink=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
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)
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
Expand Down
Empty file.
4 changes: 2 additions & 2 deletions shpc/main/modules/templates/docker.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}'
Expand Down
4 changes: 2 additions & 2 deletions shpc/main/modules/templates/docker.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 2 additions & 0 deletions shpc/main/modules/templates/no_default_version.tcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#%Module
set ModulesVersion "please_specify_a_version_number"
4 changes: 2 additions & 2 deletions shpc/main/modules/templates/singularity.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}")
Expand Down
4 changes: 2 additions & 2 deletions shpc/main/modules/templates/singularity.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion shpc/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion shpc/utils/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: ")

Expand Down