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

bpo-36849: Improve libcurses detection on HP-UX #13184

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

Conversation

michael-o
Copy link
Contributor

@michael-o michael-o commented May 8, 2019

libcurses on HP-UX is a symlink to libxcurses. Handle that properly in setup.py.

https://bugs.python.org/issue36849

libcurses on HP-UX is a symlink to libxcurses. Handle that properly in setup.py.
@mdickinson
Copy link
Member

The code changes look reasonable at a shallow level. It may be worth considering adding an explanatory comment to the code about xcurses. But more generally I'm afraid I don't have any expertise in either curses setup or HP-UX; I'll unassign myself from review.

@mdickinson mdickinson removed their request for review February 7, 2020 19:03
@michael-o
Copy link
Contributor Author

michael-o commented Feb 8, 2020

Do you think that the commit message paired with blame isn't enough? I see no harm here because all previous cases are retained.

@skrah
Copy link
Contributor

skrah commented Feb 21, 2020

@michael-o The problem is that none of us uses HP-UX and we have no HP-UX buildbots. So I have some questions:

  1. Is xcurses still widely used in 2020?

  2. When HP-UX was in broad use (say around 2000), apparently no special case for xcurses was needed in setup.py. Or was it deleted at some point?

So yes, some comment why xcurses is needed on HP-UX would help.

@michael-o
Copy link
Contributor Author

michael-o commented Feb 21, 2020

@skrah

  1. xcurses is the only library available with the OS (HP-UX). curses links to it for compat reasons:
$ ll /usr/lib/hpux32/libcurses.so
lr-xr-xr-x 1 bin bin 17 2018-09-07 15:29 /usr/lib/hpux32/libcurses.so -> ./libxcurses.so.1*

I do use xcurses for all OSS requiring curses, e.g., tail, less, vim, etc.

  1. I did engage with Python on HP-UX in 2019 (not earlier) because I needed it to run. Previously, the test simply failed and the Python curses module was not built.

So it is either xcurses or bust. Here is also an explanation why it is xcursesand not ncurses because HP-UX is a UNIX.

Does this suffice?

@skrah
Copy link
Contributor

skrah commented Feb 21, 2020

Well yes, the explanation is good and the patch looks OK. I'm still hesitating to commit because HP-UX is not officially supported (https://pythondev.readthedocs.io/platforms.html) and if something breaks (unexpected but always possible) there's additional work again.

So I guess I'm also unassigning myself.

@skrah skrah removed their request for review February 21, 2020 22:17
@michael-o
Copy link
Contributor Author

@skrah Pity, I have other patches in the pipeline and want contribute more for HP-UX, but if no one is willing to bless my PRs that's going to be hard. Note that I have also patched GNU readline (Bash) and that has been happily absorbed by the community.

As far as I know only commercial UNIX uses xcurses.

@akuchling
Copy link
Member

My inclination would be to commit it. Rationale:

  • I can't find any signs that libxcurses is available on any of the well-supported platforms (https://pythondev.readthedocs.io/platforms.html#well-supported-platforms), so it shouldn't cause problems where setup.py will now choose libxcurses instead of libcurses. The best-effort commercial platforms (AIX and Solaris) seem most at risk from this.

  • if it does break something on a supported platform, the buildbots should catch it and we can back the change out, right?

So I think this is actually pretty low-risk.

@kadler
Copy link
Contributor

kadler commented Nov 13, 2020

FYI, on AIX the curses library is also xcurses:

ls -l /usr/lib/lib*curses*
lrwxrwxrwx 1 bin bin 25 Sep 13  2019 /usr/lib/libcurses.a -> /usr/ccs/lib/libxcurses.a
lrwxrwxrwx 1 bin bin 25 Sep 13  2019 /usr/lib/libxcurses.a -> /usr/ccs/lib/libxcurses.a

@michael-o
Copy link
Contributor Author

Because it is a commercial UNIX....

@kadler
Copy link
Contributor

kadler commented Nov 13, 2020

I'm not disagreeing with you. My comment was in regard to this:

The best-effort commercial platforms (AIX and Solaris) seem most at risk from this.

My point was that since libcurses is libxcurses on AIX already, if anything this should help AIX.

I believe the reason AIX doesn't already run in to this issue is because the code is trying to use ldd to detect the curses library it's linked to, but that doesn't work on AIX when the library is an archive (which is the norm on AIX) and instead one should be using dump -H instead. Since ldd returns exit code 2, it will bypass the regex entirely and use the AIX-specific code below to force using the curses library: https://github.com/python/cpython/blob/master/setup.py#L1027-L1030

@michael-o
Copy link
Contributor Author

Doesn't this mean that one can completely drop the AIX handling which should now be covered by my change?

@kadler
Copy link
Contributor

kadler commented Nov 14, 2020

No. As I stated, the code you changed is not even run on AIX, since ldd does not work the same there.

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

Successfully merging this pull request may close these issues.

None yet

9 participants