chrisant996 / clink

Bash's powerful command line editing in cmd.exe
https://chrisant996.github.io/clink/
GNU General Public License v3.0
3.58k stars 141 forks source link

Using Clink readline in windows compile of PSQL #377

Closed kirkw closed 1 year ago

kirkw commented 1 year ago

@chrisant996 I appreciate your help from before. We finally dug into this problem. We have accidentally built everything as win32 and not 64 bit.

But I was able to use Clink to compile readline, and link it into the psql, and compiled with VS2022 (community edition).

So, after figuring out how to get it linked in (so the _rl_initialize() etc was not undefined)... We get errors we THINK are because readline is referencing libc which is listed as "do not link this default library".

we are HOPING that these messages will have an OBVIOUS cause to you. Is there something we should do when compiling your readline to avoid this. was readline assuming LIBCMTD would be available (but libc is blocked directly by the PSQL configuration)

HUGE Appreciation. Please include a donate link so I can say thank you! The good news is that we went from thousands of errors... Down to a small finite number... :-)

here is the LINKER error output (when trying to link PSQL.exe to readline) (completely): readline_x86.lib(readline.obj) : warning LNK4075: ignoring '/EDITANDCONTINUE' due to '/INCREMENTAL:NO' specification 1>LINK : warning LNK4098: defaultlib 'LIBCMTD' conflicts with use of other libs; use /NODEFAULTLIB:library 1>readline_x86.lib(complete.obj) : error LNK2001: unresolved external symbol _wcwidth 1>readline_x86.lib(display.obj) : error LNK2001: unresolved external symbol _wcwidth 1>readline_x86.lib(mbutil.obj) : error LNK2001: unresolved external symbol _wcwidth 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _is_exec_ext referenced in function _hooked_lstat 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _host_clear_suggestion referenced in function _end_prompt 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _end_recognizer referenced in function _end_prompt 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _end_task_manager referenced in function _end_prompt 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _host_filter_transient_prompt referenced in function _end_prompt 1>readline_x86.lib(hooks.obj) : error LNK2019: unresolved external symbol _end_prompt_lf referenced in function _end_prompt 1>readline_x86.lib(display.obj) : error LNK2019: unresolved external symbol _tputs referenced in function rl_clear_screen 1>readline_x86.lib(terminal.obj) : error LNK2001: unresolved external symbol _tputs 1>readline_x86.lib(display.obj) : error LNK2019: unresolved external symbol _tgoto referenced in function _delete_chars 1>readline_x86.lib(display.obj) : error LNK2019: unresolved external symbol _host_on_new_line referenced in function _rl_on_new_line 1>readline_x86.lib(terminal.obj) : error LNK2019: unresolved external symbol _tgetent referenced in function __rl_init_terminal_io 1>readline_x86.lib(terminal.obj) : error LNK2019: unresolved external symbol _tgetflag referenced in function rl_init_terminal_io 1>readline_x86.lib(terminal.obj) : error LNK2019: unresolved external symbol _tgetnum referenced in function __rl_get_screen_size 1>readline_x86.lib(terminal.obj) : error LNK2019: unresolved external symbol _tgetstr referenced in function _get_term_capabilities 1>readline_x86.lib(shell.obj) : error LNK2019: unresolved external symbol _host_get_env referenced in function _sh_get_env_value 1>readline_x86.lib(xmalloc.obj) : error LNK2019: unresolved external symbol _dbgsetlabel referenced in function _xmalloc 1>readline_x86.lib(xmalloc.obj) : error LNK2019: unresolved external symbol _dbgverifylabel referenced in function _xrealloc 1>readline_x86.lib(xmalloc.obj) : error LNK2019: unresolved external symbol _dbgsetignore referenced in function _xmalloc 1>.\Debug\psql\psql.exe : fatal error LNK1120: 18 unresolved externals 1>Done Building Project "C:\postgresql-14.6\psql.vcxproj" (Rebuild target(s)) -- FAILED.

chrisant996 commented 1 year ago

I'll try to be more clear:

The reason Readline doesn't work in PSQL is because PSQL is missing the following code:

https://github.com/chrisant996/clink/blob/b7ef37f9427547e65121b9dd598a02c703f8ba5a/clink/core/src/os.cpp#L36-L43

That sets the C runtime to use UTF8. Readline assumes UTF8, because it is designed for use on Unix and Linux, which use UTF8. Code to set the C runtime to use UTF8 belongs outside of Readline.

I don't think you should try to use Clink's copy of Readline; it won't help solve your problem, and it may cause other problems. The public Readline will work fine, as long as the host remembers to set the C runtime to use UTF8, as expected by Readline.

