easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

Support checksums specified in external file (checksums.json)

Open mboisson opened this issue 3 years ago • 2 comments

Renamed the branch since we switch from yaml to json. This replaces https://github.com/easybuilders/easybuild-framework/pull/3748 to address https://github.com/easybuilders/easybuild-framework/issues/3746

  • [x] no test implemented
  • [X] not sure what to do with --inject-checksums (added --inject-checksums-to-json)

I have tested this PR with Python-3.8.2-GCCcore-9.3.0.eb by removing the checksums from the EasyConfig, and with --enforce-checksums enabled.

It allows to remove checksums both for the main file and for the exts_list.

mboisson avatar Jun 18 '21 14:06 mboisson

So far, I have assumed that one had either checksums in the EasyConfig file or a checksums.json file. The checksums.json file mechanism only takes effect if there are no checksums in the EasyConfig file.

This is not really useful to reuse existing EasyConfigs with a bump --try-software-version, as the list provided in the EasyConfig will take precedence and the build will fail.

I am rewriting the code such that both the checksums from the EasyConfig and the checksum from the Json file are loaded, and then providing a build_option to guide which one should be prioritized to validate the checksum if there are both.

mboisson avatar Jul 08 '21 13:07 mboisson

@boegel, do you think we need to address --new-pr and the --inject-checksums-to-json when using --try-software-version issue in this PR ?

mboisson avatar Aug 03 '21 16:08 mboisson

@mboisson: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-framework/actions/runs/3267259052 Output from first failing test suite run:

FAIL: test_enforce_checksums (test.framework.options.CommandLineOptionsTest)
Test effect of --enforce-checksums
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/3b89da80f12eb7bcd1b5504faf9051ac8a854bcd/lib/python2.7/site-packages/test/framework/options.py", line 5915, in test_enforce_checksums
    self.assertErrorRegex(EasyBuildError, error_pattern, self.eb_main, args, do_build=True, raise_error=True)
  File "/tmp/runner/3b89da80f12eb7bcd1b5504faf9051ac8a854bcd/lib/python2.7/site-packages/easybuild/base/testing.py", line 158, in assertErrorRegex
    self.assertTrue(False, "Expected errors with %s(%s) call should occur" % (call.__name__, str_args))
AssertionError: False is not true : Expected errors with eb_main(['/tmp/eb-n_m547/eb-t46xo6/eb-l77fhV/test.eb', '--stop=source', '--enforce-checksums'], raise_error=True, do_build=True) call should occur

======================================================================
FAIL: test_index_functions (test.framework.filetools.FileToolsTest)
Test *_index functions.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/3b89da80f12eb7bcd1b5504faf9051ac8a854bcd/lib/python2.7/site-packages/test/framework/filetools.py", line 2398, in test_index_functions
    self.assertTrue(fp.endswith('.eb'))
AssertionError: False is not true

======================================================================
FAIL: test_check_checksums (test.framework.easyblock.EasyBlockTest)
Test for check_checksums_for and check_checksums methods.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/3b89da80f12eb7bcd1b5504faf9051ac8a854bcd/lib/python2.7/site-packages/test/framework/easyblock.py", line 2512, in test_check_checksums
    self.assertEqual(eb.check_checksums(), [])
  File "/tmp/runner/3b89da80f12eb7bcd1b5504faf9051ac8a854bcd/lib/python2.7/site-packages/easybuild/base/testing.py", line 116, in assertEqual
    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))
AssertionError: Lists differ: ['Checksums missing for one or... != []

First list contains 1 additional elements.
First extra element 0:
'Checksums missing for one or more sources/patches in toy-0.0.eb: found 1 sources + 2 patches vs 2 checksums'

- ['Checksums missing for one or more sources/patches in toy-0.0.eb: found 1 sources + 2 patches vs 2 checksums']
+ []:
DIFF:
- ['Checksums missing for one or more sources/patches in toy-0.0.eb: found 1 sources + 2 patches vs 2 checksums']

----------------------------------------------------------------------
Ran 818 tests in 1001.226s

FAILED (failures=3)
ERROR: Not all tests were successful.

bleep, bloop, I'm just a bot (boegelbot v20200716.01) Please talk to my owner @boegel if you notice you me acting stupid), or submit a pull request to https://github.com/boegel/boegelbot fix the problem.

boegelbot avatar Oct 17 '22 19:10 boegelbot

@boegel Should this be closed or fixed ? I won't spend time fixing its tests if it's not going to end up getting merged.

mboisson avatar Oct 31 '22 20:10 mboisson

@mboisson Having support for storing checksums externally makes sense, it can help a lot to declutter easyconfigs, so it would be great if we could get the tests fixed... I can't make any promises on when this will go in though, sorry.

boegel avatar Nov 09 '22 08:11 boegel

@mboisson Some proposed cleanup for the test part of this in https://github.com/ComputeCanada/easybuild-framework/pull/26

I intend to make a pass over the non-test part too to propose some minor improvements, but this looks really good overall, thanks for all the effort you've put into this already! 👍

boegel avatar Nov 18 '22 07:11 boegel