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

setup.py: Use gnureadline for all other OS #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yzgyyang
Copy link
Contributor

@yzgyyang yzgyyang commented Jul 19, 2018

setup.py: Use gnureadline for all other OS

This adds detection for if the readline library is built in, and use
gnureadline if that is not the case and not on Windows.

Since gnureadline links against a static internal version of the
readline library, it could be used for systems where readline support
is not built in (except on Windows where we have pyreadline).

Closes #1

setup.py Outdated
elif 'linux' in sysplat or 'freebsd' in sysplat and has_readline:
pass
else:
raise ImportError("readline is not built-in, and no custom package is
Copy link
Collaborator

Choose a reason for hiding this comment

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

ImportError doesnt work in setup.py

setup.py Outdated

sysplat = str(sys.platform).lower()
has_readline = sysconfig.get_config_var('HAVE_LIBREADLINE')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename to have_libreadline to avoid confusion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops.

setup.py Outdated

install_requires = ['setuptools']
if 'darwin' in sysplat:
install_requires += ['gnureadline']
if 'win32' in sysplat:
install_requires += ['pyreadline']
elif 'linux' in sysplat or 'freebsd' in sysplat and has_readline:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would break every other OS. Like all the other BSDs. OS checks are inherently bad, unless we know the OS very well and it is known to be broken.

If any OS has libreadline, it is good.

If OS is Windows, use pyreadline.

If anything else (implicitly inc Darwin), add gnureadline as a dependency?
(Read discussion at #1 and at gnureadline issue.)

@yzgyyang yzgyyang force-pushed the linux branch 2 times, most recently from f38b0ed to cecdb1c Compare July 24, 2018 07:01
Copy link
Collaborator

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

Commit message doesnt match commit.

Also pls do #7 first , so we can check what happens.

This adds detection for if the readline library is built in, and use
gnureadline if that is not the case and not on Windows.

Since gnureadline links against a static internal version of the
readline library, it could be used for systems where readline support
is not built in (except on Windows where we have pyreadline).

Closes pombredanne#1
@yzgyyang yzgyyang changed the title setup.py: Fails the script if nothing installed setup.py: Use gnureadline for all other OS Jul 24, 2018
@jayvdb jayvdb added the blocked label Jul 24, 2018
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.

2 participants