NOTE: you don't have to literally include the cited code. The setlocale() call is the relevant part. And if PSQL is not able to use UTF8 internally, then it would need to save the current C runtime locale, call setlocale() to set UTF8 before calling Readline, and then after Readline returns call setlocale() again to restore the locale that was saved earlier.

chrisant996 commented 1 year ago

P.S. the linker errors are happening because PSQL is not Clink, and the Clink-Readline has some places where it has changes that are very specific to being hosted inside Clink. You should use the public Readline, not the Clink-Readline.

chrisant996 commented 1 year ago

P.P.S. and if Readline still doesn't handle \ or etc correctly after setting the C runtime locale to UTF8, then there is something else wrong in PSQL -- perhaps in how it translates keyboard input into key sequences suitable for consumption by Readline. You'd need to debug through the keymap dispatch stuff.

kirkw commented 1 year ago

@chrisant996 Your help is so much appreciated. We've made some good progress. We have this linking and almost working. Unfortunately we linked to a pre-built DLL (mingw64, readline8.dll). We finally have things starting to work. (readline is called and history works with C-p, up/down arrows, C-r, etc!)

We added the following code to get rid of the missing EXTERN items: (Surprised readline did not declare them, but maybe this way, it can be re-entrant?) not sure... char rl_line_buffer = (char )NULL; const char rl_readline_name; char const rl_basic_word_break_characters = (char )NULL; char const rl_completer_quote_characters = (char)NULL; int rl_completion_suppress_quote = 0; rl_completion_func_t rl_attempted_completion_function = (rl_completion_func_t*)NULL;

We have 2 hiccups: 1) TAB completion never calls the function we setup in initialize_readline(); rl_attempted_completion_function = psql_completion; rl_bind_key('\t', rl_complete); // Tried WITH and WITHOUT this, we added it 2) When we free(line); // We get a HEAP error... this is returned by: *line = readline((char )prompt);**

At this point, I believe that mingw linked with different library code (maybe). Unfortunatley, we have the pre-built with the headers and not the .c code.

We are going to get that, and build it as part of the VS2022 solution. And enable debugging.

If anything useful comes to mind... It's appreciated. (I was not sure why readline DEFINED those externals, and did not declare them. Again, I will assume it has to do with the DLL nature. I would prefer to NOT have a DLL, and just link the readline code into PSQL. I can start that process once I get it building in VS...

chrisant996 commented 1 year ago

@kirkw If you can share the following, then I can probably take a quick peek in the debugger and identify exactly what's wrong:

  1. A link to the PSQL repo.
  2. A branch that contains changes to use the public Readline library (i.e. as long as it compiles and typing A works, that's sufficient to enable debugging).
  3. A link to instructions to build PSQL. (ideally via VS2019 or mingw)
  4. Steps to reproduce each of the problems (e.g. for "backslash doesn't work" what are specific detailed steps to encounter the problem, what is the expected behavior, and what is the actual observed behavior).
kirkw commented 1 year ago

Wow, that would be great. But it's a huge ask... PSQL is a piece inside of PostgreSQL DB (the command line tool). https://www.postgresql.org/docs/current/git.html

I am working with an IT guy (my best friend), who setup the build environment. (It took 2-3 attempts, since they are accustomed to building mostly in linux). I would have to ask him to send you the instructions he used. (He is a linux guy, and knows very little about Visual Studio), so some of the other instructions on how to do this may make more sense to you.

Actually, a VS configuration to build mingw64 readline (if you have it) would get me there... I've just learned how to run the debugger and have the environment and a database to connect to.

Getting ALL of the pieces is a MAJOR Pain (I had to install A PostgreSQL server, so I have something to connect to, while using the debugger)...

I do appreciate your help.

Is it normal that I had to declare some of those variables in my code? [1000% new to working with readline(), and am more of a DB developer, and used to doing things in windows]

chrisant996 commented 1 year ago

Is it normal that I had to declare some of those variables in my code? [1000% new to working with readline(), and am more of a DB developer, and used to doing things in windows]

No, it's not normal, something is wrong somewhere.

Are there instructions for building PSQL normally with either VS or mingw on Windows? Even that would be enough to let me focus on integrating Readline, rather than having to reinvent building, from scratch. 🙃

chrisant996 commented 1 year ago

@kirkw I cloned the repo and followed the directions in the wiki (cited in src\DEVELOPERS) to build using MSVC. But there are tons of errors about missing source files.

I'm willing to debug Readline integration and help get that working. But I don't want to debug the build system itself end to end. Does it build cleanly with mingw on Windows? Or has building on Windows just fallen into disrepair in general?

kirkw commented 1 year ago

@chrisant996 yeah, EnterpriseDB (EDB) maintains the builds for windows, and the docs suck.

I have a Cloud Based setup. If you use TeamViewer, I could send you a link to join to it, and let you hit it from there. Because it's all installed, compiling/linking.

In the meantime, I have taken READLINE 8.2 source and have created a project for it. I ended up merging SOME of your changes in. But I stopped short of including the CPP code. tputs, tgetnum, wcwidth are the last 3 undefined symbols. before I can build PSQL.

I am building this to a LIB and linking it in directly. I (minutes ago) got this far. Now I see that TERMINAL functions (like these) are NOT in the MING download. I am likely going to bring your CPP code in, to resolve those. I've been hacking away at this all morning.

Let me get this /terminal code linked in from your stuff (NOT that this is the final solution). I would just like to see the basics work (tab key expansion calling psql code).

Once I have it all running inside the debugger, I will ATTEMPT to debug it. And then after that, I will gladly reach out to you.

I really appreciate your help, and your offers to help. It's crazy that this is this complicated. I now understand that READLINE is a mini editor+terminal emulator and not a simple library. And that in the natural process to port it to windows, there were a few missing pieces that had to be dealt with. This was already 10x harder than I thought it would be. (Which explains why they never went back to look at it in the last 17yrs). But I am hopeful. [and the code is hacky, but I have the ability to DIFF it against the original and rework the changes]...

Cross your fingers... :-)

