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

Merge of the ConfigureMake and CMakeMake versions of the easyblock for QuantumESPRESSO

Open Crivella opened this issue 1 year ago • 2 comments
trafficstars

The QuantumESPRESSO ConfigMake and CMakeMake easyblocks have been merged in the same file with a wrapper easyblock deciding which one will get used depending on the version of QE.

This PR would

  • replace #3306
  • Need changes in https://github.com/easybuilders/easybuild-easyconfigs/pull/20070 to be reverted
  • Should be tested with one or more legacy (<=7.3) EC file and with a CMake EC file (https://github.com/easybuilders/easybuild-easyconfigs/pull/20138)

Crivella avatar May 24 '24 14:05 Crivella

@boegel I tried merging the 2 easyblocks for QuantumESPRESSO into a new one acting as a wrapper and forwarding the calls to the correct EB based on the version. It is slightly hackish but still very clean code-wise.

Unfortunately eb --list-easyblocks and it's associated unittest is causing the CI to fail, due to having multiple classes defined in one file. I am not sure if the change i proposed in https://github.com/easybuilders/easybuild-framework/issues/4543 would make sense.

The alternatives would be to either define the classes inside something like another class (or the wrapper class itself) or a dict (anything that will let class not be at the beginning of the string to trick the ^class regex), but that would make the code less maintainable in my opinion. There is also the option of squashing the classes together and have a wrapper for every ..._step method, but that would make it even less maintainable.

Any suggestion in which direction to proceed?

Crivella avatar May 27 '24 08:05 Crivella

This is extensively tested in https://github.com/easybuilders/easybuild-easyconfigs/pull/20070 and https://github.com/easybuilders/easybuild-easyconfigs/pull/20138

ocaisa avatar Jun 14 '24 12:06 ocaisa

Put version specific classes inside the wrapper as to not have trouble with framework EB detection regex

Crivella avatar Jul 16 '24 13:07 Crivella