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

[RFC] Replace GNU Readline with an alternative #97

Open
mertyildiran opened this issue Jan 4, 2021 · 5 comments
Open

[RFC] Replace GNU Readline with an alternative #97

mertyildiran opened this issue Jan 4, 2021 · 5 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@mertyildiran
Copy link
Member

GNU Readline emposing GPLv3 on Chaos and we cannot switch to BSD 3-Clause Clear License. Therefore GNU Readline needs to be replaced with one of the alternatives listed below:

@mertyildiran mertyildiran added the help wanted Extra attention is needed label Jan 4, 2021
@mertyildiran mertyildiran self-assigned this Jan 4, 2021
@apotheon
Copy link
Member

apotheon commented Feb 10, 2021

I started looking through code today, and I see references to readline in:

  • Makefile
  • requirements.sh
  • compiler/compiler.c
  • lexer/lexer.l
  • utilities/shell.c

I don't swear to those being the only references in this codebase.

edit: I may find time to play with it within the next few days.

I think libedit/editline is the best bet, because it's pretty standard in BSD Unix systems and even included in some Linux distributions. It may also be available by default on MacOS; I'm not sure, but it would certainly be available to install, at minimum.

@apotheon
Copy link
Member

apotheon commented Feb 11, 2021

It looks like I won't be able to do any work on this right now. I don't have an available system running an OS currently supported by Chaos.

It occurred to me that it might be slightly easier to migrate to linenoise, but for as much portability as you'd get with editline you might need to statically link linenoise instead of depending on OS packages.

@mertyildiran
Copy link
Member Author

@apotheon are you on OpenBSD? I don't have a BSD installed machine right now. Have you tried to compile it if so what error did you get? I guess adding BSD support to Chaos became a priority 😄

@apotheon
Copy link
Member

apotheon commented Feb 12, 2021

It is obvious just from looking at the Makefile, which leads to requirements.sh, that it is not fully portable to OpenBSD as those files currently exist. I tried skipping requirements.sh and hand-installing relevant dependencies, of course, and it seemed effective, as far as that went.

The hardcoded Bash path is incompatible with OpenBSD, and Bash would need to be added as a dependency installed from packages on OpenBSD as well (the default shells in OpenBSD are Bourne shell, aka "sh", and Korne shell, aka "ksh"). I propose an update to use /bin/sh rather than /bin/bashas the shell path, which would make the installation far more broadly portable. This roughly describes my thoughts on shell choice: http://blogstrapping.com/2013.271.13.19.30

Changing from /bin/bash to /bin/sh would require some minor syntactic changes that I noticed, but would then remain consistent with Bash, because the Bash-specific syntax for those changes I stumbled onto are redundant with syntax that also works with Bourne shell. I do not know off the top of my head whether there are actual features particular to Bash that would also need to be worked around. (edit: I created issue #100 for Bash-to-Bourne after posting this comment, and I suppose you can choose whether to dismiss it in favor of programmatically determining the Bash path where needed instead.)

In the short term, I changed the /bin/bash path to /usr/local/bin/bash (revealing additional problems, addressed below).

A work-around for getting it to run at all included running gmake instead of make, because the default make implementation on OpenBSD (and other BSD Unix systems) is bsdmake instead of GNU make. Note that the default compiler on amd64 and x86 OpenBSD systems is Clang, so I used the clang make target for compilation attempts.

In both compiler/compiler.h and utilities/messages.h I realized OS detection defaulted to assumed Linux interfaces if it did not match Apple or Windows OSes. I created an alternative for setting the limits include to sys/syslimits.h as an off-the-top-of-the-head hack to short circuit before getting to the linux/limits.h configuration. I'm not sure yet whether this was effective, and do not recall for sure whether the needed limits are in limits.h or syslimits.h on OpenBSD, but it did change the errors I got in a way that suggested an improvement:

utilities/shell.c:114:5: error: implicit declaration of function 'rl_bind_keyseq' is
      invalid in C99 [-Werror,-Wimplicit-function-declaration]
    rl_bind_keyseq("\\e[A", up_arrow_key_pressed);
    ^
utilities/shell.c:114:5: note: did you mean 'rl_bind_key'?
/usr/include/readline/readline.h:289:12: note: 'rl_bind_key' declared here
extern int rl_bind_key PARAMS((int, rl_command_func_t *));
           ^
1 error generated.
interpreter/extension.c:106:20: error: use of undeclared identifier 'RTLD_NODELETE'
    dylib.handle = OPENLIB(dynamic_library_path);
                   ^
interpreter/extension.h:37:58: note: expanded from macro 'OPENLIB'
#   define OPENLIB(libname) dlopen((libname), RTLD_NOW | RTLD_NODELETE)
                                                         ^
1 error generated.
gmake[1]: *** [Makefile:87: chaos] Error 1
gmake[1]: Leaving directory '/home/ren/src/github/chaos'
gmake: *** [Makefile:37: clang] Error 2

At this point, the exercise was beginning to look like a rabbit hole of time spent that I did not want to undertake at this time, especially when Chaos is still not as freely licensed as I'd prefer, though, so I figured I would either wait until I might come up with a Linux-based dev system at some point in the future or let you handle things for now.

If you want to make changes to deal with the identified issue in requirements.sh, I may try updating the local checkout and running the install so I can report back on efficacy.

@mertyildiran
Copy link
Member Author

@apotheon thank you so much for your detailed response. I'll work on the BSD support and converting the Bash scripts into more common shell scripts as soon as I got an OpenBSD up and running. I saw #100 thanks for opening up this issue.

@SamuelMarks SamuelMarks mentioned this issue Oct 22, 2021
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants