Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 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
11 changes: 10 additions & 1 deletion easybuild/framework/easyconfig/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@
'exts_list': [[], 'List with extensions added to the base installation', EXTENSIONS],

# MODULES easyconfig parameters
'whatis': [None, "List of brief (one line) package description entries", MODULES],
'modaliases': [{}, "Aliases to be defined in module file", MODULES],
'modextrapaths': [{}, "Extra paths to be prepended in module file", MODULES],
'modextravars': [{}, "Extra environment variables to be added to module file", MODULES],
Expand All @@ -171,6 +170,16 @@
'include_modpath_extensions': [True, "Include $MODULEPATH extensions specified by module naming scheme.", MODULES],
'recursive_module_unload': [False, 'Recursive unload of all dependencies when unloading module', MODULES],

# MODULES documentation easyconfig parameters
# (docurls is part of MANDATORY)
'docpaths': [None, "List of paths of installed documentation relative to package root dir", MODULES],
Copy link
Member

Choose a reason for hiding this comment

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

List of paths for documentation relative to installation directory

'examples': [None, "Free-form text with examples", MODULES],
Copy link
Member

Choose a reason for hiding this comment

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

... with examples on using the software

'site_contact': [None, "String/list of strings with site contacts for the package", MODULES],
Copy link
Member

Choose a reason for hiding this comment

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

plural: site_contacts

also: s/package/software/

'upstream_contact': [None, ("String/list of strings with upstream contact addresses "
Copy link
Member

Choose a reason for hiding this comment

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

I'd make this plural: upstream_contacts

"(e.g., support e-mail, mailing list, bugtracker)"), MODULES],
'usage': [None, "Usage instructions for the package", MODULES],
Copy link
Member

Choose a reason for hiding this comment

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

s/package/software/

'whatis': [None, "List of brief (one line) package description entries", MODULES],
Copy link
Member

Choose a reason for hiding this comment

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

s/package/software/


# OTHER easyconfig parameters
'buildstats': [None, "A list of dicts with build statistics", OTHER],
}
Expand Down
125 changes: 104 additions & 21 deletions easybuild/tools/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@
import re
import sys
import tempfile
from textwrap import wrap
from vsc.utils import fancylogger
from vsc.utils.missing import get_subclasses

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, remove_file, resolve_path, symlink, write_file
from easybuild.tools.modules import modules_tool
from easybuild.tools.filetools import convert_name, mkdir, read_file, remove_file, resolve_path, symlink, write_file
from easybuild.tools.modules import ROOT_ENV_VAR_NAME_PREFIX, modules_tool
from easybuild.tools.utilities import quote_str


