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

add support for marking installed module file as new default version #2110

Merged
merged 18 commits into from
Mar 24, 2017
Merged
Show file tree
Hide file tree
Changes from 13 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
17 changes: 16 additions & 1 deletion easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,19 @@ def _sanity_check_step(self, custom_paths=None, custom_commands=None, extension=
else:
self.log.debug("Sanity check passed!")

def _set_module_as_default(self):
"""
Defining default module Version

sets the default module version except if we are in dry run.
"""
mod_folderpath = os.path.dirname(self.module_generator.get_module_filepath())

if self.dry_run:
dry_run_msg("Marked %s v%s as default version" % (self.name, self.version))
else:
self.module_generator.set_as_default(mod_folderpath, self.version)
Copy link
Member

Choose a reason for hiding this comment

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

maybe emit a meaningful message in the case of dry run?

if self.dry_run:
    dry_run_msg("Marked %s v%s as default version" % (self.name, self.version))
else:
    self.module_generator.set_as_default(mod_folderpath, self.version)


def cleanup_step(self):
"""
Cleanup leftover mess: remove/clean build directory
Expand Down Expand Up @@ -2138,7 +2151,6 @@ def make_module_step(self, fake=False):
self.dry_run_msg("Generating module file %s, with contents:\n", mod_filepath)
for line in txt.split('\n'):
self.dry_run_msg(' ' * 4 + line)

else:
write_file(mod_filepath, txt)
self.log.info("Module file %s written: %s", mod_filepath, txt)
Expand All @@ -2163,6 +2175,9 @@ def make_module_step(self, fake=False):
if not fake:
self.make_devel_module()

if build_option('set_as_default'):
self._set_module_as_default()

return modpath

def permissions_step(self):
Expand Down
1 change: 1 addition & 0 deletions easybuild/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'use_ccache',
'use_f90cache',
'use_existing_modules',
'set_as_default',
],
True: [
'cleanup_builddir',
Expand Down
44 changes: 34 additions & 10 deletions easybuild/tools/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,38 @@ def write_file(path, txt, append=False, forced=False):
raise EasyBuildError("Failed to write to %s: %s", path, err)


def readlink(symlink_path):
"""
Read symlink target at the specified path to the given path.

:param symlink_path: symlink file path
"""
try:
target_symlink_path = os.readlink(symlink_path)
except OSError as err:
raise EasyBuildError("Get target of symlink %s failed: %s", symlink_path, err)

return target_symlink_path


def symlink(source_path, symlink_path, use_abspath_source=True):
"""
Create a symlink at the specified path to the given path.

:param source_path: source file path
:param symlink_path: symlink file path
Copy link
Member

Choose a reason for hiding this comment

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

use_abspath_source is not documented in docstring, please fix

"""
try:
if use_abspath_source:
os.symlink(os.path.abspath(source_path), symlink_path)
_log.info("Symlinked %s to %s", os.path.abspath(source_path), symlink_path)
else:
os.symlink(source_path, symlink_path)
_log.info("Symlinked %s to %s", source_path, symlink_path)
Copy link
Member

Choose a reason for hiding this comment

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

you can shorten this a bit:

if use_abspath_source:
    source_path = os.path.abspath(source_path)

 os.symlink(source_path, symlink_path)
_log.info("Symlinked %s to %s", source_path, symlink_path)

except OSError as err:
raise EasyBuildError("Symlinking %s to %s failed: %s", source_path, symlink_path, err)


def remove_file(path):
"""Remove file at specified path."""

Expand All @@ -180,7 +212,8 @@ def remove_file(path):
return

try:
if os.path.exists(path):
# note: file may also be a broken symlink...
if os.path.exists(path) or os.path.islink(path):
Copy link
Member

Choose a reason for hiding this comment

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

@victorusu please add a dedicated test case for this fixed bug?

Copy link
Contributor Author

@victorusu victorusu Feb 16, 2017

Choose a reason for hiding this comment

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

I have added as test_remove_symlinks

os.remove(path)
except OSError, err:
raise EasyBuildError("Failed to remove %s: %s", path, err)
Expand Down Expand Up @@ -1046,15 +1079,6 @@ def weld_paths(path1, path2):
return os.path.join(path1, path2_tail)


def symlink(source_path, symlink_path):
"""Create a symlink at the specified path to the given path."""
try:
os.symlink(os.path.abspath(source_path), symlink_path)
_log.info("Symlinked %s to %s", source_path, symlink_path)
except OSError as err:
raise EasyBuildError("Symlinking %s to %s failed: %s", source_path, symlink_path, err)


def path_matches(path, paths):
"""Check whether given path matches any of the provided paths."""
if not os.path.exists(path):
Expand Down
39 changes: 38 additions & 1 deletion easybuild/tools/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@

from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.config import build_option, get_module_syntax, install_path
from easybuild.tools.filetools import mkdir, read_file
from easybuild.tools.filetools import mkdir, readlink, read_file, remove_file, symlink, write_file
from easybuild.tools.modules import modules_tool
from easybuild.tools.utilities import quote_str

Expand Down Expand Up @@ -213,7 +213,11 @@ def swap_module(self, mod_name_out, mod_name_in, guarded=True):
"""
raise NotImplementedError

def set_as_default(self, module_folder_path, module_version):
raise NotImplementedError


# noinspection PyPackageRequirements
Copy link
Member

Choose a reason for hiding this comment

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

please remove?

class ModuleGeneratorTcl(ModuleGenerator):
"""
Class for generating Tcl module files.
Expand Down Expand Up @@ -452,6 +456,19 @@ def getenv_cmd(self, envvar):
"""
return '$env(%s)' % envvar

def set_as_default(self, module_folder_path, module_version):
"""
Create .version file inside the package module folder in order to set the default version
for TMod
:param module_folder_path: module folder path, e.g. $HOME/easybuild/modules/all/Bison
:param module_version: module version, e.g. 3.0.4
"""
txt = self.MODULE_SHEBANG + '\n'
txt += 'set ModulesVersion %s\n' % module_version

# write the file no matter what
write_file(os.path.join(module_folder_path, '.version'), txt)


class ModuleGeneratorLua(ModuleGenerator):
"""
Expand Down Expand Up @@ -693,6 +710,26 @@ def getenv_cmd(self, envvar):
return 'os.getenv("%s")' % envvar


def set_as_default(self, module_folder_path, module_version):
"""
Create a symlink called "default" inside the package's module folder in order
to set the default module version
:param module_folder_path: module folder path, e.g. $HOME/easybuild/modules/all/Bison
:param module_version: module version, e.g. 3.0.4
"""
default_filepath = os.path.join(module_folder_path, 'default')

if os.path.islink(default_filepath):
link_target = readlink(default_filepath)
remove_file(default_filepath)
self.log.info("Removed default version marking from %s.", link_target)
elif os.path.exists(default_filepath):
raise EasyBuildError('Found an unexpected file called default in dir %s' % module_folder_path)
Copy link
Member

Choose a reason for hiding this comment

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

s/called default/named 'default'/g


symlink(module_version + self.MODULE_FILE_EXTENSION, default_filepath, use_abspath_source=False)
self.log.info("Module default version file written to point to %s", default_filepath)


def avail_module_generators():
"""
Return all known module syntaxes.
Expand Down
2 changes: 2 additions & 0 deletions easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ def override_options(self):
None, 'store_true', False),
'zip-logs': ("Zip logs that are copied to install directory, using specified command",
None, 'store_or_None', 'gzip'),
'set-as-default': ("Set the generated module as default", None, 'store_true', False),
Copy link
Member

