-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix Windows build #15
Comments
What breaks on windows? I don't have a windows machine on hand, so help would be appreciated. |
Here are the details, antirez/linenoise#113 it looks like that there are workarounds, i.e., versions of linenoise that we can compiler under windows (referenced in the issue). See also https://github.com/ocaml-community/ocaml-linenoise/blob/master/src/linenoise_src.c#L49 Also, I think that it would be OK, at least for starters, to just fallback dump input on windows. |
I'm swamped right now (new baby!!) but if you're able to contribute that it'd be awesome. |
I think using redox liner for example would be better on the long term. It's more manageable than a C codebase, especially wrt unicode. |
But this will require the presence of a rust compiler/package manager and will make packing of linenoise a bit convoluted. |
It would require cargo, yes. The rest of the packaging wouldn't change much (you can vendor the entire tree of the project). |
Yep, and requiring cargo is a much bigger hassle than requiring a C compiler (which is already required by opam itself). I don't really think that unfolding a new full toolchain is justified for linenoise which is considered as a lightweight drop in tool. Also, the name itself will no longer be correct :) I mean having a different readline-like utility with the redox backing is probably a good idea. But switching from linenoise to redox as the main backend for ocaml-linenoise doesn't sound that much good and will prevent at least myself from using it. Besides, I am still planning to resolve this issue, just finishing stuff from the last year. My main plan is to adopt one of the portability clones of linenoise referenced in antirez/linenoise#113 It doesn't look like a lot of work, except that it was more than 20 years ago when I built something on Windows) |
If you think you can fix the C, then power to you!! 😁 |
@ivg Do you want to take a crack at it? |
It's a bit of a mess since it looks like they're all based on different versions of the upstream linenoise, but there is a working fork of linenoise with win32 support (https://github.com/msteveb/linenoise), with some missing/fixed features in a yet to be added pull request here: msteveb/linenoise#27 . It was possible for me to get ocaml-linenoise to easily compile using this fork. |
@EmmaJaneBonestell just to make it clear, does this fork also handle the features we added, like ctrl-r? At a glance it seems like it but I'm not sure. |
@c-cube With the pull request, I believe so, yes. I haven't done any extensive testing, but it seems to work all right. It seems like it would be easier to implement/lift needed Windows code from this. The fork added lots of fixes/cleanups like 11 and 9 years ago, and I don't know if they are still relevant, should be kept, etc. (e.g. escaping in linenoiseHistoryLoad.) freehistory is named linenoiseHistoryFree and has two more lines of code for some supposed bug fixes, and so on. For anyone who comes across this and needs a working repo (not guaranteed to match all features/be bug free): |
@EmmaJaneBonestell, would you mind making a pull request to this repository? |
Currently it's marked as incompatible by @fdopen: fdopen/opam-repository-mingw@23748b9
It's required for enabling Windows builds for BAP: BinaryAnalysisPlatform/bap#1254
The text was updated successfully, but these errors were encountered: