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

Readline on OpenBSD has a different API; oils-for-unix compiler error #1942

Open
dontlaugh opened this issue Apr 21, 2024 · 17 comments
Open

Comments

@dontlaugh
Copy link
Contributor

dontlaugh commented Apr 21, 2024

System Details

  • OpenBSD 7.5 GENERIC.MP#138 arm64
  • oils-for-unix release tarball 0.21.0

Error

The compiler starts erroring due to some missing symbols related to readline.

CXX cpp/frontend_pyreadline.cc
cpp/frontend_pyreadline.cc:123:9: error: use of undeclared identifier 'rl_callback_sigcleanup'
        rl_callback_sigcleanup();
        ^
cpp/frontend_pyreadline.cc:276:3: error: use of undeclared identifier 'rl_clear_history'; did you mean 'clear_history'?
  rl_clear_history();
  ^~~~~~~~~~~~~~~~
  clear_history
cpp/frontend_pyreadline.cc:274:16: note: 'clear_history' declared here
void Readline::clear_history() {
               ^
cpp/frontend_pyreadline.cc:288:21: error: use of undeclared identifier 'free_history_entry'
  histdata_t data = free_history_entry(entry);
                    ^
3 errors generated.

The rl_clear_history function seems to be called clear_history in readline/readline.h. Oops that's Oils code

Ripgreppin' through /usr led me to this comment in OpenBSD's Bash NEWS file (a changelog)

2.  New Features in Readline

a.  Many changes to the signal handling:
        o Readline now catches SIGQUIT and cleans up the tty before returning;
        o A new variable, rl_catch_signals, is available to application writers 
          to indicate to readline whether or not it should install its own
          signal handlers for SIGINT, SIGTERM, SIGQUIT, SIGALRM, SIGTSTP,
          SIGTTIN, and SIGTTOU;
        o A new variable, rl_catch_sigwinch, is available to application
          writers to indicate to readline whether or not it should install its
          own signal handler for SIGWINCH, which will chain to the calling
          applications's SIGWINCH handler, if one is installed;
        o There is a new function, rl_free_line_state, for application signal
          handlers to call to free up the state associated with the current
          line after receiving a signal;
        o There is a new function, rl_cleanup_after_signal, to clean up the
          display and terminal state after receiving a signal;
        o There is a new function, rl_reset_after_signal, to reinitialize the
          terminal and display state after an application signal handler
          returns and readline continues

b.  There is a new function, rl_resize_terminal, to reset readline's idea of
    the screen size after a SIGWINCH.

c.  New public functions: rl_save_prompt and rl_restore_prompt.  These were
    previously private functions with a `_' prefix.

d.  New function hook: rl_pre_input_hook, called just before readline starts
    reading input, after initialization.

e.  New function hook: rl_display_matches_hook, called when readline would
    display the list of completion matches.  The new function
    rl_display_match_list is what readline uses internally, and is available
    for use by application functions called via this hook.

So there seem to be a lot of differences related to signal handling, so they ought to be approached with care.

@dontlaugh
Copy link
Contributor Author

A workaround is to compile without readline and use rlwrap

./configure --without-readline
./_build/oils.sh
doas ./install

Needs hacks from #1943 to build/install. After that, invoke with rlwrap

% rlwrap osh         
osh-0.21.0$ echo $HOME
/home/coleman
osh-0.21.0$ 

@andychu
Copy link
Contributor

andychu commented Apr 22, 2024

Hm do you know what version of GNU readline is on OpenBSD?

I am not sure what versions we support, but we should document it

It looks like the current version is 8.2 - https://tiswww.case.edu/php/chet/readline/rltop.html

@dontlaugh
Copy link
Contributor Author

Looks like 8.2 on my system:

% pkg_info readline
Information for https://ftp.openbsd.org/pub/OpenBSD/7.5/packages/aarch64/readline-8.2.tgz

Comment:
library to edit command lines as they are typed in

Description:
The GNU Readline library provides a set of functions for use by
applications that allow users to edit command lines as they are typed
in.  Both Emacs and vi editing modes are available. The Readline library
includes additional functions to maintain a list of previously-entered
command lines to recall and perhaps reedit those lines, and perform
csh-like history expansion on previous commands.

The history facilities are also placed into a separate library, the
History library, as part of the build process.  The History library may
be used without Readline in applications which desire its capabilities.

To avoid any confusion with the base readline/history library make sure
you link to ereadline and ehistory.  In the headers you can check for
RL_READLINE_VERSION, RL_VERSION_MAJOR and RL_VERSION_MINOR to see which
header you have picked up.

Maintainer: The OpenBSD ports mailing-list <ports@openbsd.org>

WWW: https://tiswww.case.edu/php/chet/readline/rltop.html

@andychu
Copy link
Contributor

andychu commented Apr 26, 2024

Hmm I'm surprised that a newer version is not working... I have readline 7.0 on my Ubuntu machine

I wonder if this is reproducible on Linux with readline 8.2

@andychu
Copy link
Contributor

andychu commented Apr 26, 2024

Oh I wonder if you can do

$ head _build/detected*
==> _build/detected-config.h <==
#define SIZEOF_INT 4
#define SIZEOF_LONG 8
#define SIZEOF_VOID_P 8
#define SIZEOF_SHORT 2
#define SIZEOF_FLOAT 4
#define SIZEOF_DOUBLE 8
#define SIZEOF_SIZE_T 8
#define SIZEOF_FPOS_T 16
#define SIZEOF_PID_T 4
#define SIZEOF_OFF_T 8

==> _build/detected-config.sh <==
HAVE_READLINE=1
READLINE_DIR=
PREFIX=/usr/local
DATAROOTDIR=/usr/local/share
STRIP_FLAGS=--gc-sections

==> _build/detected-cpp-config.h <==
#define HAVE_READLINE 1
#define HAVE_PWENT 1

That will show what your HAVE_READLINE setting is

Maybe that test is broken on OpenBSD somehow?

@dontlaugh
Copy link
Contributor Author

dontlaugh commented Apr 27, 2024

I think you might be onto something Doh! I forgot I had files from --without-readline on disk. Here is a new configure run.

0 % ./configure 
./configure: Wrote _build/detected-cpp-config.h
./configure: Wrote _build/detected-config.sh
./configure: Wrote _build/detected-config.h
                                                                                                                                                                                                
0 % head _build/detected*
==> _build/detected-config.h <==
#define SIZEOF_INT 4
#define SIZEOF_LONG 8
#define SIZEOF_VOID_P 8
#define SIZEOF_SHORT 2
#define SIZEOF_FLOAT 4
#define SIZEOF_DOUBLE 8
#define SIZEOF_SIZE_T 8
#define SIZEOF_FPOS_T 8
#define SIZEOF_PID_T 4
#define SIZEOF_OFF_T 8

==> _build/detected-config.sh <==
HAVE_READLINE=1
READLINE_DIR=
PREFIX=/usr/local
DATAROOTDIR=/usr/local/share
STRIP_FLAGS=--gc-sections

==> _build/detected-cpp-config.h <==
#define HAVE_READLINE 1
#define HAVE_PWENT 1

There still might be something wrong with feature detection, but I'm not sure what it is. OpenBSD puts the headers here:

% ls /usr/include/readline        
chardefs.h  history.h  keymaps.h  readline.h  rlconf.h  rlstdc.h  rltypedefs.h  tilde.h

I wonder if this is reproducible on Linux with readline 8.2

Not on my Linux laptop. I have compiled Oils without any issues on my Void Linux machine, and it has readline 8.2

Perhaps OpenBSD patches readline.

@dontlaugh
Copy link
Contributor Author

okay whoa I asked on libera #openbsd and got the lowdown.

pkg_info is not showing my current readline. the readline in BASE is much older

#define RL_READLINE_VERSION     0x0403          /* Readline 4.3 */

@dontlaugh
Copy link
Contributor Author

dontlaugh commented Apr 27, 2024

So I found some more info. The readline in base is quite old, and a lot of gnu stuff in OpenBSD base is old.

However, readline 8.2 is available from the ports tree.

https://github.com/openbsd/ports/blob/master/devel/readline/Makefile#L31

I installed it with

doas pkg_add readline

Anything not in base gets installed under /usr/local. As you can see from the ports tree's makefile for readline, they also rename the library to ereadline. So some build script updates will need to happen.

Headers get installed here:

ls /usr/local/include/ereadline/readline 
chardefs.h  history.h  keymaps.h  readline.h  rlconf.h  rlstdc.h  rltypedefs.h  tilde.h

Example programs (?) here

ls /usr/local/share/readline 
excallback.c  fileman.c  histexamp.c  manexamp.c  rl-fgets.c  rl.c  rlbasic.c  rlcat.c  rlevent.c  rlptytest.c  rltest.c  rlversion.c

And libraries here

ls /usr/local/lib | grep readline
libereadline.a
libereadline.so.3.0

And it's got the symbols

output from nm
 % nm /usr/local/lib/libereadline.so.3.0 | awk '/(_callback|_sigcleanup|clear_history)/'
0005a82c T _rl_arg_callback
0008ea08 B _rl_callback_data
00054ddc T _rl_callback_data_alloc
00054d1c T _rl_callback_data_dispose
0008ea00 B _rl_callback_func
00059a14 t _rl_char_search_callback
0003f748 t _rl_complete_sigcleanup
00032b1c T _rl_dispatch_callback
00058624 t _rl_insert_next_callback
00047818 T _rl_isearch_callback
0003aa5c T _rl_nsearch_callback
0008e748 B _rl_sigcleanup
00038dc8 t _rl_vi_callback_change_char
000386c8 t _rl_vi_callback_char_search
000398a0 t _rl_vi_callback_goto_mark
00039778 t _rl_vi_callback_set_mark
000370b0 T _rl_vi_domove_callback
0005cb90 T clear_history
0005476c T rl_callback_handler_install
00054d24 T rl_callback_handler_remove
00054850 T rl_callback_read_char
0008ea20 b rl_callback_read_char.olevel
00054e38 T rl_callback_sigcleanup
0005b058 T rl_clear_history
0003783c t rl_domove_motion_callback
00037120 t rl_domove_read_callback

So it seems like you need to include/link against ereadline on OpenBSD.

@dontlaugh
Copy link
Contributor Author

dontlaugh commented Apr 27, 2024

Here is the ports entry https://github.com/openbsd/ports/blob/9c275c5eec69b00ca423afc88f72f39a640dfb1d/devel/readline/pkg/DESCR#L12

ereadline and perhaps also ehistory will be required.


On the other hand, pkg-config shows this.

% pkg-config --libs readline
-L/usr/local/lib -lreadline

% pkg-config --libs history               
-L/usr/local/lib -lhistory

% pkg-config --libs ereadline
Package ereadline was not found in the pkg-config search path

Update: currently manually hacking the readline paths in build/ninja-rules-cpp.sh to see if I can get oils to compile. That can help determine what changes the configure script needs, if any.

Update 2: adding -lreadline to $link_flags works, but -lhistory fails to link. -I/usr/local/include/ereadline seems to find the headers.

@andychu
Copy link
Contributor

andychu commented Apr 29, 2024

Oh thanks for trying it! Yes I'd be interested in the diff of what works

I don't have an OpenBSD machine to test on

Do you need -lhistory? is that separate on OpenBSD?

@dontlaugh
Copy link
Contributor Author

Do you need -lhistory? is that separate on OpenBSD?

Yes, according to the DESC file in the ports entry for readline:

The history facilities are also placed into a separate library, the
History library, as part of the build process. The History library may
be used without Readline in applications which desire its capabilities.

To avoid any confusion with the base readline/history library make sure
you link to ereadline and ehistory. In the headers you can check for
RL_READLINE_VERSION, RL_VERSION_MAJOR and RL_VERSION_MINOR to see which
header you have picked up.

I'm having some trouble getting it to link, though. Even though the compiled libraries are right next to the readline ones

% ls /usr/local/lib | grep history
libehistory.a
libehistory.so.3.0

@andychu
Copy link
Contributor

andychu commented Apr 29, 2024

Oh it has to be -lehistory not -lhistory ? confusingly the flag has to match whatever's after lib I think

@dontlaugh
Copy link
Contributor Author

dontlaugh commented Apr 30, 2024

Not sure :( pkg-config seems to indicate it's -lhistory

pkg-config .pc files
% cat /usr/local/lib/pkgconfig/readline.pc 
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: Readline
Description: Gnu Readline library for command line editing
URL: http://tiswww.cwru.edu/php/chet/readline/rltop.html
Version: 8.2
Requires.private: termcap
Libs: -L${libdir} -lreadline
Cflags: -I${includedir}

                                                                                                      
% cat /usr/local/lib/pkgconfig/history.pc 
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: History
Description: Gnu History library for managing previously-entered lines
URL: http://tiswww.cwru.edu/php/chet/readline/rltop.html
Version: 8.2
Libs: -L${libdir} -lhistory
Cflags: -I${includedir}

I might have to try a simplified C program to see precisely what's required to include these libraries.

@andychu
Copy link
Contributor

andychu commented May 8, 2024

Yeah I agree the best solution here is to produce a self-contained C program on OpenBSD that links GNU readline and calls a few functions

We have a similar problem on FreeBSD

I would like this to work, but I don't have access to either OpenBSD or FreeBSD machines

Description of how Oils works here:

https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/installing.20on.20FreeBSD

So if there's a simple C program example, then it can probably ported over to our build system pretty easily

@andychu
Copy link
Contributor

andychu commented May 8, 2024


Or I guess another solution is to use pkg-config ? Is that the standard on OpenBSD ?

I'm unclear about this

@dontlaugh
Copy link
Contributor Author

I'm not sure if pkg-config is standard, but some libraries drop .pc files and stuff.

Regarding CI it seems like SourceHut has FreeBSD and OpenBSD images. I've been shopping around for a solution, myself, since I've become BSD-curious.

I haven't had time to work this issue in particular, but I haven't forgotten. Feel free to poke me again if I don't get back to you in ... a few weeks? ❤️

@greggyb
Copy link

greggyb commented May 9, 2024

I'm planning on getting a POC C program with readline on FreeBSD (from the Zulip thread Andy linked earlier), probably this weekend, in a Sourcehut build. I'll also give OpenBSD a try at the same time. And I'll update this issue with my results.

POC program and compilation: https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/installing.20on.20FreeBSD/near/437738525

POC program and compilation copied below:

#include <stdio.h>
#include <stdlib.h>

#include "readline/readline.h"

int main() {
    char *line;
    line = readline("Enter your name: ");
    printf("Hello, %s!\n", line);
    free(line);
    return 0;
}
# FreeBSD compilation
$ cc -I/usr/local/include -L/usr/local/lib -l readline r.c -o r
$ ./r
Enter your name: a
Hello, a!

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

No branches or pull requests

3 participants