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.

Sign up for GitHub

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

Jump to bottom

don't change installopts easyconfig parameter value in-place in PythonPackage easyblock #3080

Merged
merged 1 commit into from
Jan 18, 2024
Merged

don't change installopts easyconfig parameter value in-place in PythonPackage easyblock #3080

merged 1 commit into from
Jan 18, 2024

Conversation

boegel
Copy link
Member

@boegel boegel commented Jan 17, 2024

The PythonPackage easyblock currently updates installopts in-place when the determine_install_command method is called (which is done in the constructor), like it owns the place.

This causes problems for other easyblocks that derive from PythonPackage though, especially when use_pip is enabled.
For CMakePythonPackage for example, it results in options for the pip install command being passed to make install, which leads to problems like this (for tmap-20220502-GCC-11.2.0.eb with use_pip set to True, or using the changes being made in #3022):

== FAILED: Installation ended unsuccessfully: cmd " make install  --verbose  --no-deps  --ignore-installed  --no-index " exited with exit code 2 and output:
make: unrecognized option '--verbose'
make: unrecognized option '--no-deps'
...

This fix is important in the context of #3022, where use_pip is being enabled by default.

The VersionIndependentPythonPackage generic easyblock (which is currently only used by archived easyconfigs) was following the example of PythonPackage and was also updated installopts in place, so it has been updated to append to the custom py_installopts class variable instead.

A couple of other easyblocks that derives from PythonPackage assumed that all options for pip were available in installopts, which is now no longer the case since py_installopts must also be taken into account. This includes the easyblocks for dm-reverb, TensorRT, and wxPytyon.

Other easyblock already had a workaround in place for the fact that PythonPackage was updating installopts in place, while it shouldn't. These easyblocks were updated to remove the workaround, since it's no longer needed: pybind11 + scipy.

Marked as WIP since needs testing first with easyconfigs that use the modified easyblocks, as well as with easyconfigs that use an easyblock like CMakePythonPackage (cfr. list of easyconfigs that are currently not enabling use_pip in #3022 (comment)).

@boegel boegel added this to the 5.0 milestone Jan 17, 2024
@boegel
Copy link
Member Author

boegel commented Jan 17, 2024

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS SciPy-bundle-2022.05-foss-2022a.eb
  • SUCCESS dm-reverb-0.7.0-foss-2021b.eb
  • SUCCESS pybind11-2.9.2-GCCcore-11.3.0.eb
  • SUCCESS TensorRT-8.6.1-foss-2022a-CUDA-11.7.0.eb
  • SUCCESS wxPython-4.2.1-foss-2022a.eb
  • SUCCESS JUBE-2.4.2.eb
  • SUCCESS matplotlib-3.7.0-gfbf-2022b.eb
  • SUCCESS SciPy-bundle-2023.11-gfbf-2023b.eb
  • SUCCESS tqdm-4.66.1-GCCcore-12.3.0.eb

Build succeeded for 9 out of 9 (9 easyconfigs in total)
node3587.doduo.os - Linux RHEL 8.8, x86_64, AMD EPYC 7552 48-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/boegel/a3c3c965bec2bb7b1021f83f79742065 for a full test report.

@boegel boegel marked this pull request as ready for review January 17, 2024 23:39
@boegel boegel changed the title don't change installopts easyconfig parameter value in-place in PythonPackage easyblock (WIP) don't change installopts easyconfig parameter value in-place in PythonPackage easyblock Jan 17, 2024
@SebastianAchilles
Copy link
Member

@boegelbot please test @ jsc-zen3
EB_BRANCH=5.0.x
CORE_CNT=16
EB_ARGS="Pillow-10.0.0-GCCcore-12.3.0.eb"

@boegelbot
Copy link

@SebastianAchilles: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ "5.0.x" != 'develop' ]]; then EB_BRANCH="5.0.x" ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/"5.0.x" source init_env_easybuild_develop.sh; fi; EB_PR=3080 EB_ARGS="Pillow-10.0.0-GCCcore-12.3.0.eb" EB_REPO=easybuild-easyblocks EB_BRANCH="5.0.x" /opt/software/slurm/bin/sbatch --job-name test_PR_3080 --ntasks="16" ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 3423

Test results coming soon (I hope)...

- notification for comment with ID 1898426343 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS Pillow-10.0.0-GCCcore-12.3.0.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.3, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.18
See https://gist.github.com/boegelbot/afa11f2db0ff2b6b64033150e39fff77 for a full test report.

@SebastianAchilles
Copy link
Member

Test report by @SebastianAchilles

Overview of tested easyconfigs (in order)

  • SUCCESS Pillow-10.0.0-GCCcore-12.3.0.eb
  • SUCCESS pybind11-2.11.1-GCCcore-13.2.0.eb
  • SUCCESS SciPy-bundle-2023.11-gfbf-2023b.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
