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

cmake: link libsclang against ncurses #4900

Merged
merged 1 commit into from
May 1, 2020
Merged

cmake: link libsclang against ncurses #4900

merged 1 commit into from
May 1, 2020

Conversation

mossheim
Copy link
Contributor

Purpose and Motivation

Fixes #4899

On some systems readline is not linked against ncurses; developers must
choose to link with it against either ncurses or termcap. since ncurses
is widely available, we simply find and link against it directly on Linux.

This also appears to be the same problem as https://scsynth.org/t/installation-issue-in-ubuntu-19-10/1952

Types of changes

  • Bug fix

To-do list

  • Code is tested - this works on my machine, but @bjadel you should give this a try if you have the time!
  • All tests are passing
  • Updated documentation - perhaps we should update documentation on necessary packages?
  • This PR is ready for review

@mossheim
cmake: link libsclang against ncurses
49ba8b8 
Fixes #4899

On some systems readline is not linked against ncurses; developers must
choose to link with it against either ncurses or termcap. since ncurses
is widely available, we simply find and link against it directly on Linux.
@mossheim mossheim added comp: build CMake build system comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library" labels Apr 27, 2020
@mossheim mossheim mentioned this pull request Apr 27, 2020
Closed
@dyfer
Copy link
Member

dyfer commented Apr 30, 2020

thanks @brianlheim
What would be the best way to test this (ideally without installing a new distro... if possible?) ? I.e. how to make the problem appear without this fix in order to test the fix? Do we know which distros don't have readline linked against ncurses? Maybe having these answers would guide what to put into documentation...

@mossheim
Copy link
Contributor Author

@dyfer it has been tested on a Linux system with a copy of libreadline that requires further linking (the issue's OP's) and on my system with a libreadline that does not require extra linking. i think that should be enough? at the very least this solves that issue and the other variations on it, and shouldn't cause any noticeable difference otherwise.

Do we know which distros don't have readline linked against ncurses? Maybe having these answers would guide what to put into documentation...

as far as informing documentation, this PR requires the library to be present regardless. my question is more whether it's OK to assume this library exists on a typical machine. at least on my system, both less and bash require it, and i've personally never been on a Linux system that didn't have those.

@bjadel
Copy link

bjadel commented May 1, 2020

Could successfully compile the branch brianlheim:topic/link-ncurses on openSuSE Tumbleweed.

dyfer
dyfer approved these changes May 1, 2020
View reviewed changes
Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

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

Great, thank you.
@bjadel confirmed that this fixed an issue on their system. I've installed this on Ubuntu 18.04 to make sure it doesn't break anything and it worked fine.

@mossheim
Copy link
Contributor Author

mossheim commented May 1, 2020

thank you both!

@mossheim mossheim merged commit 8c01dcf into supercollider:3.11 May 1, 2020
@mossheim mossheim deleted the topic/link-ncurses branch May 1, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dyfer dyfer dyfer approved these changes

Assignees
No one assigned
Labels
comp: build CMake build system comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mossheim @dyfer @bjadel