-
Notifications
You must be signed in to change notification settings - Fork 211
Description
This is a follow-up discussion to #4624
That PR moved the verification of the checksums for sources and patches to the fetch-step.
That results in the "source"-step to only extract the sources.
We have for (almost) all steps now a 1:1 correspondence of (user-facing) step names and their method in the EasyBlock
class:
easybuild-framework/easybuild/framework/easyblock.py
Lines 4043 to 4048 in e0e04fb
patch_step_spec = (PATCH_STEP, 'patching', [lambda x: x.patch_step], True) | |
prepare_step_spec = (PREPARE_STEP, 'preparing', [lambda x: x.prepare_step], False) | |
configure_step_spec = (CONFIGURE_STEP, 'configuring', [lambda x: x.configure_step], True) | |
build_step_spec = (BUILD_STEP, 'building', [lambda x: x.build_step], True) | |
test_step_spec = (TEST_STEP, 'testing', [lambda x: x._test_step], True) | |
extensions_step_spec = (EXTENSIONS_STEP, 'taking care of extensions', [lambda x: x.extensions_step], False) |
However for the (new) "source"-step this isn't true any longer:
extract_step_spec = (SOURCE_STEP, "unpacking", [lambda x: x.extract_step], True) |
Note that even prior to the change the status (printed to stdout) was "unpacking":
== fetching files...
== creating build dir, resetting environment...
== unpacking...
== patching...
== preparing...
== configuring...
== building...
== testing...
== installing...
It could be argued that "source" (for use in e.g. --stop=source
) was fitting because it verified the sources (&patches) and then extracted them, even though the former wasn't explicitly indicated.
However after the change I think "source" is no longer appropriate and we should follow the method names at least for consistency and rename this step to "extract". I'd argue that --stop=extract
is as clear as --stop=configure
and similar.
An alternative would be "unpack" to match the status-message although it then won't match the method name extract_step
This change can only be for 5.x as it is a breaking change and the above change is only for that branch.
I'm pretty sure we already have a check for valid step names, hence any use of the old name "source" would yield a clear message that it is an invalid value and to choose from the valid set of values. If not we should enhance that error message to do that.
With that the change will only cause some required change in habits for people doing --stop=source
. This was however often just a workaround for --fetch
not verifying the checksums, so there is a better alternative for that.
Theoretically there could be easyconfigs using skipsteps=["source"]
although I can hardly imagine a reason for that. We currently have 4 such official ECs in the __archive__
and I'm not sure that was ever really OK as e.g. it meant that the checksums were not verified.
No current official ECs use this.
Requesting input from @boegel, possibly putting this up on the next confcall agenda.