Expand Down Expand Up @@ -298,6 +299,100 @@ def use(self, paths, prefix=None, guarded=False):
"""
raise NotImplementedError

def _generate_help_text(self):
"""
Generate syntax-independent help text used for `module help`.
"""

def generate_section(sec_name, sec_txt, strip=False):
"""
Generate section with given name and contents.
"""
if sec_txt:
if strip:
text = sec_txt.strip()
else:
text = sec_txt

return [
'',
'',
sec_name,
'=' * len(sec_name),
text,
]
else:
return []
Copy link
Member

Choose a reason for hiding this comment

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

hmm, this is a bit too vertically verbose, no? also, we generally avoid inline returns

also, let's make this a private method like the others you're calling out to, rather than an inner function?

def _generate_help_section(sec_name, sec_txt, strip=False):
    """
    Generate section with given name and contents.
    """
    res = []
    if sec_txt:
        if strip:
            sec_txt = sec_txt.strip()
        res = ['', '', sec_name, '=' * len(sec_name), sec_txt]
    return res


# General package description (mandatory)
lines = generate_section('Description', self.app.cfg['description'], strip=True)

# Package usage instructions (optional)
lines.extend(generate_section('Usage', self.app.cfg['usage'], strip=True))

# Examples (optional)
lines.extend(generate_section('Examples', self.app.cfg['examples'], strip=True))

# Additional information: homepage + (if available) doc paths/urls, upstream/site contact
lines.extend(generate_section("More information", " - Homepage: %s" % self.app.cfg['homepage']))

docpaths = self.app.cfg['docpaths'] or []
docurls = self.app.cfg['docurls'] or []
if docpaths or docurls:
env_name = convert_name(self.app.name, upper=True)
root_envvar = ROOT_ENV_VAR_NAME_PREFIX + env_name
Copy link
Member

Choose a reason for hiding this comment

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

collapse these into

root_envvar = ROOT_ENV_VAR_NAME_PREFIX + convert_name(self.app.name, upper=True)

lines.extend([" - Documentation:"])
lines.extend([" - $%s/%s" % (root_envvar,path) for path in docpaths])
Copy link
Member

Choose a reason for hiding this comment

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

style nitpicking: space after ,

lines.extend([" - %s" % url for url in docurls])

upstream = self.app.cfg['upstream_contact']
if upstream:
Copy link
Member

Choose a reason for hiding this comment

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

s/upstream/upstream_contacts/

if isinstance(upstream, list):
lines.extend([" - Upstream contacts:"])
lines.extend([" - %s" % address for address in upstream])
Copy link
Member

Choose a reason for hiding this comment

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

s/address/contact/

(may be just a name, or an URL)

else:
lines.extend([" - Upstream contact: %s" % upstream])

site_contact = self.app.cfg['site_contact']
if site_contact:
if isinstance(site_contact, list):
lines.extend([" - Site contacts:"])
Copy link
Member

Choose a reason for hiding this comment

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

drop the Site ?

lines.extend([" - %s" % address for address in site_contact])
Copy link
Member

Choose a reason for hiding this comment

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

s/address/contact/g

else:
lines.extend([" - Site contact: %s" % site_contact])
Copy link
Member

Choose a reason for hiding this comment

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

since the code is copy-paste for upstream_contacts and site_contacts, how about:

for contacts_type in ['upstream', 'site']:
    contacts = self.cfg['%s_contacts' % contacts_type]
    if contacts:
        if isinstance(contacts, list):
            lines.append(" - %s contacts" % contacts_type.capitalize())
            lines.extend(["    - %s" % c for c in contacts])
        else:
            lines.append(" - %s contact: %s" % (contacts_type.capitalize(), contacts))


# Extensions (if any)
extensions = self._generate_extension_list()
lines.extend(generate_section("Included extensions", '\n'.join(wrap(extensions, 78))))

return '\n'.join(lines)

def _generate_whatis_lines(self):
"""
Generate a list of entries used for `module whatis`.
"""
whatis = self.app.cfg['whatis']
if whatis is None:
# default: include 'whatis' statements with description, homepage, and extensions (if any)
whatis = [
"Description: %s" % self.app.cfg['description'],
"Homepage: %s" % self.app.cfg['homepage']
]
extensions = self._generate_extension_list()
if extensions:
whatis.append("Extensions: %s" % extensions)

return whatis

def _generate_extension_list(self):
Copy link
Member

Choose a reason for hiding this comment

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

sort these methods alphabetically, let's not let @damianam's efforts go to waste ;)

"""
Generate a string with a comma-separated list of extensions.
"""
exts_list = self.app.cfg['exts_list']
extensions = ', '.join(sorted(['%s-%s' % (ext[0], ext[1]) for ext in exts_list], key=str.lower))
Copy link
Member

Choose a reason for hiding this comment

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

s/(ext[0], ext[1])/ext[:2]/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeError: not enough arguments for format string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now using '-'.join(ext[:2]) for ext in ... but I'm not sure it helps w.r.t. readablity...


return extensions


class ModuleGeneratorTcl(ModuleGenerator):
"""
Expand Down Expand Up @@ -346,16 +441,9 @@ def get_description(self, conflict=True):
"""
Generate a description.
"""
description = "%s - Homepage: %s" % (self.app.cfg['description'], self.app.cfg['homepage'])

