Merge pull request #1 from boegel/checksum_comment

extend enhanced `--inject-checksums` to extensions + enhance test

easybuild/framework/easyblock.py

@@ -58,6 +58,7 @@
 from easybuild.framework.easyconfig.easyconfig import get_module_path, letter_dir_for, resolve_template
 from easybuild.framework.easyconfig.format.format import INDENT_4SPACES
 from easybuild.framework.easyconfig.parser import fetch_parameters_from_easyconfig
+from easybuild.framework.easyconfig.style import MAX_LINE_LENGTH
 from easybuild.framework.easyconfig.tools import get_paths_for
 from easybuild.framework.easyconfig.templates import TEMPLATE_NAMES_EASYBLOCK_RUN_STEP
 from easybuild.tools.build_details import get_build_stats
@@ -2955,9 +2956,11 @@ def inject_checksums(ecs, checksum_type):
             checksum_lines = ['checksums = [']
             for fn, checksum in checksums:
                 check_line = "%s'%s',  # %s" % (INDENT_4SPACES, checksum, fn)
-                if len(check_line) > 120:
-                    checksum_lines.append("%s# %s" % (INDENT_4SPACES, fn))
-                    checksum_lines.append("%s'%s'," % (INDENT_4SPACES, checksum))
+                if len(check_line) > MAX_LINE_LENGTH:
+                    checksum_lines.extend([
+                        "%s# %s" % (INDENT_4SPACES, fn),
+                        "%s'%s'," % (INDENT_4SPACES, checksum),
+                    ])
                 else:
                     checksum_lines.append(check_line)
             checksum_lines.append(']\n')
@@ -3050,7 +3053,15 @@ def inject_checksums(ecs, checksum_type):
                         else:
                             exts_list_lines.append("%s'checksums': [" % (INDENT_4SPACES * 2))
                             for fn, checksum in ext_checksums:
-                                exts_list_lines.append("%s'%s',  # %s" % (INDENT_4SPACES * 3, checksum, fn))
+                                line_indent = INDENT_4SPACES * 3
+                                ext_checksum_line = "%s'%s',  # %s" % (line_indent, checksum, fn)
+                                if len(ext_checksum_line) > MAX_LINE_LENGTH:
+                                    exts_list_lines.extend([
+                                        "%s# %s" % (line_indent, fn),
+                                        "%s'%s'," % (line_indent, checksum),
+                                    ])
+                                else:
+                                    exts_list_lines.append(ext_checksum_line)
                             exts_list_lines.append("%s]," % (INDENT_4SPACES * 2))
 
                     if ext_options or ext_checksums:

easybuild/framework/easyconfig/style.py

@@ -53,6 +53,8 @@
 COMMENT_REGEX = re.compile(r'^\s*#')
 PARAM_DEF_REGEX = re.compile(r"^(?P<key>[a-z_]+)\s*=\s*")
 
+MAX_LINE_LENGTH = 120
+
 
 # Any function starting with _eb_check_ (see EB_CHECK variable) will be
 # added to the tests if the test number is added to the select list.
@@ -126,7 +128,7 @@ def check_easyconfigs_style(easyconfigs, verbose=False):
     options = styleguide.options
     # we deviate from standard pep8 and allow 120 chars
     # on a line: the default of 79 is too narrow.