chrisant996 commented 1 year ago

@kirkw Oh, yes, another thing about Readline is that it assumes it's used within the context of a terminal that supports ANSI escape sequences. It isn't a terminal emulator. The tgetstr stuff is how Readline queries the host for the capabilities of the current terminal. On *nix there is a standard mechanism for that. On Windows there is not. Which means Readline can't work on pre-Windows 8.1 without either a terminal emulator or an external tool such as ANSICON. To get past that compatibility hurdle, Clink implements a full terminal emulator internally.

So I would recommend to have a function that checks for Win 8.1 or higher, and chooses whether to use Readline at runtime based on that. And then the tgetstr (etc) from Clink would be appropriate for terminal codes on Windows.

And yes, access to a cloud environment that builds and executes would be helpful.

Another thing -- Readline has a config.h file which describes how the compile time capabilities of the current platform. So there will need to be (at least) two flavors -- whatever was already being used, and also a separate configuration for Windows (they can exist in a single config.h file, positioned via #ifdef directives).

chrisant996 commented 1 year ago

P.S. I think what's making it hard is that Readline doesn't have much documentation about certain dependencies that are present in *nix but aren't present in Windows. Once the existence of the dependencies is known, then it's just a very mechanical procedure to fill in the expected functions.

But if you have to declare variables then I suspect something is wrong with the linkage. Which might also explain why setting the completion callback function didn't seem to work -- it may be setting the variable you declared, rather than the variable Readline declared.

We'll get to the bottom of it sooner or later.

kirkw commented 1 year ago

Agreed. (FWIW, this is why finding CLINK gave me some hope that this could be solvable). It's a holiday week here in the states, so I have some time to work on this for my own learning... But there are DEFINITELY DRAGONS in here...

Let me link in your terminal code... And then try this. (Also, the build for windows excluded ALL of the stuff used for readline for linux (it appears), so a couple of things are defined in the COMMON code wcwidth/wcsidth, but not linked into the library they should compile into, that IS being linked in...

With only 1 more module to add... I am hopeful. But totally exhausted! My last push for the day... Then I have to get to the gym...

chrisant996 commented 1 year ago

Let me link in your terminal code...

Yes, but only the few functions like tgetstr. Don't link in the actual ECMA48 terminal emulator! Just have a function that chooses whether to call Readline vs whatever has been used instead, and only call Readline on Win8.1 and higher, which have the necessary terminal support built in.

I am hopeful.

We'll get there. The code itself will end up very simple.

kirkw commented 1 year ago

LOL, Perfect timing... I tried linking in "terminal/." and that just bit me. 40 unresolved items. I am done for the day. I will go back and add just the code I need.

FWIW, in the first pass, we are testing on Windows 10. Nobody on this project has anything before that! And at first, this will be internal, but I concur.

I almost wish your project was split in two... A Working Readline() for windows... And the CLINK which leveraged that!

For the last few functions... You have the references to the original code. Should I just refer to that, so I don't have to cross into the CPP arena?

chrisant996 commented 1 year ago

I almost wish your project was split in two...

...

For the last few functions... You have the references to the original code. Should I just refer to that, so I don't have to cross into the CPP arena?

@kirkw That gave me an idea, which will probably make it easier for both of us.

I'll make a standalone simple sample Windows program that uses Readline. Two, actually: a C sample program and a C++ sample program, to demonstrate details for each.

That'll be even less work than debugging what's wrong in PSQL, and it will demonstrate what PSQL needs to do.

But, the whole UTF8 thing is still a complexity that can't be escaped.

Questions:

kirkw commented 1 year ago

PSQL is all C code (no C++). They Simply Call READLINE() in the code I work with (PSQL)! [AWESOME Dude!]

But... I found these 3 references with a quick search... [I believe is UNRELATED] 99.9% confident that it does NOT call this in the GUI stuff ( this is all in /backend/ )

static void tsearch_readline_callback(void arg); stp->cb.callback = tsearch_readline_callback; tsearch_readline_callback(void arg)

// But NONE of it tied to "rl_" type readline... // No rl_optimize_typeahead either...

chrisant996 commented 1 year ago

@kirkw I made a simple C program that uses Readline.

The simple program just does the basics and gets Readline working. A host program will of course likely want to configure various aspects of Readline, and that is a matter of reading Readline documentation and writing appropriate C/C++ code, etc.


There are a few things that need to be completed before publishing it to a public github repo:

  1. Keyboard input works, but it currently relies on SetConsoleMode and ENABLE_VIRTUAL_TERMINAL_INPUT, which is only available in Windows 10 build 16299 or so. I'll need to make a simple (as much as possible) "keyboard driver" function that can translate Windows keyboard input into virtual terminal input codes as expected by Readline.
  2. ~The input line display mostly works. I've run into some display glitches, and I need to take a closer look to see why they're happening.~ Update: I resolved the display glitches; I had just forgotten to add the \x01 and \x02 characters around the escape codes in the prompt string.
  3. Due to changes in how Windows tells programs what version of Windows they're running on, a program must include a manifest that tells the OS the program knows about at least Windows 8.1 otherwise the OS will say it's running on something older. Windows 8.1 or higher is required for virtual terminal output (ANSI escape codes, which are required for updating the Readline display). So the host program will have to take responsibility for including an appropriate manifest and checking whether Readline will work properly, and then fall back to something else if not.
  4. I need to finish writing the README.md file for the sample.


Readline source modifications: Readline source code is not kept very up to date for compiling successfully for Windows.

So the sample repo will use a git submodule for the Readline repo (checked out at the readline-8.2 tag), and it will include a patch file to apply locally to get Readline sources into a state that compiles on Windows.

chrisant996 commented 1 year ago

@kirkw Since the display glitches were just a usage mistake, the sample is fully working.

However, I still need to provide a keyboard driver function so it can work on Win8.1 through early Win10 builds, and I need to finish the README.md file.

In the meantime you can clone the repo, try it out, and see what is needed. https://github.com/chrisant996/readline_win32_sample

You'll need Premake to generate build projects for VS or mingw (e.g. premake5 vs2019), and then you can build using the selected toolset. Be sure to read the README.md file, because there is a patch file that you need to apply in the readline submodule.

kirkw commented 1 year ago

@chrisant996 That's great news. How do you want to send it to me?

As an UPDATE: 1) with some hackery, I got it compiling, and "running" (cough, cough) 2) The crash on free(line); is GONE!!!

