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

make Pillow easyconfigs aware of sysroot template #19226

Merged

Conversation

ocaisa
Copy link
Member

@ocaisa ocaisa commented Nov 13, 2023

(created using eb --new-pr)

@ocaisa ocaisa added this to the next release (4.9.0?) milestone Nov 13, 2023
@ocaisa ocaisa added the update label Nov 13, 2023
@ocaisa
Copy link
Member Author

ocaisa commented Nov 13, 2023

@boegelbot please test @ jsc-zen2

@boegelbot
Copy link
Collaborator

@ocaisa: Request for testing this PR well received on jsczen2l1.int.jsc-zen2.easybuild-test.cluster

PR test command 'EB_PR=19226 EB_ARGS= EB_REPO=easybuild-easyconfigs /opt/software/slurm/bin/sbatch --mem-per-cpu=4000M --job-name test_PR_19226 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen2.sh' executed!

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

Test results coming soon (I hope)...

- notification for comment with ID 1808186514 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
Collaborator

Test report by @boegelbot
FAILED
Build succeeded for 12 out of 14 (13 easyconfigs in total)
jsczen2c1.int.jsc-zen2.easybuild-test.cluster - Linux Rocky Linux 8.5, x86_64, AMD EPYC 7742 64-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/boegelbot/38f2fc2e97563d1f071fd6bf1cd3eb92 for a full test report.

@easybuilders easybuilders deleted a comment from boegelbot Nov 13, 2023
@ocaisa
Copy link
Member Author

ocaisa commented Nov 13, 2023

@boegelbot please test @ generoso

@boegelbot
Copy link
Collaborator

@ocaisa: Request for testing this PR well received on login1

PR test command 'EB_PR=19226 EB_ARGS= EB_CONTAINER= EB_REPO=easybuild-easyconfigs /opt/software/slurm/bin/sbatch --job-name test_PR_19226 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

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

Test results coming soon (I hope)...

- notification for comment with ID 1808323932 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
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 13 out of 13 (13 easyconfigs in total)
cns1 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/a02dd5f8a4dea5778c5dfb244e8dba78 for a full test report.

@verdurin
Copy link
Member

Test report by @verdurin
SUCCESS
Build succeeded for 14 out of 14 (13 easyconfigs in total)
easybuild-c7.novalocal - Linux CentOS Linux 7.9.2009, x86_64, Intel Xeon Processor (Skylake, IBRS), Python 3.6.8
See https://gist.github.com/verdurin/d6ec8432c7ee073ee5be40c3a02514a7 for a full test report.

Copy link
Member

@verdurin verdurin left a comment

Choose a reason for hiding this comment

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

Looks fine.

@verdurin
Copy link
Member

Going in, thanks @ocaisa!

@verdurin verdurin merged commit 1ae8ca7 into easybuilders:develop Nov 13, 2023
9 checks passed
@boegel
Copy link
Member

boegel commented Nov 13, 2023

@ocaisa Wouldn't it be better to try and patch the hardcoded paths in Pillow's setup.py via sed & %(sysroot)s, rather than always adding <sysroot>/usr/include to $CPATH (and likewise for $LD_LIBRARY_PATH), so we're really only changing something when %(sysroot)s is not an empty string (now we're always adding /usr/include to $CPATH, I'm not sure that's wise).

@boegel
Copy link
Member

boegel commented Nov 20, 2023

@ocaisa Wouldn't it be better to try and patch the hardcoded paths in Pillow's setup.py via sed & %(sysroot)s, rather than always adding <sysroot>/usr/include to $CPATH (and likewise for $LD_LIBRARY_PATH), so we're really only changing something when %(sysroot)s is not an empty string (now we're always adding /usr/include to $CPATH, I'm not sure that's wise).

implemented in #19267

@boegel boegel changed the title Make Pillow easyconfigs aware of sysroot make Pillow easyconfigs aware of sysroot template Nov 22, 2023
@boegel boegel added bug fix and removed update labels Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix EESSI Related to EESSI project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants