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

Update ncurses to not build a separate libtinfo but provide a soft link instead. Force linking to ncurses in libreadline #5067

Merged
merged 8 commits into from
Aug 30, 2017

Conversation

ocaisa
Copy link
Member

@ocaisa ocaisa commented Aug 28, 2017

(created using eb --new-pr)

…nk instead. Force linking to ncurses in libreadline
@ocaisa
Copy link
Member Author

ocaisa commented Aug 28, 2017

Test report by @ocaisa
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in this PR)
jrl01 - Linux centos linux 7.3.1611, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/3b403327fbdf397c5666f279f5765e55 for a full test report.

@ocaisa
Copy link
Member Author

ocaisa commented Aug 28, 2017

Test report by @ocaisa
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in this PR)
alanc-VirtualBox - Linux ubuntu 16.04, Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz, Python 2.7.12
See https://gist.github.com/453de02d373fc10d96b0693bcbf3a6c1 for a full test report.

@boegel boegel added this to the 3.4.0 milestone Aug 29, 2017
libs = ["form", "menu", "ncurses", "panel", "tinfo"]
# need to take care of $CFLAGS ourselves with dummy toolchain
# we need to add -fPIC, but should also include -O* option to avoid compiling with -O0 (default for GCC)
buildopts = ['CFLAGS="-O2 -fPIC"', 'CFLAGS="-O2 -fPIC"']
Copy link
Member

Choose a reason for hiding this comment

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

you don't need to make this a list, just a string will work (it will be used once for every entry in configopts)

buildopts = 'CFLAGS="-O2 -fPIC"

# we need to add -fPIC, but should also include -O* option to avoid compiling with -O0 (default for GCC)
buildopts = ['CFLAGS="-O2 -fPIC"', 'CFLAGS="-O2 -fPIC"']

# Symlink to ncurses for libtinfo (since it can handle the API) so it doesn't get picked up from the OS
Copy link
Member

Choose a reason for hiding this comment

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

I'd rephrase this to symlink libtinfo to libncurses (since ...?

@boegel
Copy link
Member

boegel commented Aug 29, 2017

Other than the minor remarks, this looks good to me. I don't consider symlinking libtinfo to libncurses a dirty hack at all...

You mentioned in #3545 (comment) that Debian "has done this in the past". Are you saying they are not doing this anymore, and if so, why not, and what are they doing now? Any references?

@boegel
Copy link
Member

boegel commented Aug 29, 2017

Test report by @boegel
SUCCESS
Build succeeded for 12 out of 12 (12 easyconfigs in this PR)
node2497.golett.os - Linux centos linux 7.3.1611, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/c0b51919108f3a01891ecb7d33625b5c for a full test report.

@boegel
Copy link
Member

boegel commented Aug 29, 2017

Test report by @boegel
SUCCESS
Build succeeded for 12 out of 12 (12 easyconfigs in this PR)
node2125.delcatty.os - Linux centos linux 7.3.1611, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/f4d24b4e769911130073e2e01b931246 for a full test report.

@ocaisa
Copy link
Member Author

ocaisa commented Aug 30, 2017

I was only reading that from https://bugzilla.redhat.com/show_bug.cgi?id=499837#c2 (which was in 2010), I don't know what the current status is.

@boegel
Copy link
Member

boegel commented Aug 30, 2017

Seems like they provide a separate libtinfo.so now (not a symlink to libncurses.so, since they're split across different pacakges: libncurses5-dev which requires libtinfo-dev as a dep: https://packages.debian.org/stretch/amd64/libtinfo-dev/filelist, https://packages.debian.org/stretch/amd64/libncurses5-dev/filelist

@ocaisa
Copy link
Member Author

ocaisa commented Aug 30, 2017

So do you think we should be doing the same?

@boegel
Copy link
Member

boegel commented Aug 30, 2017

@ocaisa No, not really, if this approach works, fine. We had the split libtinfo before, and that caused problems in our context...

@ocaisa
Copy link
Member Author

ocaisa commented Aug 30, 2017

Ok, my bad, I thought you meant to give ncurses a termcap dep (but termcap hasn't been updated since 2002)

# for the termcap symbols, use EB ncurses
preconfigopts = "env LDFLAGS='-lncurses'"
buildopts += "SHLIB_LIBS='-lncurses'"
Copy link
Member Author

@ocaisa ocaisa Aug 30, 2017

Choose a reason for hiding this comment

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

@boegel
Copy link
Member

boegel commented Aug 30, 2017

Going in, thanks @ocaisa!

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

Successfully merging this pull request may close these issues.

2 participants