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 implementing pre- and post-step hooks #2343

Merged
merged 18 commits into from
Dec 12, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
25 changes: 19 additions & 6 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@
from easybuild.tools.filetools import CHECKSUM_TYPE_MD5, CHECKSUM_TYPE_SHA256
from easybuild.tools.filetools import adjust_permissions, apply_patch, back_up_file, change_dir, convert_name
from easybuild.tools.filetools import compute_checksum, copy_file, derive_alt_pypi_url, diff_files, download_file
from easybuild.tools.filetools import encode_class_name, extract_file, is_alt_pypi_url, mkdir, move_logs, read_file
from easybuild.tools.filetools import remove_file, rmtree2, write_file
from easybuild.tools.filetools import verify_checksum, weld_paths
from easybuild.tools.filetools import encode_class_name, extract_file, find_hook, is_alt_pypi_url, mkdir, move_logs
from easybuild.tools.filetools import read_file, remove_file, rmtree2, write_file, verify_checksum, weld_paths
from easybuild.tools.run import run_cmd
from easybuild.tools.jenkins import write_to_xml
from easybuild.tools.module_generator import ModuleGeneratorLua, ModuleGeneratorTcl, module_generator, dependencies_for
Expand Down Expand Up @@ -136,7 +135,7 @@ def extra_options(extra=None):
#
# INIT
#
def __init__(self, ec):
def __init__(self, ec, hooks=None):
"""
Initialize the EasyBlock instance.
:param ec: a parsed easyconfig file (EasyConfig instance)
Expand All @@ -145,6 +144,9 @@ def __init__(self, ec):
# keep track of original working directory, so we can go back there
self.orig_workdir = os.getcwd()

# list of pre- and post-step hooks
self.hooks = hooks or []

# list of patch/source files, along with checksums
self.patches = []
self.src = []
Expand Down Expand Up @@ -2434,6 +2436,12 @@ def run_step(self, step, step_methods):
"""
self.log.info("Starting %s step", step)
self.update_config_template_run_step()

Copy link
Member

Choose a reason for hiding this comment

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

Calling run_hook when hooks = None would work, but seems odd.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather have the logic for dealing with not having any hooks defined in run_hook rather than having to copy-paste the logic to check whether any hooks are defined in multiple places.

Copy link
Member

Choose a reason for hiding this comment

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

Seems reasonable.

pre_hook = find_hook(step, self.hooks, pre_hook=True)
if pre_hook:
self.log.info("Found pre-%s hook, so running it...", step)
pre_hook(self)

for step_method in step_methods:
self.log.info("Running method %s part of step %s" % ('_'.join(step_method.func_code.co_names), step))

Expand All @@ -2457,6 +2465,11 @@ def run_step(self, step, step_methods):
# and returns the actual method, so use () to execute it
step_method(self)()

post_hook = find_hook(step, self.hooks, pre_hook=False)
if post_hook:
self.log.info("Found post-%s hook, so running it...", step)
post_hook(self)

if self.cfg['stop'] == step:
self.log.info("Stopping after %s step.", step)
raise StopException(step)
Expand Down Expand Up @@ -2601,7 +2614,7 @@ def print_dry_run_note(loc, silent=True):
dry_run_msg(msg, silent=silent)


def build_and_install_one(ecdict, init_env):
def build_and_install_one(ecdict, init_env, hooks=None):
"""
Build the software
:param ecdict: dictionary contaning parsed easyconfig + metadata
Expand Down Expand Up @@ -2639,7 +2652,7 @@ def build_and_install_one(ecdict, init_env):
try:
app_class = get_easyblock_class(easyblock, name=name)

app = app_class(ecdict['ec'])
app = app_class(ecdict['ec'], hooks=hooks)
_log.info("Obtained application instance of for %s (easyblock: %s)" % (name, easyblock))
except EasyBuildError, err:
print_error("Failed to get application instance for %s (easyblock: %s): %s" % (name, easyblock, err.msg),
Expand Down
19 changes: 14 additions & 5 deletions easybuild/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak
from easybuild.tools.config import find_last_log, get_repository, get_repositorypath, build_option
from easybuild.tools.docs import list_software
from easybuild.tools.filetools import adjust_permissions, cleanup, write_file
from easybuild.tools.filetools import adjust_permissions, cleanup, load_hooks, write_file
from easybuild.tools.github import check_github, find_easybuild_easyconfig, install_github_token
from easybuild.tools.github import new_pr, merge_pr, update_pr
from easybuild.tools.modules import modules_tool
Expand Down Expand Up @@ -104,8 +104,15 @@ def find_easyconfigs_by_specs(build_specs, robot_path, try_to_generate, testing=
return [(ec_file, generated)]


def build_and_install_software(ecs, init_session_state, exit_on_failure=True):
"""Build and install software for all provided parsed easyconfig files."""
def build_and_install_software(ecs, init_session_state, hooks=None, exit_on_failure=True):
"""
Build and install software for all provided parsed easyconfig files.

