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

Remove readline from lua #80

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

Remove readline from lua #80

wants to merge 1 commit into from

Conversation

blt
Copy link

@blt blt commented Apr 7, 2017

The cernan use-case for lua is one that does not use readline. On
systems that attempt to statically linked cernan against MUSL libc the
readline dependency breaks the build on account of the difficulty of
building readline in that environment. I could not determine if there
were a cleaner way to disable readline other than but permanently.
That is, maybe this approach is a hack.

I think disabling readline is okay here as the focus of the library
appears to be the use of lua in a purely embedded environment.

Signed-off-by: Brian L. Troutwine blt@postmates.com

@sagebind
Copy link
Contributor

sagebind commented Apr 7, 2017

I think readline is only used for the lua command-line interpreter, right?

@blt
Copy link
Author

blt commented Apr 7, 2017

@sagebind That's my understanding, yeah. I went through the C source to confirm and believe that to be correct.

The cernan use-case for lua is one that does not use readline. On
systems that attempt to statically linked cernan against MUSL libc the
readline dependency breaks the build on account of the difficulty of
building readline in that environment. I could not determine if there
were a cleaner way to disable readline other than but permanently.
That is, maybe this approach is a hack.

I _think_ disabling readline is okay here as the focus of the library
appears to be the use of lua in a purely embedded environment.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>
@SirVer
Copy link
Contributor

SirVer commented Jan 20, 2018

Readline also gives me a headache using this library with https://github.com/japaric/trust, I would appreciate if this would be merged!

@blt
Copy link
Author

blt commented Jan 20, 2018

@SirVer I forked this library to https://github.com/blt/mond which removes the readline issue per this PR, updates the codebase some based on clippy suggestions. I aim to do more work in support of https://github.com/postmates/cernan.

Perhaps mond will work for you? It's on crates if you want to give it a try.

@SirVer
Copy link
Contributor

SirVer commented Jan 20, 2018

Unfortunately, mond still contains the problem I describe in #91, so it is only a partial fix for me. I maintain my own fork (but retaining the fork history) with the changes I needed to make to support the architectures I care about.

I have been watching your talk about cernan and am very intrigued by it. I hope I get a chance to test it out at work.

@blt
Copy link
Author

blt commented Jan 20, 2018

Unfortunately, mond still contains the problem I describe in #91, so it is only a partial fix for me. I maintain my own fork (but retaining the fork history) with the changes I needed to make to support the architectures I care about.

@SirVer I'd be happy to incorporate your work into mond.

I have been watching your talk about cernan and am very intrigued by it. I hope I get a chance to test it out at work.

Thanks! Let us know if you run into any issues and we'll work them.

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

Successfully merging this pull request may close these issues.

None yet

3 participants