gh-136687: allow manually excluding ncurses from the build#136696
gh-136687: allow manually excluding ncurses from the build#136696duaneg wants to merge 5 commits into
Conversation
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
Please, don't forget to update Doc/using/configure.rst with the new option.
I haven't reviewed the AC code, because I am not an expert there :)
…pping building the python curses module as opposed to not using the system ncurses library. Remove mistakenly repeated test modules section. Document configure option.
Ah, thanks! I didn't spot those docs, updated. On review I think it would be better to think of this from the higher-level user perspective of excluding the Although that does prompt the question of whether we should allow excluding other modules at build time. I'm not sure if this has been discussed before or would be considered useful.
As it turns out, I had made a silly mistake and left a copy & pasted chunk of code in there, so I'm glad you didn't review it as yet 🙂 A tip for reviewing: most of the |
|
This PR is stale because it has been open for 30 days with no activity. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Please resolve conflicts. A large amount of probes was added recently.
| AC_ARG_ENABLE([curses], [AS_HELP_STRING( | ||
| [--disable-curses], | ||
| [disable building curses module (default is no)])], [ | ||
| AS_VAR_IF([enable_curses], [yes], [Py_CHECK_NCURSES=yes], [Py_CHECK_NCURSES=no]) |
There was a problem hiding this comment.
"curses" or "ncurses"? Inconsistent.
There was a problem hiding this comment.
One ("curses") refers to the python module (whether to build it or not), while the other ("ncurses") refers to the system library (whether to look for it or not), so it is the referents that are inconsistent. I thought a higher-level option was better as a configure flag, with the existing system library check variable being essentially an implementation detail.
However, the inconsistency isn't ideal, agreed. If you prefer I'll switch the configure option back to ncurses. Or add a comment, if that would suffice.
There was a problem hiding this comment.
The system library is not necessary ncurses.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Documentation build overview
|
|
I am sorry, but I changed the configure file again, breaking your PR. |
Resolve conflicts in configure.ac/configure: the curses probes added on main (nofilter, has_mouse, is_keypad, is_leaveok, new_prescr, use_screen, use_window, key_defined, term_attrs, define_key, keyok, set_escdelay, set_tabsize, the getmouse X/Open signature check, ESCDELAY/TABSIZE, scr_dump) now sit inside the new --disable-curses (Py_CHECK_NCURSES) conditional, so they are skipped when curses is disabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
I have resolved conflicts, but the question about ncurses remains. |
3 participants
Add an option to manually exclude
ncursesfrom the build.