:param ecs: easyconfig files to install software with
:param init_session_state: initial session state, to use in test reports
:param hooks: list of defined pre- and post-step hooks
:param exit_on_failure: whether or not to exit on installation failure
"""
# obtain a copy of the starting environment so each build can start afresh
# we shouldn't use the environment from init_session_state, since relevant env vars might have been set since
# e.g. via easyconfig.handle_allowed_system_deps
Expand All @@ -115,7 +122,7 @@ def build_and_install_software(ecs, init_session_state, exit_on_failure=True):
for ec in ecs:
ec_res = {}
try:
(ec_res['success'], app_log, err) = build_and_install_one(ec, init_env)
(ec_res['success'], app_log, err) = build_and_install_one(ec, init_env, hooks=hooks)
ec_res['log_file'] = app_log
if not ec_res['success']:
ec_res['err'] = EasyBuildError(err)
Expand Down Expand Up @@ -453,9 +460,11 @@ def main(args=None, logfile=None, do_build=None, testing=False, modtool=None):
sys.exit(0)

# build software, will exit when errors occurs (except when testing)
hooks = load_hooks(build_option('hooks'))
exit_on_failure = not options.dump_test_report and not options.upload_test_report
if not testing or (testing and do_build):
ecs_with_res = build_and_install_software(ordered_ecs, init_session_state, exit_on_failure=exit_on_failure)
ecs_with_res = build_and_install_software(ordered_ecs, init_session_state, hooks=hooks,
exit_on_failure=exit_on_failure)
else:
ecs_with_res = [(ec, {}) for ec in ordered_ecs]

Expand Down
9 changes: 5 additions & 4 deletions easybuild/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'force_download',
'from_pr',
'git_working_dirs_path',
'pr_branch_name',
'pr_target_account',
'pr_target_branch',
'pr_target_repo',
'hooks',
'github_user',
'github_org',
'group',
Expand All @@ -142,6 +139,10 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX):
'optarch',
'package_tool_options',
'parallel',
'pr_branch_name',
'pr_target_account',
'pr_target_branch',
'pr_target_repo',
'rpath_filter',
'regtest_output_dir',
'skip',
Expand Down
48 changes: 48 additions & 0 deletions easybuild/tools/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import fileinput
import glob
import hashlib
import imp
Copy link
Member

Choose a reason for hiding this comment

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

No longer needed

import os
import re
import shutil
Expand Down Expand Up @@ -1651,3 +1652,50 @@ def diff_files(path1, path2):
file1_lines = ['%s\n' % l for l in read_file(path1).split('\n')]
file2_lines = ['%s\n' % l for l in read_file(path2).split('\n')]
return ''.join(difflib.unified_diff(file1_lines, file2_lines, fromfile=path1, tofile=path2))


def load_hooks(hooks_path):
"""Load defined hooks (if any)."""
hooks = []
if hooks_path:
(hooks_dir, hooks_filename) = os.path.split(hooks_path)
(hooks_mod_name, hooks_file_ext) = os.path.splitext(hooks_filename)
if hooks_file_ext == '.py':
_log.info("Importing hooks implementation from %s...", hooks_path)
(fh, pathname, descr) = imp.find_module(hooks_mod_name, [hooks_dir])
try:
# import module that defines hooks, and collect all functions of which name ends with '_hook'
imported_hooks = imp.load_module(hooks_mod_name, fh, pathname, descr)
for attr in dir(imported_hooks):
if attr.endswith('_hook'):
hook = getattr(imported_hooks, attr)
if callable(hook):
hooks.append(hook)
else:
_log.debug("Skipping non-callable attribute '%s' when loading hooks", attr)
_log.debug("Found hooks: %s", hooks)
except ImportError as err:
raise EasyBuildError("Failed to import hooks implementation from %s: %s", hooks_path, err)
else:
raise EasyBuildError("Provided path for hooks implementation should be location of a Python file (*.py)")
else:
_log.info("No location for hooks implementation provided, no hooks defined")

return hooks


def find_hook(step_name, known_hooks, pre_hook=True):
"""
Find pre- or post-hook for specified step.