But it is currently crashing because "fd" is -1; (the file descriptor). So, I had to HACK a break point, and skip the IsTTY(fd) check, and just run _getch();

Stack is: rl_read_key() -> rl_get_char() [input.c circa line 514] then it calls c = (*rl_getc_function) (rl_instream);

    which calls rl_getc(FILE *stream)

    And this *stream is not properly initialized.
    I think in MY READLINE_INITIALIZE I need to set this to stdin somehow?
kirkw commented 1 year ago

Thank you I downloaded it to that machine...

I am looking for a quick way to initialize the rl_instream to stdin...

chrisant996 commented 1 year ago

@chrisant996 That's great news. How do you want to send it to me?

@kirkw See previous reply. I published the sample repo.

But it is currently crashing because "fd" is -1; (the file descriptor). So, I had to HACK a break point, and skip the IsTTY(fd) check, and just run _getch();

Stack is: rl_read_key() -> rl_get_char() [input.c circa line 514] then it calls c = (*rl_getc_function) (rl_instream);

    which calls rl_getc(FILE *stream)

    And this *stream is not properly initialized.
    I think in MY READLINE_INITIALIZE I need to set this to stdin somehow?

Take a look at the sample repo. I don't know exactly what is causing the problems described above, but I think they're probably self-inflicted wounds. 😉

You mentioned earlier something about needing to declare your own copies of some Readline variables. If that's still being done, then don't. That can only create problems.

chrisant996 commented 1 year ago

Also, let's move the rest of this discussion to the sample repo, since this is all off-topic with respect to Clink.