Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance test and install step of CMakePythonPackage easyblock #2318

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Jan 26, 2021

Currently, CMakePythonPackage fails with any easyconfig that does not explicitly set runtest because it executes the test step in CMakeMake with the defaults of PythonPackaging, resulting in a make True test command.
This PR fixes this issue by adding specific options for runtest in CMakeMake and in PythonPackage.

Moreover, installopts suffers from a similar problem in this easyblock. This option is unusable because it is handled by PythonPackage, which might add a bunch of options for pip if usepip = True, whereas the install step is executed by make.
This PR fixes this issue by adding a specific installopts for make and it also adds the option to execute the install step from PythonPackage (ie pip install) after make install. This is useful for packages that do not install the python module with make install.

I also made a little bit of cleanup of the code, using inheritance through super() where possible.

Existing easyconfigs using CMakePythonPackage are not affected by this PR, see tests below. The low number of easyconfigs using this easyblock might be a result of these issues.

@lexming
Copy link
Contributor Author

lexming commented Jan 26, 2021

An example of the failure of CMakePythonPackage can be found in the test report of Z3 at easybuilders/easybuild-easyconfigs#12002 (comment)

== 2021-01-26 05:53:38,577 run.py:222 INFO running cmd: make True
== 2021-01-26 05:53:39,027 build_log.py:169 ERROR EasyBuild crashed with an error (at easybuild/easybuild-framework/easybuild/base/exceptions.py:124 in init): cmd " make True " exited with exit code 2 and output:
make: *** No rule to make target 'True'. Stop.

@lexming
Copy link
Contributor Author

lexming commented Jan 26, 2021

Successful test with Z3 and the CMakePythonPackage easyblock from this PR at: easybuilders/easybuild-easyconfigs#12002 (comment)

@lexming
Copy link
Contributor Author

lexming commented Jan 26, 2021

Test report by @lexming

Overview of tested easyconfigs (in order)

  • SUCCESS CRPropa-3.1.6-foss-2020a-Python-3.8.2.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node379.hydra.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6148 CPU @ 2.40GHz, Python 2.7.5
See https://gist.github.com/26f59abf9e1e53c909d73fd8e2afecc6 for a full test report.

@lexming
Copy link
Contributor Author

lexming commented Jan 26, 2021

Test report by @lexming

Overview of tested easyconfigs (in order)

  • SUCCESS ADIOS-1.13.1-foss-2019a-Python-2.7.15.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node381.hydra.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6148 CPU @ 2.40GHz, Python 2.7.5
See https://gist.github.com/86323a900beccc5621e5a767245ee190 for a full test report.

@lexming
Copy link
Contributor Author

lexming commented Jan 27, 2021

An example of the start_dir_python option can be found in the easyconfigs of SimpleITK v2.0.2 in PR easybuilders/easybuild-easyconfigs#12055

This option defines the directory containing the source of the Python package to be installed with PythonPackage, once make install from CMakeMake finishes:
Line 44 of SimpleITK-2.0.2-foss-2020a-Python-3.8.2.eb

start_dir_python = 'Wrapping/Python'

The path can be an absolute path or a path relative to buildir.

return PythonPackage.make_module_extra(self)
def post_install_step(self):
"""Reset working directory before post-installation commands"""
change_dir(os.path.join(self.builddir, 'easybuild_obj'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct if build_in_installdir is True for the CMake part, or if separate_builddir is False.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexming friendly reminder, any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants