include 'sources' key to entries in list returned by collect_exts_file_info #4054
Conversation
|
@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 :) |
|
This came up during development of a fix for a bug in the CI checks, see: easybuilders/easybuild-easyconfigs#15973 Basically the function 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 ( So the intended behavior is that the returned dict for each extensions contains a So actually this is a 2 part fix:
|
easybuild/framework/easyblock.py
Outdated
| @@ -594,6 +594,7 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): | |||
| if 'source_urls' not in source: | |||
| source['source_urls'] = source_urls | |||
|
|
|||
| ext_src['sources'] = sources | |||
There was a problem hiding this comment.
Hmm, shouldn't this be [source] (to always have a dict value here)?
Otherwise we'll sometimes have a dict value, sometimes a string value, which is annoying to deal with wherever this is used...
There was a problem hiding this comment.
The idea is that similar to 'patches' this is always a list of filenames. See the test(s). This is especially important for the nosource option.
| @@ -616,6 +617,7 @@ def collect_exts_file_info(self, fetch_files=True, verify_checksums=True): | |||
| error_msg = "source_tmpl value must be a string! (found value of type '%s'): %s" | |||
| raise EasyBuildError(error_msg, type(src_fn).__name__, src_fn) | |||
|
|
|||
| ext_src['sources'] = [src_fn] | |||
There was a problem hiding this comment.
Maybe this should be [{'filename': src_fn}]?
There was a problem hiding this comment.
It should be a list of strings, see https://github.com/easybuilders/easybuild-framework/pull/4054/files#r952807882. However I found a case where it might not be and fixed that
boegel
changed the title
collect_exts_file_info: Add 'sources' key to extensions
Flamefire
force-pushed
the
ext_sources
branch
4 times, most recently
from
e7dba90
to
12ff5bb
Compare
|
CI failure is unrelated:
|
|
@Flamefire Please sync again with |
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` and `sources` being a list.
The returned dict per extension has a 'patches' key but no 'sources'.
Add this incoorporating
source_tmplor the default value.Add test for that and also for
nosource: True.