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

use versioned symbols in ncurses built with system toolchain (by adding --with-versioned-syms configure option) #16064

Merged

Conversation

Micket
Copy link
Contributor

@Micket Micket commented Aug 19, 2022

(created using eb --new-pr)
fixes #16014
but i still have a bunch of questions; since i'm not familiar with what versioned syms actually does.

  1. do i need to rebuild all the stuff that depends on ncurses-6.x.eb after this change?
  2. should we also change the ncurses we build for each toolchain as well?

@Micket Micket marked this pull request as draft August 19, 2022 10:18
@Micket Micket requested a review from boegel August 19, 2022 11:15
@Micket Micket added the bug fix label Aug 19, 2022
@Micket
Copy link
Contributor Author

Micket commented Aug 19, 2022

Test report by @Micket
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in total)
alvis-c1 - Linux Rocky Linux 8.5, x86_64, Intel Xeon Processor (Skylake), Python 3.6.8
See https://gist.github.com/81e1a4baca6e510f0c8ff204faaa2bbe for a full test report.

@boegel boegel added this to the next release (4.6.1?) milestone Aug 22, 2022
@boegel
Copy link
Member

boegel commented Aug 23, 2022

(created using eb --new-pr) fixes #16014 but i still have a bunch of questions; since i'm not familiar with what versioned syms actually does.

1. do i need to rebuild all the stuff that depends on ncurses-6.x.eb after this change?

I think strictly speaking the answer is "yes" here (if you did a forced re-installation of ncurses that is configured with --with-versioned-syms), since it results in no longer exposing (internal) symbols (according to this), but in practice it probably won't matter (since stuff building on top of ncurses should not be using the internal symbols).

2. should we also change the ncurses we build for each toolchain as well?

Based on the problems reported in #16014, we should be using --with-versioned-syms in all ncurses easyconfigs...
Let's start with these 4 though, and try and get a test report for these easyconfigs + stuff that depends on them (like libreadline, gettext, CMake, Lua, htop, tmux)

@Micket Micket marked this pull request as ready for review August 23, 2022 14:17
@boegel
Copy link
Member

boegel commented Aug 23, 2022

@boegelbot please test @ generoso

@boegelbot
Copy link
Collaborator

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

PR test command 'EB_PR=16064 EB_ARGS= /opt/software/slurm/bin/sbatch --job-name test_PR_16064 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

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

Test results coming soon (I hope)...

- notification for comment with ID 1224269093 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 4 out of 4 (4 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/9d8d0f4ee89efad86d2dc166faada12a for a full test report.

@Micket
Copy link
Contributor Author

Micket commented Sep 8, 2022

@boegel merge?

Also, we should probably start doing this to at least all the new ncurses easyconfigs we add?

@verdurin
Copy link
Member

verdurin commented Sep 9, 2022

Test report by @verdurin
SUCCESS
Build succeeded for 4 out of 4 (4 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/a8e167f2a9f455c1013baf32ecda496e 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

verdurin commented Sep 9, 2022

Going in, thanks @Micket!

@verdurin verdurin merged commit 30e9b29 into easybuilders:develop Sep 9, 2022
@boegel
Copy link
Member

boegel commented Sep 9, 2022

I did some checking whether this would cause trouble for stuff built on top of ncurses, but as far as I can tell it's fine (for the stuff we build with system toolchain and that has a direct dependency declared on ncurses), no matter whether that software is rebuilt or not after ncurses was rebuilt with the --with-versioned-syms included.

I meant to upload test reports to confirm that, but didn't get around to it yet.

Also, we should probably start doing this to at least all the new ncurses easyconfigs we add?

Yes, and probably also to recent existing ncurses easyconfigs... I'll make a separate PR for that.

@boegel boegel changed the title Use versioned syms in system level ncurses use versioned symbols in ncurses built with system toolchain (by adding --with-versioned-syms configure option) Sep 9, 2022
@Micket Micket deleted the 20220819121805_new_pr_ncurses60 branch April 21, 2023 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure for libreadline-8.1-GCCcore-11.2.0
4 participants