Choose a reason for hiding this comment

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

maybe rename to --set-default-module? leaves less room for misinterpretation...


})

self.log.debug("override_options: descr %s opts %s" % (descr, opts))
Expand Down
44 changes: 44 additions & 0 deletions test/framework/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,49 @@ def test_is_readable(self):
os.chmod(test_file, 0)
self.assertFalse(ft.is_readable(test_file))

def test_symlink_readlink_file(self):
"""Test read link target file"""

# write_file and read_file tests are elsewhere. so not getting their states
fp = os.path.join(self.test_prefix, 'test.txt')
txt = "test_my_link_file"
ft.write_file(fp, txt)

link = os.path.join(self.test_prefix, 'test.link')
# creating the link file
ft.symlink(fp, link)

# checking if file is symlink
self.assertTrue(os.path.islink(link))
# reading link target and comparing to file name
self.assertEqual(os.path.realpath(fp), ft.readlink(link))
Copy link
Member

Choose a reason for hiding this comment

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

hmm, I think this may fail, e.g. if self.test_prefix is a symlink...

os.path.realpath will give you the fully resolved location, while ft.readlink will only resolve one symlink 'level', I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right! Setting

self.assertEqual(os.path.realpath(fp), os.path.realpath(ft.readlink(link)))

worked for me.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, ok, but then the test is worthless, no?