whatis = self.app.cfg['whatis']
if whatis is None:
# default: include single 'whatis' statement with description as contents
whatis = ["Description: %s" % description]

lines = [
"proc ModulesHelp { } {",
" puts stderr { %(description)s",
" puts stderr {%s" % self._generate_help_text(),
" }",
'}',
'',
Expand All @@ -376,10 +464,11 @@ def get_description(self, conflict=True):
# - 'conflict Compiler/GCC/4.8.2/OpenMPI' for 'Compiler/GCC/4.8.2/OpenMPI/1.6.4'
lines.extend(['', "conflict %s" % os.path.dirname(self.app.short_mod_name)])

whatis = self._generate_whatis_lines()
Copy link
Member

Choose a reason for hiding this comment

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

just move this inline, only used in one place?

txt = '\n'.join(lines + ['']) % {
'name': self.app.name,
'version': self.app.version,
'description': description,
'description': self.app.cfg['description'],
'whatis_lines': '\n'.join(["module-whatis {%s}" % line for line in whatis]),
'installdir': self.app.installdir,
}
Expand Down Expand Up @@ -601,16 +690,9 @@ def get_description(self, conflict=True):
"""
Generate a description.
"""

description = "%s - Homepage: %s" % (self.app.cfg['description'], self.app.cfg['homepage'])

whatis = self.app.cfg['whatis']
if whatis is None:
# default: include single 'whatis' statement with description as contents
whatis = ["Description: %s" % description]

lines = [
"help([[%(description)s]])",
'help([[%s' % self._generate_help_text(),
']])',
'',
"%(whatis_lines)s",
'',
Expand All @@ -624,10 +706,11 @@ def get_description(self, conflict=True):
# conflict on 'name' part of module name (excluding version part at the end)
lines.extend(['', 'conflict("%s")' % os.path.dirname(self.app.short_mod_name)])

whatis = self._generate_whatis_lines()
Copy link
Member

Choose a reason for hiding this comment

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

same here, move this inline?

txt = '\n'.join(lines + ['']) % {
'name': self.app.name,
'version': self.app.version,
'description': description,
'description': self.app.cfg['description'],
'whatis_lines': '\n'.join(["whatis([[%s]])" % line for line in whatis]),
'installdir': self.app.installdir,
'homepage': self.app.cfg['homepage'],
Expand Down
52 changes: 46 additions & 6 deletions test/framework/module_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,27 @@ def setUp(self):
def test_descr(self):
"""Test generation of module description (which includes '#%Module' header)."""

gzip_txt = "gzip (GNU zip) is a popular data compression program as a replacement for compress "
gzip_txt += "- Homepage: http://www.gzip.org/"
gzip_txt = "gzip (GNU zip) is a popular data compression program as a replacement for compress"
Copy link
Member

Choose a reason for hiding this comment

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

rename gzip_txt to descr?

homepage = "http://www.gzip.org/"

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
"proc ModulesHelp { } {",
" puts stderr { %s" % gzip_txt,
" puts stderr {",
'',
'Description',
'===========',
"%s" % gzip_txt,
'',
'',
"More information",
"================",
" - Homepage: %s" % homepage,
" }",
"}",
'',
"module-whatis {Description: %s}" % gzip_txt,
"module-whatis {Homepage: %s}" % homepage,
'',
"set root %s" % self.modgen.app.installdir,
'',
Expand All @@ -94,9 +104,20 @@ def test_descr(self):

else:
expected = '\n'.join([
'help([[%s]])' % gzip_txt,
"help([[",
'',
'Description',
'===========',
"%s" % gzip_txt,
'',
'',
"More information",
"================",
" - Homepage: %s" % homepage,
']])',
'',
"whatis([[Description: %s]])" % gzip_txt,
"whatis([[Homepage: %s]])" % homepage,
'',
'local root = "%s"' % self.modgen.app.installdir,
'',
Expand All @@ -112,7 +133,16 @@ def test_descr(self):
if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
"proc ModulesHelp { } {",
" puts stderr { %s" % gzip_txt,
" puts stderr {",
'',
'Description',
'===========',
"%s" % gzip_txt,
'',
'',
"More information",
"================",
" - Homepage: %s" % homepage,
" }",
"}",
'',
Expand All @@ -127,7 +157,17 @@ def test_descr(self):

else:
expected = '\n'.join([
'help([[%s]])' % gzip_txt,
"help([[",
'',
'Description',
'===========',
"%s" % gzip_txt,
'',
'',
"More information",
"================",
" - Homepage: %s" % homepage,
']])',
'',
"whatis([[foo]])",
"whatis([[bar]])",
Expand Down
51 changes: 46 additions & 5 deletions test/framework/toy_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,13 @@ def test_toy_tweaked(self):
"modextravars = {'FOO': 'bar'}",
"modloadmsg = '%s'" % modloadmsg,
"modtclfooter = 'puts stderr \"oh hai!\"'", # ignored when module syntax is Lua
"modluafooter = 'io.stderr:write(\"oh hai!\")'" # ignored when module syntax is Tcl
"modluafooter = 'io.stderr:write(\"oh hai!\")'", # ignored when module syntax is Tcl
"usage = 'This toy is easy to use'",
"examples = 'No example available'",
"docpaths = ['share/doc/toy/readme.txt', 'share/doc/toy/html/index.html']",
"docurls = ['http://hpcugent.github.com/easybuild/toy/docs.html']",
"upstream_contact = 'support@toy.org'",
"site_contact = ['Jim Admin', 'Jane Admin']",
])
write_file(ec_file, ec_extra, append=True)

Expand Down Expand Up @@ -945,11 +951,43 @@ def test_toy_module_fulltxt(self):
r' I AM toy v0.0\]==\]\)',
]

help_txt = '\n'.join([
r'Description',
r'===========',
r'Toy C program.',
r'',
r'',
r'Usage',
r'=====',
r'This toy is easy to use',
r'',
r'',
r'Examples',
r'========',
r'No example available',
r'',
r'',
r'More information',
r'================',
r' - Homepage: http://hpcugent.github.com/easybuild',
r' - Documentation:',
r' - \$EBROOTTOY/share/doc/toy/readme.txt',
r' - \$EBROOTTOY/share/doc/toy/html/index.html',
r' - http://hpcugent.github.com/easybuild/toy/docs.html',
r' - Upstream contact: support@toy.org',
r' - Site contacts:',
r' - Jim Admin',
r' - Jane Admin',
])
if get_module_syntax() == 'Lua':
mod_txt_regex_pattern = '\n'.join([
r'help\(\[\[Toy C program. - Homepage: http://hpcugent.github.com/easybuild\]\]\)',
r'help\(\[\[',
r'',
r'%s' % help_txt,
r'\]\]\)',
r'',
r'whatis\(\[\[Description: Toy C program. - Homepage: http://hpcugent.github.com/easybuild\]\]\)',
r'whatis\(\[\[Description: Toy C program.\]\]\)',
r'whatis\(\[\[Homepage: http://hpcugent.github.com/easybuild\]\]\)',
r'',
r'local root = "%s/software/toy/0.0-tweaked"' % self.test_installpath,
r'',
Expand Down Expand Up @@ -978,11 +1016,14 @@ def test_toy_module_fulltxt(self):
mod_txt_regex_pattern = '\n'.join([
r'^#%Module',
r'proc ModulesHelp { } {',
r' puts stderr { Toy C program. - Homepage: http://hpcugent.github.com/easybuild',
r' puts stderr {',
r'',
r'%s' % help_txt,
r' }',
r'}',
r'',
r'module-whatis {Description: Toy C program. - Homepage: http://hpcugent.github.com/easybuild}',
r'module-whatis {Description: Toy C program.}',
r'module-whatis {Homepage: http://hpcugent.github.com/easybuild}',
r'',
r'set root %s/software/toy/0.0-tweaked' % self.test_installpath,
r'',
Expand Down