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

include 'sources' key to entries in list returned by collect_exts_file_info

Open Flamefire opened this issue 3 years ago • 4 comments

The returned dict per extension has a 'patches' key but no 'sources'. Add this incoorporating source_tmpl or the default value.

Add test for that and also for nosource: True.

Flamefire avatar Aug 05 '22 11:08 Flamefire

@Flamefire Can you provide a bit of context for this PR? Is there a related issue, and if not, can you shortly describe what was the previous behavior, why that was unwanted, and what the (intended) behavior of your fix is?

I saw your thread with @boegel on this in Slack, but it wasn't fully clear to me what 'inconsistency' you were referring to there. Plus, to make it easier to find the reason for this change (also in the future), it's probably good to log it here together with the PR :)

casparvl avatar Aug 09 '22 14:08 casparvl

This came up during development of a fix for a bug in the CI checks, see: https://github.com/easybuilders/easybuild-easyconfigs/pull/15973

Basically the function collect_exts_file_info returns a dict where the patches are included but not sources which is confusing at least and makes one wonder whether to use ['options']['patches'] or ['patches'] or ['options']['sources]` etc respectively.

Especially as this functions is intended to handle the correct resolution of templates in the extension sources and patches and there is even a special case (nosource) which needs to be handled this should be done completely in that function.

So the intended behavior is that the returned dict for each extensions contains a sources key with a list of sources if-and-only-if nosource is not set. The sources should have correctly resolved templates (i.e. using name and versions of the extension not the parent EC)

So actually this is a 2 part fix:

  • Return the (correct) sources at all (i.e. especially using the default value when missing or derive from source_tmpl)
  • Return sources and patches consistently in the main dict and not in the sub-dict options

Flamefire avatar Aug 15 '22 08:08 Flamefire

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

FAIL: test_inject_checksums (test.framework.options.CommandLineOptionsTest)
Test for --inject-checksums
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/test/framework/options.py", line 5822, in test_inject_checksums
    'start_dir': 'src',
  File "/tmp/runner/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/easybuild/base/testing.py", line 119, in assertEqual
    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))
AssertionError: Tuples differ: ('barbar', '1.2', {'checksums'... != ('barbar', '1.2', {'checksums'...

First differing element 2:
{'checksums': ['d5bd9908cdefbe2d29c6f8d5b45b2aaed9fd904b5e6397418bb5094fbdb3d838'], 'start_dir': 'src', 'sources': {'filename': '%(name)s-%(version)s.tar.gz'}}
{'checksums': ['d5bd9908cdefbe2d29c6f8d5b45b2aaed9fd904b5e6397418bb5094fbdb3d838'], 'start_dir': 'src'}

  ('barbar',
   '1.2',
   {'checksums': ['d5bd9908cdefbe2d29c6f8d5b45b2aaed9fd904b5e6397418bb5094fbdb3d838'],
-   'sources': {'filename': '%(name)s-%(version)s.tar.gz'},
    'start_dir': 'src'}):
DIFF:
  ('barbar',
   '1.2',
   {'checksums': ['d5bd9908cdefbe2d29c6f8d5b45b2aaed9fd904b5e6397418bb5094fbdb3d838'],
-   'sources': {'filename': '%(name)s-%(version)s.tar.gz'},


======================================================================
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/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/test/framework/easyblock.py", line 2607, in test_check_checksums
    self.assertEqual(len(res), 4)
  File "/tmp/runner/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/easybuild/base/testing.py", line 119, in assertEqual
    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))
AssertionError: 3 != 4:
DIFF:
- 3

======================================================================
FAIL: test_collect_exts_file_info (test.framework.easyblock.EasyBlockTest)
Test collect_exts_file_info method.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/runner/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/test/framework/easyblock.py", line 1860, in test_collect_exts_file_info
    self.assertEqual(exts_file_info[2]['sources'], ['barbar-0.0.tar.gz'])
  File "/tmp/runner/e6068f50e59420796bc14759b62b33c77b5d2669/lib/python2.7/site-packages/easybuild/base/testing.py", line 119, in assertEqual
    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))
AssertionError: Lists differ: ['barbar-1.2.tar.gz'] != ['barbar-0.0.tar.gz']

First differing element 0:
'barbar-1.2.tar.gz'
'barbar-0.0.tar.gz'

- ['barbar-1.2.tar.gz']
?          ^ ^

+ ['barbar-0.0.tar.gz']
?          ^ ^
:
DIFF:
- ['barbar-1.2.tar.gz']?          ^ ^
+ ['barbar-0.0.tar.gz']

----------------------------------------------------------------------
Ran 848 tests in 1084.010s

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 me acting stupid), or submit a pull request to https://github.com/boegel/boegelbot fix the problem.

boegelbot avatar May 11 '23 15:05 boegelbot

CI failure is unrelated:

Unexpected error occurred when trying to download https://ftpmirror.gnu.org/gnu/binutils/binutils-2.37.tar.gz to /scratch/sources/b/binutils/binutils-2.37.tar.gz: 'Failed to write to /scratch/sources/b/binutils/binutils-2.37.tar.gz: The read operation timed out' (took 17 secs)

Flamefire avatar May 12 '23 07:05 Flamefire

@Flamefire Please sync again with develop after merge of #4255

boegel avatar May 23 '23 18:05 boegel