:param step_name: name of the step that hook relates to
:param pre_hook: True to search for pre-step hook, False to search for post-step hook
"""
res = None
hook_name = ('post_', 'pre_')[pre_hook] + step_name + '_hook'
for hook in known_hooks:
if hook.__name__ == hook_name:
res = hook
break

return res
7 changes: 6 additions & 1 deletion easybuild/tools/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
from easybuild.tools.docs import avail_toolchain_opts, avail_easyconfig_params, avail_easyconfig_templates
from easybuild.tools.docs import list_easyblocks, list_toolchains
from easybuild.tools.environment import restore_env, unset_env_vars
from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, mkdir
from easybuild.tools.filetools import CHECKSUM_TYPE_SHA256, CHECKSUM_TYPES, mkdir, resolve_path
from easybuild.tools.github import GITHUB_EB_MAIN, GITHUB_EASYCONFIGS_REPO, HAVE_GITHUB_API, HAVE_KEYRING
from easybuild.tools.github import fetch_github_token
from easybuild.tools.include import include_easyblocks, include_module_naming_schemes, include_toolchains
Expand Down Expand Up @@ -433,6 +433,7 @@ def config_options(self):
'buildpath': ("Temporary build path", None, 'store', mk_full_default_path('buildpath')),
'external-modules-metadata': ("List of files specifying metadata for external modules (INI format)",
'strlist', 'store', None),
'hooks': ("Location of Python module with hook implementations", 'str', 'store', None),
'ignore-dirs': ("Directory names to ignore when searching for files/dirs",
'strlist', 'store', ['.git', '.svn']),
'include-easyblocks': ("Location(s) of extra or customized easyblocks", 'strlist', 'store', []),
Expand Down Expand Up @@ -779,6 +780,10 @@ def postprocess(self):
# handle configuration options that affect other configuration options
self._postprocess_config()

# make sure absolute path is used for location of hooks
if self.options.hooks:
self.options.hooks = resolve_path(self.options.hooks)

# show current configuration and exit, if requested
if self.options.show_config or self.options.show_full_config:
self.show_config()
Expand Down
30 changes: 30 additions & 0 deletions test/framework/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,36 @@ def test_diff_files(self):
regex = re.compile('^--- .*/foo\s*\n\+\+\+ .*/bar\s*$', re.M)
self.assertTrue(regex.search(res), "Pattern '%s' found in: %s" % (regex.pattern, res))

def test_hooks(self):
"""Test for functions that support use of hooks."""
test_hooks_pymod = os.path.join(self.test_prefix, 'test_hooks.py')
test_hooks_pymod_txt = '\n'.join([
'def pre_install_hook(self):',
' pass',
'def foo():',
' pass',
'def post_configure_hook(self):',
' foo()',
' pass',
])
ft.write_file(test_hooks_pymod, test_hooks_pymod_txt)

hooks = ft.load_hooks(test_hooks_pymod)
self.assertEqual(len(hooks), 2)
self.assertEqual(sorted(h.__name__ for h in hooks), ['post_configure_hook', 'pre_install_hook'])
self.assertTrue(all(callable(h) for h in hooks))

post_configure_hook = [h for h in hooks if h.__name__ == 'post_configure_hook'][0]
pre_install_hook = [h for h in hooks if h.__name__ == 'pre_install_hook'][0]

self.assertEqual(ft.find_hook('configure', hooks), None)
self.assertEqual(ft.find_hook('configure', hooks, pre_hook=True), None)
self.assertEqual(ft.find_hook('configure', hooks, pre_hook=False), post_configure_hook)

self.assertEqual(ft.find_hook('install', hooks), pre_install_hook)
self.assertEqual(ft.find_hook('install', hooks, pre_hook=True), pre_install_hook)
self.assertEqual(ft.find_hook('install', hooks, pre_hook=False), None)


def suite():
""" returns all the testcases in this module """
Expand Down
35 changes: 35 additions & 0 deletions test/framework/toy_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,41 @@ def test_toy_build_trace(self):
regex = re.compile(pattern, re.M)
self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout))

def test_toy_build_hooks(self):
"""Test use of --hooks."""
hooks_file = os.path.join(self.test_prefix, 'my_hooks.py')
hooks_file_txt = '\n'.join([
"import os",
'',
"def pre_configure_hook(self):",
" print('pre-configure: toy.source: %s' % os.path.exists('toy.source'))",
'',
"def post_configure_hook(self):",
" print('post-configure: toy.source: %s' % os.path.exists('toy.source'))",
'',
"def post_install_hook(self):",
" print('in post-install hook for %s v%s' % (self.name, self.version))",
" print(', '.join(sorted(os.listdir(self.installdir))))",
])
write_file(hooks_file, hooks_file_txt)

self.mock_stderr(True)
self.mock_stdout(True)
self.test_toy_build(extra_args=['--hooks=%s' % hooks_file])
stderr = self.get_stderr()
stdout = self.get_stdout()
self.mock_stderr(False)
self.mock_stdout(False)

self.assertEqual(stderr, '')
expected_output = '\n'.join([
"pre-configure: toy.source: True",
"post-configure: toy.source: False",
"in post-install hook for toy v0.0",
"bin, lib",
])
self.assertEqual(stdout.strip(), expected_output)


def suite():
""" return all the tests in this file """
Expand Down