readline: link to ncurses instead of termcap; drop termcap - #31018
readline: link to ncurses instead of termcap; drop termcap#31018dragon-archer wants to merge 3 commits into
Conversation
This aligns with MSYS-packages and Arch.
It's so old that cannot be built with current GCC without massive patching. The last user (readline) is gone now. Besides, MSYS2-packages and Arch all dropped it.
|
lgtm @mmuetzel any objections? |
|
I'm currently away from a PC. Can't really test. |
Tested with BTW, it seemes like |
|
@mmuetzel we can wait for you to test too if you want |
|
I meant "readline". But autocorrect on my mobile thought it knew better...
I'll probably be able to run some tests myself by the end of the week. |
| bash_cv_termcap_lib=libncurses \ | ||
| CFLAGS="${CFLAGS} -DNCURSES_STATIC" | ||
|
|
||
| make SHLIB_LIBS=-lncursesw |
There was a problem hiding this comment.
Maybe, there is a good reason for this. But why are you telling the configure script to prepare for a different library variant (libncurses) compared to the one that you link to (libncursesw)?
Wouldn't it be more consistent to use the same for the configuration and make command? I.e., bash_cv_termcap_lib=libncursesw.
The same for the shared library.
If I read the readline repository correctly, the generated pkg-config file (and potentially other things) will be wrong with the changes from here. It is likely referencing the ncurses module while it should be referencing the ncursesw module.
Afaics, the MINGW ncurses packages don't even install a ncurses.pc file. The ncurses package for the MSYS2 environment installs it in the ncurses-devel package.
That might be a different issue though.
Looking at the readline repository, setting bash_cv_termcap_lib=libncursesw might require additional changes so the correct termcap.h is being used. It currently has Windows-specific checks only for libncurses.
There was a problem hiding this comment.
To check if there is an issue with the generated pkg-config file: What does pkg-config --static --libs readline do with the package from here?
There was a problem hiding this comment.
Back on a PC.
The pkg-config file is broken indeed:
$ pkg-config --libs readline
Package ncurses was not found in the pkg-config search path.
Perhaps you should add the directory containing `ncurses.pc'
to the PKG_CONFIG_PATH environment variable
Package 'ncurses', required by 'readline', not found
There was a problem hiding this comment.
Yes, the pkg-config is broken now, manually editting the Requires.private: ncurses to ncursesw can fix it. I'll look into lt later, and try whether this can be fixed in make, or maybe have to be done by sed
There was a problem hiding this comment.
I think it's easier to sed readline.pc, as the configure has several special handling for ncurses but not for ncursesw.
There was a problem hiding this comment.
I suspect the reason why we copy ncursew into ncurse is that most "Unixy" applications doesn't care about wide character support, therefore many of them only use ncurses. OTOH, on Windows wide character is usually preferred. Considering ncursesw works well to emulate ncurses now, I think there's little reason for us to distribute a real ncurses in the near future, so the patch here shouldn't be a big problem.
There was a problem hiding this comment.
That's not describing the situation on posixy platforms entirely correctly. The difference between ncurses is not only single byte character width vs. wide character C types. More importantly for those platforms, it is the support for UTF-8 (that is present in ncursesw but not (completely) in ncurses).
If I recall correctly, the difference matters, e.g. in this case: If you are using an application that does not support UTF-8 (but expects a "single byte locale"), the highest bit is "discarded" with ncursesw (essentially limiting support to the ASCII character range). With ncurses (and the locale set correctly), you can use the whole range of characters in that locale.
I still don't think it is a good idea to use this "Frankenstein-configuration" where the ncursesw library is mixed with the headers of ncurses.
There was a problem hiding this comment.
Then what do you think should we do? readline can't compile successfully with bash_cv_termcap_lib=libncursew, because many of its configure doesn't treat ncursew as almost similar to ncurse. And if you really heavily patch to it add this part, then what's the difference between the status quo?
There was a problem hiding this comment.
Then what do you think should we do?
What I wrote about 3 weeks ago:
Looking at the readline repository, setting bash_cv_termcap_lib=libncursesw might require additional changes so the correct termcap.h is being used. It currently has Windows-specific checks only for libncurses.
There was a problem hiding this comment.
what's the difference between the status quo?
There are still quite a few issues when it comes to supporting non-ASCII characters in packages distributed by MSYS2. I don't know if any (or how many) of these are caused because we "sell" them an ncurses library that is actually ncursesw.
Maybe, it would be better to be "honest" and actually distribute different versions for ncurses and ncursesw.
Maybe, doing that would cause a lot of other problems. Hard to tell without having any background about why the status quo is as it is.
In any case, we shouldn't rely on the current "happenstance" when updating build rules for existing packages imho.
3 participants
This aligns with MSYS-packages and Arch.
termcap is so old that cannot be built with current GCC without massive patching. The last user (readline) is gone now. Besides, MSYS2-packages and Arch all dropped it.