jscclxc1.int.jsc-clx.fz-juelich.de - Linux Rocky Linux 9.3, x86_64, Intel Xeon Processor (Cascadelake) (cascadelake), Python 3.9.18
See https://gist.github.com/SebastianAchilles/b5956138c798be3b7f37c8d42e15c2aa for a full test report.

@branfosj
Copy link
Member

Test report by @branfosj

Overview of tested easyconfigs (in order)

  • SUCCESS SciPy-bundle-2020.03-foss-2020a-Python-2.7.18.eb
  • SUCCESS pybind11-2.4.3-GCCcore-9.3.0-Python-3.8.2.eb
  • SUCCESS tqdm-4.47.0-GCCcore-9.3.0.eb
  • SUCCESS SciPy-bundle-2020.03-foss-2020a-Python-3.8.2.eb
  • SUCCESS matplotlib-3.2.1-foss-2020a-Python-3.8.2.eb
  • SUCCESS Tkinter-2.7.18-GCCcore-9.3.0.eb
  • SUCCESS matplotlib-2.2.5-foss-2020a-Python-2.7.18.eb
  • SUCCESS GST-plugins-base-1.18.4-GCC-10.3.0.eb
  • SUCCESS Zip-3.0-GCCcore-10.2.0.eb
  • SUCCESS dbus-glib-0.112-GCCcore-10.3.0.eb
  • SUCCESS Bazel-3.7.2-GCCcore-10.2.0.eb
  • SUCCESS GConf-3.2.6-GCCcore-10.3.0.eb
  • SUCCESS protobuf-3.14.0-GCCcore-10.2.0.eb
  • SUCCESS pkgconfig-1.5.1-GCCcore-10.2.0-python.eb
  • SUCCESS flatbuffers-1.12.0-GCCcore-10.2.0.eb
  • SUCCESS libunistring-0.9.10-GCCcore-10.3.0.eb
  • SUCCESS libpsl-0.21.1-GCCcore-10.3.0.eb
  • SUCCESS LibSoup-2.74.0-GCCcore-10.3.0.eb
  • SUCCESS giflib-5.2.1-GCCcore-10.2.0.eb
  • SUCCESS libtasn1-4.17.0-GCCcore-10.3.0.eb
  • SUCCESS libgpg-error-1.42-GCCcore-10.3.0.eb
  • SUCCESS libgcrypt-1.9.2-GCCcore-10.3.0.eb
  • SUCCESS JsonCpp-1.9.4-GCCcore-10.2.0.eb
  • SUCCESS Ruby-3.0.1-GCCcore-10.3.0.eb
  • SUCCESS WebKitGTK+-2.27.4-GCC-10.3.0.eb
  • SUCCESS wxPython-4.1.1-foss-2021a.eb
  • SUCCESS LMDB-0.9.24-GCCcore-10.2.0.eb
  • SUCCESS nsync-1.24.0-GCCcore-10.2.0.eb
  • SUCCESS protobuf-python-3.14.0-GCCcore-10.2.0.eb
  • SUCCESS flatbuffers-python-1.12-GCCcore-10.2.0.eb
  • SUCCESS typing-extensions-3.7.4.3-GCCcore-10.2.0.eb
  • SUCCESS TensorFlow-2.4.1-foss-2020b.eb
  • SUCCESS dm-reverb-0.2.0-foss-2020b.eb

Build succeeded for 33 out of 33 (8 easyconfigs in total)
bear-pg0211u03a.bear.cluster - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Gold 6248 CPU @ 2.50GHz (cascadelake), Python 3.6.8
See https://gist.github.com/branfosj/b22cf30890cd842899687aa818ab25d2 for a full test report.

@branfosj branfosj merged commit 7bc2b57 into easybuilders:5.0.x Jan 18, 2024
29 checks passed
@boegel boegel deleted the pythonpackage_installopts branch January 18, 2024 16:59
@boegel boegel mentioned this pull request Jan 18, 2024
Merged
@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Jan 22, 2024
Flamefire
Flamefire reviewed Jan 24, 2024
View reviewed changes
easybuild/easyblocks/generic/pythonpackage.py
@@ -431,26 +432,26 @@ def determine_install_command(self):

pip_verbose = self.cfg.get('pip_verbose', None)
if pip_verbose or (pip_verbose is None and build_option('debug')):
self.cfg.update('installopts', '--verbose')
self.py_installopts.append('--verbose')
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is a behavior change: self.cfg.update only adds the value if it doesn't exist yet. Especially a double --verbose means "VERY verbose"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flamefire Flamefire Flamefire left review comments

@branfosj branfosj branfosj approved these changes

Assignees
No one assigned
Labels
bug fix change EasyBuild-5.0 EasyBuild 5.0
Projects
EasyBuild v5.0
Status: No status
Milestone
5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@boegel @SebastianAchilles @boegelbot @branfosj @Flamefire