check for correct version values when parsing easystack file

easybuild/framework/easystack.py

@@ -27,12 +27,13 @@
 
 :author: Denis Kristak (Inuits)
 :author: Pavel Grochal (Inuits)
+:author: Kenneth Hoste (HPC-UGent)
 """
-
 from easybuild.base import fancylogger
 from easybuild.tools.build_log import EasyBuildError
 from easybuild.tools.filetools import read_file
 from easybuild.tools.module_naming_scheme.utilities import det_full_ec_version
+from easybuild.tools.py2vs3 import string_type
 from easybuild.tools.utilities import only_if_module_is_available
 try:
     import yaml
@@ -41,6 +42,25 @@
 _log = fancylogger.getLogger('easystack', fname=False)
 
 
+def check_value(value, context):
+    """
+    Check whether specified value obtained from a YAML file in specified context represents is valid.
+    The value must be a string (not a float or an int).
+    """
+    if not isinstance(value, string_type):
+        error_msg = '\n'.join([
+            "Value %(value)s (of type %(type)s) obtained for %(context)s is not valid!",
+            "Make sure to wrap the value in single quotes (like '%(value)s') to avoid that it is interpreted "
+            "by the YAML parser as a non-string value.",
+        ])
+        format_info = {
+            'context': context,
+            'type': type(value),
+            'value': value,
+        }
+        raise EasyBuildError(error_msg % format_info)
+
+
 class EasyStack(object):
     """One class instance per easystack. General options + list of all SoftwareSpecs instances"""
 
@@ -90,7 +110,12 @@ class EasyStackParser(object):
     def parse(filepath):
         """Parses YAML file and assigns obtained values to SW config instances as well as general config instance"""
         yaml_txt = read_file(filepath)
-        easystack_raw = yaml.safe_load(yaml_txt)
+
+        try:
+            easystack_raw = yaml.safe_load(yaml_txt)
+        except yaml.YAMLError as err:
+            raise EasyBuildError("Failed to parse %s: %s" % (filepath, err))
+
         easystack = EasyStack()
 
         try:
@@ -103,13 +128,13 @@ def parse(filepath):
         for name in software:
             # ensure we have a string value (YAML parser returns type = dict
             # if levels under the current attribute are present)
-            name = str(name)
+            check_value(name, "software name")
             try:
                 toolchains = software[name]['toolchains']
             except KeyError:
                 raise EasyBuildError("Toolchains for software '%s' are not defined in %s", name, filepath)
             for toolchain in toolchains:
-                toolchain = str(toolchain)
+                check_value(toolchain, "software %s" % name)
 
                 if toolchain == 'SYSTEM':
                     toolchain_name, toolchain_version = 'system', ''
@@ -140,12 +165,13 @@ def parse(filepath):
                 # Example of yaml structure:
                 # ========================================================================
                 # versions:
-                #   2.25:
-                #   2.23:
+                #   '2.25':
+                #   '2.23':
                 #     versionsuffix: '-R-4.0.0'
                 # ========================================================================
                 if isinstance(versions, dict):
                     for version in versions:
+                        check_value(version, "%s (with %s toolchain)" % (name, toolchain_name))
                         if versions[version] is not None:
                             version_spec = versions[version]
                             if 'versionsuffix' in version_spec:
@@ -172,35 +198,25 @@ def parse(filepath):
                         easystack.software_list.append(sw)
                     continue
 
-                # is format read as a list of versions?
-                # ========================================================================
-                # versions:
-                #   [2.24, 2.51]
-                # ========================================================================
-                elif isinstance(versions, list):
-                    versions_list = versions
+                elif isinstance(versions, (list, tuple)):
+                    pass
 
-                # format = multiple lines without ':' (read as a string)?
-                # ========================================================================
+                # multiple lines without ':' is read as a single string; example:
                 # versions:
-                #   2.24
-                #   2.51
-                # ========================================================================
-                elif isinstance(versions, str):
-                    versions_list = str(versions).split()
+                #   '2.24'
+                #   '2.51'
+                elif isinstance(versions, string_type):
+                    versions = versions.split()
 
-                # format read as float (containing one version only)?
-                # ========================================================================
-                # versions:
-                #   2.24
-                # ========================================================================
-                elif isinstance(versions, float):
-                    versions_list = [str(versions)]
+                # single values like '2.24' should be wrapped in a list
+                else:
+                    versions = [versions]
 
-                # if no version is a dictionary, versionsuffix isn't specified
+                # if version is not a dictionary, versionsuffix is not specified
                 versionsuffix = ''
 
-                for version in versions_list:
+                for version in versions:
+                    check_value(version, "%s (with %s toolchain)" % (name, toolchain_name))
                     sw = SoftwareSpecs(
                         name=name, version=version, versionsuffix=versionsuffix,
                         toolchain_name=toolchain_name, toolchain_version=toolchain_version)

test/framework/easystack.py

@@ -0,0 +1,207 @@
+# #
+# Copyright 2013-2021 Ghent University
+#
+# This file is part of EasyBuild,
+# originally created by the HPC team of Ghent University (http://ugent.be/hpc/en),
+# with support of Ghent University (http://ugent.be/hpc),
+# the Flemish Supercomputer Centre (VSC) (https://www.vscentrum.be),
+# Flemish Research Foundation (FWO) (http://www.fwo.be/en)
+# and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en).
+#
+# https://github.com/easybuilders/easybuild
+#
+# EasyBuild is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation v2.
+#
+# EasyBuild is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with EasyBuild.  If not, see <http://www.gnu.org/licenses/>.
+# #
+"""
+Unit tests for easystack files
+
+@author: Denis Kristak (Inuits)
+@author: Kenneth Hoste (Ghent University)
+"""
+import os
+import sys
+from unittest import TextTestRunner
+
+import easybuild.tools.build_log
+from easybuild.framework.easystack import check_value, parse_easystack
+from easybuild.tools.build_log import EasyBuildError
+from easybuild.tools.filetools import write_file
+from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered
+
+
+class EasyStackTest(EnhancedTestCase):
+    """Testcases for easystack files."""
+
+    logfile = None
+
+    def setUp(self):
+        """Set up test."""
+        super(EasyStackTest, self).setUp()
+        self.orig_experimental = easybuild.tools.build_log.EXPERIMENTAL
+        # easystack files are an experimental feature
+        easybuild.tools.build_log.EXPERIMENTAL = True
+
+    def tearDown(self):
+        """Clean up after test."""
+        easybuild.tools.build_log.EXPERIMENTAL = self.orig_experimental
+        super(EasyStackTest, self).tearDown()
+
+    def test_parse_fail(self):
+        """Test for clean error when easystack file fails to parse."""
+        test_yml = os.path.join(self.test_prefix, 'test.yml')
+        write_file(test_yml, 'software: %s')
+        error_pattern = "Failed to parse .*/test.yml: while scanning for the next token"
+        self.assertErrorRegex(EasyBuildError, error_pattern, parse_easystack, test_yml)
+
+    def test_easystack_wrong_structure(self):
+        """Test for --easystack <easystack.yaml> when yaml easystack has wrong structure"""
+        topdir = os.path.dirname(os.path.abspath(__file__))
+        test_easystack = os.path.join(topdir, 'easystacks', 'test_easystack_wrong_structure.yaml')
+
+        expected_err = r"[\S\s]*An error occurred when interpreting the data for software Bioconductor:"
+        expected_err += r"( 'float' object is not subscriptable[\S\s]*"
+        expected_err += r"| 'float' object is unsubscriptable"
+        expected_err += r"| 'float' object has no attribute '__getitem__'[\S\s]*)"
+        self.assertErrorRegex(EasyBuildError, expected_err, parse_easystack, test_easystack)
+
+    def test_easystack_asterisk(self):
+        """Test for --easystack <easystack.yaml> when yaml easystack contains asterisk (wildcard)"""
+        topdir = os.path.dirname(os.path.abspath(__file__))
+        test_easystack = os.path.join(topdir, 'easystacks', 'test_easystack_asterisk.yaml')
+
+        expected_err = "EasyStack specifications of 'binutils' in .*/test_easystack_asterisk.yaml contain asterisk. "
+        expected_err += "Wildcard feature is not supported yet."
+
+        self.assertErrorRegex(EasyBuildError, expected_err, parse_easystack, test_easystack)
+
+    def test_easystack_labels(self):
+        topdir = os.path.dirname(os.path.abspath(__file__))
+        test_easystack = os.path.join(topdir, 'easystacks', 'test_easystack_labels.yaml')
+
+        error_msg = "EasyStack specifications of 'binutils' in .*/test_easystack_labels.yaml contain labels. "
+        error_msg += "Labels aren't supported yet."
+        self.assertErrorRegex(EasyBuildError, error_msg, parse_easystack, test_easystack)
+
+    def test_check_value(self):
+        """Test check_value function."""
+        check_value('1.2.3', None)
+        check_value('1.2', None)
+        check_value('3.50', None)
+        check_value('100', None)
+
+        context = "<some context>"
+        for version in (1.2, 100, None):
+            error_pattern = r"Value .* \(of type .*\) obtained for <some context> is not valid!"
+            self.assertErrorRegex(EasyBuildError, error_pattern, check_value, version, context)
+
+    def test_easystack_versions(self):
+        """Test handling of versions in easystack files."""
+
+        test_easystack = os.path.join(self.test_prefix, 'test.yml')
+        tmpl_easystack_txt = '\n'.join([
+            "software:",
+            "  foo:",
+            "    toolchains:",
+            "       SYSTEM:",
+            "           versions:",
+        ])
+
+        # normal versions, which are not treated special by YAML: no single quotes needed
+        versions = ('1.2.3', '1.2.30', '2021a', '1.2.3')
+        for version in versions:
+            write_file(test_easystack, tmpl_easystack_txt + ' ' + version)
+            ec_fns, _ = parse_easystack(test_easystack)
+            self.assertEqual(ec_fns, ['foo-%s.eb' % version])
+
+        # multiple versions as a list
+        test_easystack_txt = tmpl_easystack_txt + " [1.2.3, 3.2.1]"
+        write_file(test_easystack, test_easystack_txt)
+        ec_fns, _ = parse_easystack(test_easystack)
+        expected = ['foo-1.2.3.eb', 'foo-3.2.1.eb']
+        self.assertEqual(sorted(ec_fns), sorted(expected))
+
+        # multiple versions listed with more info
+        test_easystack_txt = '\n'.join([
+            tmpl_easystack_txt,
+            "             1.2.3:",
+            "             2021a:",
+            "             3.2.1:",
+            "                 versionsuffix: -foo",
+        ])
+        write_file(test_easystack, test_easystack_txt)
+        ec_fns, _ = parse_easystack(test_easystack)
+        expected = ['foo-1.2.3.eb', 'foo-2021a.eb', 'foo-3.2.1-foo.eb']
+        self.assertEqual(sorted(ec_fns), sorted(expected))
+
+        # versions that get interpreted by YAML as float or int, single quotes required
+        for version in ('1.2', '123', '3.50', '100', '2.44_01'):
+            error_pattern = r"Value .* \(of type .*\) obtained for foo \(with system toolchain\) is not valid\!"
+
+            write_file(test_easystack, tmpl_easystack_txt + ' ' + version)
+            self.assertErrorRegex(EasyBuildError, error_pattern, parse_easystack, test_easystack)
+
+            # all is fine when wrapping the value in single quotes
+            write_file(test_easystack, tmpl_easystack_txt + " '" + version + "'")
+            ec_fns, _ = parse_easystack(test_easystack)
+            self.assertEqual(ec_fns, ['foo-%s.eb' % version])
+
+            # one rotten apple in the basket is enough
+            test_easystack_txt = tmpl_easystack_txt + " [1.2.3, %s, 3.2.1]" % version
+            write_file(test_easystack, test_easystack_txt)
+            self.assertErrorRegex(EasyBuildError, error_pattern, parse_easystack, test_easystack)
+
+            test_easystack_txt = '\n'.join([
+                tmpl_easystack_txt,
+                "             1.2.3:",
+                "             %s:" % version,
+                "             3.2.1:",
+                "                 versionsuffix: -foo",
+            ])
+            write_file(test_easystack, test_easystack_txt)
+            self.assertErrorRegex(EasyBuildError, error_pattern, parse_easystack, test_easystack)
+
+            # single quotes to the rescue!
+            test_easystack_txt = '\n'.join([
+                tmpl_easystack_txt,
+                "             1.2.3:",
+                "             '%s':" % version,
+                "             3.2.1:",
+                "                 versionsuffix: -foo",
+            ])
+            write_file(test_easystack, test_easystack_txt)
+            ec_fns, _ = parse_easystack(test_easystack)
+            expected = ['foo-1.2.3.eb', 'foo-%s.eb' % version, 'foo-3.2.1-foo.eb']
+            self.assertEqual(sorted(ec_fns), sorted(expected))
+
+        # also check toolchain version that could be interpreted as a non-string value...
+        test_easystack_txt = '\n'.join([
+            'software:',
+            '  test:',
+            '    toolchains:',
+            '      intel-2021.03:',
+            "        versions: [1.2.3, '2.3']",
+        ])
+        write_file(test_easystack, test_easystack_txt)
+        ec_fns, _ = parse_easystack(test_easystack)
+        expected = ['test-1.2.3-intel-2021.03.eb', 'test-2.3-intel-2021.03.eb']
+        self.assertEqual(sorted(ec_fns), sorted(expected))
+
+
+def suite():
+    """ returns all the testcases in this module """
+    return TestLoaderFiltered().loadTestsFromTestCase(EasyStackTest, sys.argv[1:])
+
+
+if __name__ == '__main__':
+    res = TextTestRunner(verbosity=1).run(suite())
+    sys.exit(len(res.failures))

test/framework/easystacks/test_easystack_basic.yaml

@@ -3,8 +3,8 @@ software:
     toolchains:
       GCCcore-4.9.3:
         versions:
-          2.25:
-          2.26:
+          '2.25':
+          '2.26':
   foss:
     toolchains:
       SYSTEM:
@@ -13,5 +13,5 @@ software:
     toolchains:
       gompi-2018a:
         versions:
-          0.0:
+          '0.0':
             versionsuffix: '-test'

test/framework/easystacks/test_easystack_labels.yaml

@@ -3,5 +3,5 @@ software:
     toolchains:
       GCCcore-4.9.3:
         versions:
-            3.11:
+            '3.11':
               exclude-labels: arch:aarch64