I'm starting to think we should change ft.readlink to ft.resolve_symlink, and basically make it call os.path.realpath in a try-except?


def test_remove_symlinks(self):
"""Test remove valid and invalid symlinks"""

# creating test file
fp = os.path.join(self.test_prefix, 'test.txt')
txt = "test_my_link_file"
ft.write_file(fp, txt)

# creating the symlink
link = os.path.join(self.test_prefix, 'test.link')
ft.symlink(fp, link) # test if is symlink is valid is done elsewhere

# Attempting to remove a valid symlink
ft.remove_file(link)
self.assertFalse(os.path.islink(link))
self.assertFalse(os.path.exists(link))

# Testing the removal of invalid symlinks
# Restoring the symlink and removing the file, this way the symlink is invalid
ft.symlink(fp, link)
ft.remove_file(fp)
# attempting to remove the invalid symlink
ft.remove_file(link)
self.assertFalse(os.path.islink(link))
self.assertFalse(os.path.exists(link))

def test_read_write_file(self):
"""Test reading/writing files."""

Expand Down Expand Up @@ -388,6 +431,7 @@ def test_read_write_file(self):
self.assertTrue(os.path.exists(foo))
self.assertEqual(ft.read_file(foo), 'bar')


def test_det_patched_files(self):
"""Test det_patched_files function."""
pf = os.path.join(os.path.dirname(__file__), 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')
Expand Down
55 changes: 54 additions & 1 deletion test/framework/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,12 @@
from distutils.version import StrictVersion
from unittest import TextTestRunner, TestSuite
from vsc.utils.fancylogger import setLogLevelDebug, logToScreen
from vsc.utils.missing import nub

from easybuild.framework.easyconfig.tools import process_easyconfig
from easybuild.tools import config
from easybuild.tools.filetools import mkdir, read_file, write_file
from easybuild.tools.modules import curr_module_paths
from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl
from easybuild.tools.module_naming_scheme.utilities import is_valid_module_name
from easybuild.framework.easyblock import EasyBlock
Expand All @@ -47,7 +50,6 @@
from easybuild.tools.utilities import quote_str
from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, find_full_path, init_config


class ModuleGeneratorTest(EnhancedTestCase):
"""Tests for module_generator module."""

Expand Down Expand Up @@ -139,6 +141,57 @@ def test_descr(self):
desc = self.modgen.get_description()
self.assertEqual(desc, expected)

def test_set_default(self):
"""
Test load part in generated module file.
"""
if self.MODULE_GENERATOR_CLASS == ModuleGeneratorLua and not isinstance(self.modtool, Lmod):
pass
Copy link
Member

Choose a reason for hiding this comment

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

@victorusu: you'll need to make this a return rather than a pass to actually skip the test

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, thanks!


# creating base path
base_path = os.path.join(self.test_prefix, 'all')
mkdir(base_path)

# creating package module
module_name = 'foobar_mod'
modules_base_path = os.path.join(base_path, module_name)
mkdir(modules_base_path)

# creating two empty modules
txt = self.modgen.MODULE_SHEBANG
if txt:
txt += '\n'
txt += self.modgen.get_description()
txt += self.modgen.set_environment('foo', 'bar')

version_one = '1.0'
version_one_path = os.path.join(modules_base_path, version_one + self.modgen.MODULE_FILE_EXTENSION)
write_file(version_one_path, txt)

version_two = '2.0'
version_two_path = os.path.join(modules_base_path, version_two + self.modgen.MODULE_FILE_EXTENSION)
write_file(version_two_path, txt)

# using base_path to possible module load
self.modtool.use(base_path)

# setting foo version as default
self.modgen.set_as_default(modules_base_path, version_one)
self.modtool.load([module_name])
full_module_name = module_name + '/' + version_one

self.assertTrue(full_module_name in self.modtool.loaded_modules())
self.modtool.purge()

# setting bar version as default
self.modgen.set_as_default(modules_base_path, version_two)
self.modtool.load([module_name])
full_module_name = module_name + '/' + version_two

self.assertTrue(full_module_name in self.modtool.loaded_modules())
self.modtool.purge()


def test_load(self):
"""Test load part in generated module file."""

Expand Down