Adding dependencies through the hook framework is a bit overly complicated and undocumented

[akesandgren]

The implemented hook framework has no easy way to add dependencies (or remove them).
There are two problems:

• adding/removing dependencies must be done very early
• to complete the adding of dependencies one has to call EasyConfig.generate_template_values() and EasyConfig._finalize_dependencies(). Doing that in a hook is a bit awkward.

Adding a dependency_hook in EasyConfig.parse() before the call to self.generate_template_values() would make this much cleaner. One would still have to call self.cfg._parse_dependency() but one could add a helper function in the framework to handle the actual add/remove of a list of dependencies.

The other approach would be to write good documentation on this.

A piece of hook code that works is the following:

from distutils.version import LooseVersion
from easybuild.framework.easyconfig.format.format import DEPENDENCY_PARAMETERS
from easybuild.tools.build_log import EasyBuildError

# Changing dependencies can't be done in start_hook since it doesn't get
# any "self" argument. Use pre_fetch_hook instead since it is as early
# as it gets except start_hook
def pre_fetch_hook(self, *args, **kwargs):

    # Internal helper function
    def add_extra_dependencies(self, dep_type, extra_deps):
        """dep_type: must be in DEPENDENCY_PARAMETERS or 'osdependencies'"""
        self.log.info("[pre-fetch hook] Adding %s: %s" % (dep_type, extra_deps))

        self.cfg.enable_templating = False

        if dep_type in DEPENDENCY_PARAMETERS:
            for dep in extra_deps:
                self.cfg[dep_type].append(self.cfg._parse_dependency(dep))
        elif dep_type == 'osdependencies':
                # This currently don't work. Need to figure out how to do this.
                self.cfg[dep_type].append(extra_deps)
        else:
            raise EasyBuildError("pre_fetch_hook: Incorrect dependency type in add_extra_dependencies: %s" % dep_type)

        self.cfg.enable_templating = True
        self.cfg.generate_template_values()
        self.cfg._finalize_dependencies()

    extra_deps = []
    if self.name == 'OpenMPI':
        if LooseVersion(self.version) >= LooseVersion('2.1'):
            pmix_version = '1.2.5'
            if LooseVersion(self.version) >= LooseVersion('3'):
                pmix_version = '2.1.3'

            ucx_version = '1.3.1'

            extra_deps.append(('PMIx', pmix_version))
            # Use of external PMIx requires external libevent
            # But PMIx already has it as a dependency so we don't need
            # to explicitly set it.

            extra_deps.append(('UCX', ucx_version))

    add_extra_dependencies(self, 'dependencies', extra_deps)

[akesandgren]

There is another "problem". Since the first hook one can possibly do this in now is the pre_fetch_hook, doing a --dry-run won't show any changes in dependencies, thus possibly causing confusion.

Also --robot doesn't pick up on the added dependencies because resolving dependencies is done before pre_fetch_hook.

[boegel]

@akesandgren It sounds like we need a hook to allow you to intervene before the actual parsing of an easyconfig file happens, i.e. where you can modify the raw dict representation after the easyconfig file was read?

[akesandgren]

That might be a bit of overkill if the target is changing dependencies, (in which case the suggestion above will suffice). But it might be useful to be able to change things that early on.

[akesandgren]

--module-only will have problems if this isn't done the right way (whatever that ends up being).
I.e., using pre_fetch_hook doesn't work with --module-only

[boegel]

@akesandgren I think we can avoid that you need to call self._parse_dependency in the hook by reorganizing the code a bit...

The for key loop in the parse method should be re-written, since it now does two things at once: set all easyconfig parameter in the EasyConfig object (via self[key] = ...), and parse dependency specifications in an internal format.

The latter is why it's now so tricky to modify dependencies, you're basically being forced by doing it too late.

If we pull the for loop apart to first just set the "raw" value for the *depenendencies easyconfig parameters (i.e. the list of tuples as it appears in the easyconfig file), then you can shove a hook in between that is triggered for a follow-up that calls self._parse_dependency for everything that is needed.

Not only would that result in a hook that should allow for easily tweaking the *dependencies easyconfig parameters (and may even come in handy for something like --freeze-deps), it would actually also clean up the code a bit. :)

[akesandgren]

Removing the call to _parse_dep in the for loop leaves just self[key] = local_vars[key] right?
Thus simplifying stuff even more...

And having a hook after that loop is where i'd like to have it. One should be able to handle both deps (add/remove) and patch (add/remove) in that hook. (and technically all sorts of things)

[boegel]

@akesandgren Please check #2562, and see if it works for you...

[akesandgren]

Yes it works like a charm.

[boegel]

fixed in #2562