-    options.max_line_length = 120
+    options.max_line_length = MAX_LINE_LENGTH
     # we ignore some tests
     # note that W291 has been replaced by our custom W299
     options.ignore = (

test/framework/easyblock.py

@@ -847,27 +847,28 @@ def test_fetch_patches(self):
 
         eb.fetch_patches()
         self.assertEqual(len(eb.patches), 2)
-        self.assertEqual(eb.patches[0]['name'], 'toy-0.0_typo.patch')
+        self.assertEqual(eb.patches[0]['name'], 'toy-0.0_fix-silly-typo-in-printf-statement.patch')
         self.assertFalse('level' in eb.patches[0])
 
         # reset
         eb.patches = []
 
+        toy_patch = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
         patches = [
-            ('toy-0.0_typo.patch', 0),  # should also be level 0 (not None or something else)
-            ('toy-0.0_typo.patch', 4),   # should be level 4
-            ('toy-0.0_typo.patch', 'foobar'),  # sourcepath should be set to 'foobar'
+            (toy_patch, 0),  # should also be level 0 (not None or something else)
+            (toy_patch, 4),   # should be level 4
+            (toy_patch, 'foobar'),  # sourcepath should be set to 'foobar'
             ('toy-0.0.tar.gz', 'some/path'),  # copy mode (not a .patch file)
         ]
         # check if patch levels are parsed correctly
         eb.fetch_patches(patch_specs=patches)
 
         self.assertEqual(len(eb.patches), 4)
-        self.assertEqual(eb.patches[0]['name'], 'toy-0.0_typo.patch')
+        self.assertEqual(eb.patches[0]['name'], toy_patch)
         self.assertEqual(eb.patches[0]['level'], 0)
-        self.assertEqual(eb.patches[1]['name'], 'toy-0.0_typo.patch')
+        self.assertEqual(eb.patches[1]['name'], toy_patch)
         self.assertEqual(eb.patches[1]['level'], 4)
-        self.assertEqual(eb.patches[2]['name'], 'toy-0.0_typo.patch')
+        self.assertEqual(eb.patches[2]['name'], toy_patch)
         self.assertEqual(eb.patches[2]['sourcepath'], 'foobar')
         self.assertEqual(eb.patches[3]['name'], 'toy-0.0.tar.gz'),
         self.assertEqual(eb.patches[3]['copy'], 'some/path')
@@ -1061,7 +1062,7 @@ def test_patch_step(self):
         orig_sources = ec['ec']['sources'][:]
 
         toy_patches = [
-            'toy-0.0_typo.patch',  # test for applying patch
+            'toy-0.0_fix-silly-typo-in-printf-statement.patch',  # test for applying patch
             ('toy-extra.txt', 'toy-0.0'), # test for patch-by-copy
         ]
         self.assertEqual(ec['ec']['patches'], toy_patches)

test/framework/easyconfig.py

@@ -323,7 +323,7 @@ def test_exts_list(self):
                         # SHA256 checksum for source (gzip-1.4.eb)
             '           "6f281b6d7a3965476324a23b9d80232bd4ffe3967da85e4b7c01d9d81d649a09",',
                         # SHA256 checksum for 'patch' (toy-0.0.eb)
-            '           "044e300a051120defb01c14c7c06e9aa4bca40c5d589828df360e2684dcc9074",',
+            '           "a79ba0ef5dceb5b8829268247feae8932bed2034c6628ff1d92c84bf45e9a546",',
             '       ],',
             '   }),',
             ']',
@@ -344,7 +344,7 @@ def test_exts_list(self):
         self.assertEqual(exts_sources[1]['version'], '2.0')
         self.assertEqual(exts_sources[1]['options'], {
             'checksums': ['6f281b6d7a3965476324a23b9d80232bd4ffe3967da85e4b7c01d9d81d649a09',
-                          '044e300a051120defb01c14c7c06e9aa4bca40c5d589828df360e2684dcc9074'],
+                          'a79ba0ef5dceb5b8829268247feae8932bed2034c6628ff1d92c84bf45e9a546'],
             'patches': ['toy-0.0.eb'],
             'source_tmpl': 'gzip-1.4.eb',
             'source_urls': [('http://example.com', 'suffix')],
@@ -1349,7 +1349,8 @@ def test_update(self):
 
         # for list values: extend
         ec.update('patches', ['foo.patch', 'bar.patch'])
-        self.assertEqual(ec['patches'], ['toy-0.0_typo.patch', ('toy-extra.txt', 'toy-0.0'), 'foo.patch', 'bar.patch'])
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        self.assertEqual(ec['patches'], [toy_patch_fn, ('toy-extra.txt', 'toy-0.0'), 'foo.patch', 'bar.patch'])
 
     def test_hide_hidden_deps(self):
         """Test use of --hide-deps on hiddendependencies."""
@@ -2009,7 +2010,8 @@ def test_categorize_files_by_type(self):
         self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': []}, categorize_files_by_type([]))
 
         test_ecs_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs',)
-        toy_patch = os.path.join(os.path.dirname(test_ecs_dir), 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        toy_patch = os.path.join(os.path.dirname(test_ecs_dir), 'sandbox', 'sources', 'toy', toy_patch_fn)
         paths = [
             'bzip2-1.0.6.eb',
             os.path.join(test_ecs_dir, 'test_ecs', 'g', 'gzip', 'gzip-1.4.eb'),

test/framework/easyconfigs/test_ecs/t/toy/toy-0.0-deps.eb

@@ -17,7 +17,7 @@ checksums = [[
     ('sha1', 'f618096c52244539d0e89867405f573fdb0b55b0'),
     ('size', 273),
 ]]
-patches = ['toy-0.0_typo.patch']
+patches = ['toy-0.0_fix-silly-typo-in-printf-statement.patch']
 
 dependencies = [
     ('ictce', '4.1.13', '', True),

test/framework/easyconfigs/test_ecs/t/toy/toy-0.0-gompi-1.3.12-test.eb

@@ -18,7 +18,7 @@ checksums = [[
     ('sha1', 'f618096c52244539d0e89867405f573fdb0b55b0'),
     ('size', 273),
 ]]
-patches = ['toy-0.0_typo.patch']
+patches = ['toy-0.0_fix-silly-typo-in-printf-statement.patch']
 
 exts_default_options = {
     'source_urls': ['http://example.com/%(name)s'],
@@ -30,7 +30,7 @@ exts_list = [
         'checksums': ['f3676716b610545a4e8035087f5be0a0248adee0abb3930d3edb76d498ae91e7'],  # checksum for 
         # custom extension filter to verify use of stdin value being passed to filter command
     'exts_filter': ("cat | grep '^bar$'", '%(name)s'),
-        'patches': ['bar-0.0_typo.patch'],
+        'patches': ['bar-0.0_fix-silly-typo-in-printf-statement.patch'],
         'toy_ext_param': "mv anotherbar bar_bis",
         'unknowneasyconfigparameterthatshouldbeignored': 'foo',
     }),

test/framework/easyconfigs/test_ecs/t/toy/toy-0.0-iter.eb

@@ -8,7 +8,7 @@ description = "Toy C program, 100% toy."
 toolchain = {'name': 'dummy', 'version': 'dummy'}
 
 sources = [SOURCE_TAR_GZ]
-patches = ['toy-0.0_typo.patch']
+patches = ['toy-0.0_fix-silly-typo-in-printf-statement.patch']
 
 buildopts = [
     '',

test/framework/easyconfigs/test_ecs/t/toy/toy-0.0-multiple.eb

@@ -9,7 +9,7 @@ description = "Toy C program, 100% toy."
 toolchain = {'name': 'dummy', 'version': 'dummy'}
 
 sources = [SOURCE_TAR_GZ]
-patches = ['toy-0.0_typo.patch']
+patches = ['toy-0.0_fix-silly-typo-in-printf-statement.patch']
 checksums = [
     ('adler32', '0x998410035'),
     'e6785e1a721fc8bf79892e3ef41557c0',

test/framework/easyconfigs/test_ecs/t/toy/toy-0.0.eb

@@ -17,7 +17,7 @@ checksums = [[
     ('size', 273),
 ]]
 patches = [
-    'toy-0.0_typo.patch',
+    'toy-0.0_fix-silly-typo-in-printf-statement.patch',
     ('toy-extra.txt', 'toy-0.0'),
 ]
 

test/framework/easyconfigs/yeb/toy-0.0.yeb

@@ -23,7 +23,7 @@ checksums: [[
 ]]
 
 patches: [
-    'toy-0.0_typo.patch',
+    'toy-0.0_fix-silly-typo-in-printf-statement.patch',
     ['toy-extra.txt', 'toy-0.0'],
 ]
 

test/framework/filetools.py

@@ -535,7 +535,8 @@ def test_read_write_file(self):
 
     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')
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        pf = os.path.join(os.path.dirname(__file__), 'sandbox', 'sources', 'toy', toy_patch_fn)
         self.assertEqual(ft.det_patched_files(pf), ['b/toy-0.0/toy.source'])
         self.assertEqual(ft.det_patched_files(pf, omit_ab_prefix=True), ['toy-0.0/toy.source'])
 
@@ -1083,7 +1084,8 @@ def test_is_patch_file(self):
         """Test for is_patch_file() function."""
         testdir = os.path.dirname(os.path.abspath(__file__))
         self.assertFalse(ft.is_patch_file(os.path.join(testdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb')))
-        self.assertTrue(ft.is_patch_file(os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')))
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        self.assertTrue(ft.is_patch_file(os.path.join(testdir, 'sandbox', 'sources', 'toy', toy_patch_fn)))
 
     def test_is_alt_pypi_url(self):
         """Test is_alt_pypi_url() function."""
@@ -1133,7 +1135,8 @@ def test_apply_patch(self):
         testdir = os.path.dirname(os.path.abspath(__file__))
         tmpdir = self.test_prefix
         path = ft.extract_file(os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0.tar.gz'), tmpdir)
-        toy_patch = os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        toy_patch = os.path.join(testdir, 'sandbox', 'sources', 'toy', toy_patch_fn)
 
         self.assertTrue(ft.apply_patch(toy_patch, path))
         patched = ft.read_file(os.path.join(path, 'toy-0.0', 'toy.source'))
@@ -1251,13 +1254,14 @@ def test_copy(self):
         testdir = os.path.dirname(os.path.abspath(__file__))
 
         toy_file = os.path.join(testdir, 'easyconfigs', 'test_ecs', 't', 'toy', 'toy-0.0.eb')
-        toy_patch = os.path.join(testdir, 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        toy_patch = os.path.join(testdir, 'sandbox', 'sources', 'toy', toy_patch_fn)
         gcc_dir = os.path.join(testdir, 'easyconfigs', 'test_ecs', 'g', 'GCC')
 
         ft.copy([toy_file, gcc_dir, toy_patch], self.test_prefix)
 
         self.assertTrue(os.path.isdir(os.path.join(self.test_prefix, 'GCC')))
-        for filepath in ['GCC/GCC-4.6.3.eb', 'GCC/GCC-4.9.2.eb', 'toy-0.0.eb', 'toy-0.0_typo.patch']:
+        for filepath in ['GCC/GCC-4.6.3.eb', 'GCC/GCC-4.9.2.eb', 'toy-0.0.eb', toy_patch_fn]:
             self.assertTrue(os.path.isfile(os.path.join(self.test_prefix, filepath)))
 
         # test copying of a single file, to a non-existing directory

test/framework/github.py

@@ -270,7 +270,7 @@ def test_find_patches(self):
             'validate': False,
         })
         self.mock_stdout(True)
-        ec = gh.find_software_name_for_patch('toy-0.0_typo.patch')
+        ec = gh.find_software_name_for_patch('toy-0.0_fix-silly-typo-in-printf-statement.patch')
         txt = self.get_stdout()
         self.mock_stdout(False)
 

test/framework/options.py

@@ -2441,7 +2441,8 @@ def test_new_update_pr(self):
         topdir = os.path.dirname(os.path.abspath(__file__))
         test_ecs = os.path.join(topdir, 'easyconfigs', 'test_ecs')
         toy_ec = os.path.join(self.test_prefix, 'toy.eb')
-        toy_patch = os.path.join(topdir, 'sandbox', 'sources', 'toy', 'toy-0.0_typo.patch')
+        toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
+        toy_patch = os.path.join(topdir, 'sandbox', 'sources', 'toy', toy_patch_fn)
         # purposely picked one with non-default toolchain/versionsuffix
         copy_file(os.path.join(test_ecs, 't', 'toy', 'toy-0.0-gompi-1.3.12-test.eb'), toy_ec)
 
@@ -2515,7 +2516,7 @@ def test_new_update_pr(self):
         regexs[-1] = r"^\s*2 files changed"
         regexs.remove(r"^\* title: \"\{tools\}\[gompi/1.3.12\] toy v0.0\"")
         regexs.append(r"^\* title: \"just a test\"")
-        regexs.append(r".*/toy-0.0_typo.patch\s*\|")
+        regexs.append(r".*_fix-silly-typo-in-printf-statement.patch\s*\|")
         for regex in regexs:
             regex = re.compile(regex, re.M)
             self.assertTrue(regex.search(txt), "Pattern '%s' found in: %s" % (regex.pattern, txt))
@@ -2546,7 +2547,7 @@ def test_new_update_pr(self):
             r"^\* title: \"test-1-2-3\"",
             r"^\* overview of changes:",
             r".*/toy-0.0-gompi-1.3.12-test.eb\s*\|",
-            r".*/toy-0.0_typo.patch\s*\|",
+            r".*_fix-silly-typo-in-printf-statement.patch\s*\|",
             r"^\s*2 files changed",
         ]
         for regex in regexs:
@@ -3385,16 +3386,19 @@ def test_inject_checksums(self):
 
         toy_source_sha256 = '44332000aa33b99ad1e00cbd1a7da769220d74647060a10e807b916d73ea27bc'
         toy_patch_sha256 = '45b5e3f9f495366830e1869bb2b8f4e7c28022739ce48d9f9ebb159b439823c5'
+        bar_tar_gz_sha256 = 'f3676716b610545a4e8035087f5be0a0248adee0abb3930d3edb76d498ae91e7'
+        bar_patch = 'bar-0.0_fix-silly-typo-in-printf-statement.patch'
+        bar_patch_sha256 = '84db53592e882b5af077976257f9c7537ed971cb2059003fd4faa05d02cae0ab'
         patterns = [
             "^== injecting sha256 checksums in .*/test\.eb$",
             "^== fetching sources & patches for test\.eb\.\.\.$",
             "^== backup of easyconfig file saved to .*/test\.eb\.bak_[0-9]+\.\.\.$",
             "^== injecting sha256 checksums for sources & patches in test\.eb\.\.\.$",
             "^== \* toy-0.0\.tar\.gz: %s$" % toy_source_sha256,
-            "^== \* toy-0\.0_typo\.patch: %s$" % toy_patch_sha256,
+            "^== \* toy-0\.0_fix-silly-typo-in-printf-statement\.patch: %s$" % toy_patch_sha256,
             "^== injecting sha256 checksums for extensions in test\.eb\.\.\.$",
-            "^==  \* bar-0\.0\.tar\.gz: f3676716b610545a4e8035087f5be0a0248adee0abb3930d3edb76d498ae91e7$",
-            "^==  \* bar-0\.0_typo\.patch: 84db53592e882b5af077976257f9c7537ed971cb2059003fd4faa05d02cae0ab$",
+            "^==  \* bar-0\.0\.tar\.gz: %s$" % bar_tar_gz_sha256,
+            "^==  \* %s: %s$" % (bar_patch, bar_patch_sha256),
             "^==  \* barbar-0\.0\.tar\.gz: a33100d1837d6d54edff7d19f195056c4bd9a4c8d399e72feaf90f0216c4c91c$",
         ]
         for pattern in patterns:
@@ -3409,6 +3413,15 @@ def test_inject_checksums(self):
         # some checks on 'raw' easyconfig contents
         # single-line checksum for barbar extension since there's only one
         self.assertTrue("'checksums': ['a33100d1837d6d54edff7d19f195056c4bd9a4c8d399e72feaf90f0216c4c91c']," in ec_txt)
+
+        # single-line checksum entry for bar source tarball
+        regex = re.compile("^[ ]*'%s',  # bar-0.0.tar.gz$" % bar_tar_gz_sha256, re.M)
+        self.assertTrue(regex.search(ec_txt), "Pattern '%s' found in: %s" % (regex.pattern, ec_txt))
+
+        # no single-line checksum entry for bar*.patch, since line would be > 120 chars
+        regex = re.compile("^[ ]*# %s\n[ ]*'%s',$" % (bar_patch, bar_patch_sha256), re.M)
+        self.assertTrue(regex.search(ec_txt), "Pattern '%s' found in: %s" % (regex.pattern, ec_txt))
+
         # name/version of toy should NOT be hardcoded in exts_list, 'name'/'version' parameters should be used
         self.assertTrue('    (name, version, {' in ec_txt)
 
@@ -3424,17 +3437,17 @@ def test_inject_checksums(self):
         # no parse errors for updated easyconfig file...
         ec = EasyConfigParser(test_ec).get_config_dict()
         self.assertEqual(ec['sources'], ['%(name)s-%(version)s.tar.gz'])
-        self.assertEqual(ec['patches'], ['toy-0.0_typo.patch'])
+        self.assertEqual(ec['patches'], ['toy-0.0_fix-silly-typo-in-printf-statement.patch'])
         self.assertEqual(ec['checksums'], [toy_source_sha256, toy_patch_sha256])
         self.assertEqual(ec['exts_default_options'], {'source_urls': ['http://example.com/%(name)s']})
         self.assertEqual(ec['exts_list'][0], ('bar', '0.0', {
             'buildopts': " && gcc bar.c -o anotherbar",
             'checksums': [
-                'f3676716b610545a4e8035087f5be0a0248adee0abb3930d3edb76d498ae91e7',
-                '84db53592e882b5af077976257f9c7537ed971cb2059003fd4faa05d02cae0ab',
+                bar_tar_gz_sha256,
+                bar_patch_sha256,
             ],
             'exts_filter': ("cat | grep '^bar$'", '%(name)s'),
-            'patches': ['bar-0.0_typo.patch'],
+            'patches': [bar_patch],
             'toy_ext_param': "mv anotherbar bar_bis",
             'unknowneasyconfigparameterthatshouldbeignored': 'foo',
         }))
@@ -3499,7 +3512,7 @@ def test_inject_checksums(self):
             "^== backup of easyconfig file saved to .*/test\.eb\.bak_[0-9]+\.\.\.$",
             "^== injecting md5 checksums for sources & patches in test\.eb\.\.\.$",
             "^== \* toy-0.0\.tar\.gz: be662daa971a640e40be5c804d9d7d10$",
-            "^== \* toy-0\.0_typo\.patch: e6785e1a721fc8bf79892e3ef41557c0$",
+            "^== \* toy-0\.0_fix-silly-typo-in-printf-statement\.patch: e6785e1a721fc8bf79892e3ef41557c0$",
             "^== \* toy-extra\.txt: 3b0787b3bf36603ae1398c4a49097893$",
         ]
         for pattern in patterns: