From 0d407089ec752f47bb4b80bb6133598241dc77b0 Mon Sep 17 00:00:00 2001
From: Alexander Grund <alexander.grund@tu-dresden.de>
Date: Wed, 4 Sep 2024 17:13:53 +0200
Subject: [PATCH] verify checksums with `--fetch`

After `--fetch` finishes it is reasonable to expect that the build won't
fail due to missing or wrong sources.
However putting the checksum-step into the source-step skips over the
verification and one would need to use `--stop=source` which does a lot
more than necessary, e.g. creating build dirs and environment
variables.

Move the checksums step into the fetch step and add a test for that.
---
 easybuild/framework/easyblock.py | 16 ++++------------
 test/framework/options.py        | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py
index d119a299b7..71c69e1821 100644
--- a/easybuild/framework/easyblock.py
+++ b/easybuild/framework/easyblock.py
@@ -4038,15 +4038,6 @@ def ready_step_spec(initial):
             return get_step(READY_STEP, "creating build dir, resetting environment", ready_substeps, False,
                             initial=initial)
 
-        source_substeps = [
-            (False, lambda x: x.checksum_step),
-            (True, lambda x: x.extract_step),
-        ]
-
-        def source_step_spec(initial):
-            """Return source step specified."""
-            return get_step(SOURCE_STEP, "unpacking", source_substeps, True, initial=initial)
-
         install_substeps = [
             (False, lambda x: x.stage_install_step),
             (False, lambda x: x.make_installdir),
@@ -4060,6 +4051,7 @@ def install_step_spec(initial):
         # format for step specifications: (step_name, description, list of functions, skippable)
 
         # core steps that are part of the iterated loop
+        extract_step_spec = (SOURCE_STEP, "unpacking", [lambda x: x.extract_step], True)
         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)
@@ -4069,9 +4061,9 @@ def install_step_spec(initial):
 
         # part 1: pre-iteration + first iteration
         steps_part1 = [
-            (FETCH_STEP, 'fetching files', [lambda x: x.fetch_step], False),
+            (FETCH_STEP, 'fetching files', [lambda x: x.fetch_step, lambda x: x.checksum_step], False),
             ready_step_spec(True),
-            source_step_spec(True),
+            extract_step_spec,
             patch_step_spec,
             prepare_step_spec,
             configure_step_spec,
@@ -4085,7 +4077,7 @@ def install_step_spec(initial):
         # not all parts of all steps need to be rerun (see e.g., ready, prepare)
         steps_part2 = [
             ready_step_spec(False),
-            source_step_spec(False),
+            extract_step_spec,
             patch_step_spec,
             prepare_step_spec,
             configure_step_spec,
diff --git a/test/framework/options.py b/test/framework/options.py
index 089b068e62..0c7ae93f25 100644
--- a/test/framework/options.py
+++ b/test/framework/options.py
@@ -5497,7 +5497,7 @@ def test_fetch(self):
         # which might trip up the dependency resolution (see #4298)
         for ec in ('toy-0.0.eb', 'toy-0.0-deps.eb'):
             args = [ec, '--fetch']
-            stdout, stderr = self._run_mock_eb(args, raise_error=True, strip=True, testing=False)
+            stdout, _ = self._run_mock_eb(args, raise_error=True, strip=True, testing=False)
 
             patterns = [
                 r"^== fetching files\.\.\.$",
@@ -5510,6 +5510,17 @@ def test_fetch(self):
             regex = re.compile(r"^== creating build dir, resetting environment\.\.\.$")
             self.assertFalse(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout))
 
+        # --fetch should also verify the checksums
+        tmpdir = tempfile.mkdtemp(prefix='easybuild-sources')
+        write_file(os.path.join(tmpdir, 'toy-0.0.tar.gz'), 'Make checksum check fail')
+        args = ['--sourcepath=%s:%s' % (tmpdir, self.test_sourcepath), '--fetch', 'toy-0.0.eb']
+        with self.mocked_stdout_stderr():
+            pattern = 'Checksum verification for .*/toy-0.0.tar.gz .*failed'
+            self.assertErrorRegex(EasyBuildError, pattern, self.eb_main, args, do_build=True, raise_error=True)
+            # We can avoid that failure by ignoring the checksums
+            args.append('--ignore-checksums')
+            self.eb_main(args, do_build=True, raise_error=True)
+
     def test_parse_external_modules_metadata(self):
         """Test parse_external_modules_metadata function."""
         # by default, provided external module metadata cfg files are picked up