easybuilders / easybuild-easyblocks

Collection of easyblocks that implement support for building and installing software with EasyBuild.
https://easybuild.io
GNU General Public License v2.0
106 stars 285 forks source link

fix `--sanity-check-only` for impi #3403

Closed Flamefire closed 3 months ago

Flamefire commented 3 months ago

(created using eb --new-pr)

During sanity-check-only the prepare_step is skipped which sets up the toolchain including toolchain and environment variables such as $CC

Without that step (i.e. in sanity-check-only mode) os.getenv('CC') will fail because the variable is not set. Or even worse: It is set but to a wrong value.

Additionally ec['parallel'] is also not set yielding a mpirun -n None which of course fails.

This PR adds the prepare_step and set_parallel call to initialize those. It also uses a temporary directory for the test binary because the builddir also doesn't exist.

Flamefire commented 3 months ago

Test report by @Flamefire

Overview of tested easyconfigs (in order)

Build succeeded for 14 out of 14 (14 easyconfigs in total) n1625 - Linux RHEL 8.9 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (icelake), Python 3.8.17 See https://gist.github.com/Flamefire/a68a4257fd9a053a76a855671f68157e for a full test report.

akesandgren commented 3 months ago

Was a bit too quick here... If we use tempfile.mkdtemp outside of builddir we need to delete it afterwards. So should unconditionally remove the basedir of impi_testexe after.

Flamefire commented 3 months ago

Was a bit too quick here... If we use tempfile.mkdtemp outside of builddir we need to delete it afterwards. So should unconditionally remove the basedir of impi_testexe after.

The base directory for that is easybuild-tmp which IIRC will be deleted after the build or at least after the eb run. Isn't that enough?

If it isn't we should only delete the tmp dir, not the builddir as one might want to inspect that as before via --disable-cleanup-build

akesandgren commented 3 months ago

Ah yes, forgot that TMPDIR is set to eb controlled space. Blaming first week after vacation for brain not being fully engaged.

akesandgren commented 3 months ago

Going in, thanks @Flamefire!