Skip to content

readline: default to readline81 instead of readline6#151090

Merged
SuperSandro2000 merged 1 commit intoNixOS:stagingfrom
thefloweringash:readline-update
Nov 30, 2022
Merged

readline: default to readline81 instead of readline6#151090
SuperSandro2000 merged 1 commit intoNixOS:stagingfrom
thefloweringash:readline-update

Conversation

@thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Dec 17, 2021

Motivation for this change

I see a tig crash aarch64-darwin when attempting to search by pressing /, and rl_initialize is in the backtrace. The crash doesn't occur with the current version of readline (8.1). There's a ticket for keeping default versions current (#40015), but I couldn't find much discussion of updating readline in particular outside of a comment on that thread.

So I'm being bold and proposing to update readline to its latest version.

backtrace of tig crash
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x9f4)
    frame #0: 0x00000001002ccc6c libreadline.6.3.dylib`_rl_init_terminal_io + 272
libreadline.6.3.dylib`_rl_init_terminal_io:
->  0x1002ccc6c <+272>: ldr    w8, [x22, #0x9f4]
    0x1002ccc70 <+276>: cmp    w8, #0x0                  ; =0x0
    0x1002ccc74 <+280>: b.gt   0x1002ccc88               ; <+300>
    0x1002ccc78 <+284>: mov    w8, #0x4f
Target 0: (.tig-wrapped) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x9f4)
  * frame #0: 0x00000001002ccc6c libreadline.6.3.dylib`_rl_init_terminal_io + 272
    frame #1: 0x00000001002b7c2c libreadline.6.3.dylib`readline_initialize_everything + 164
    frame #2: 0x00000001002b6dd0 libreadline.6.3.dylib`rl_initialize + 44
    frame #3: 0x00000001002b6d48 libreadline.6.3.dylib`readline + 36
    frame #4: 0x0000000100017f58 .tig-wrapped`read_prompt + 80
    frame #5: 0x0000000100020c20 .tig-wrapped`search_view + 56
    frame #6: 0x00000001000077f8 .tig-wrapped`main + 2608
    frame #7: 0x00000001000bd0f4 dyld`start + 520
Things done
  • Built on platform(s) (Tested bashInteractive, postgresql_13, tig)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Dec 17, 2021
@thefloweringash
Copy link
Member Author

I've also found that psql from postgresql_13 behaves poorly on aarch64-darwin with old readline, so I'm looking at this again. I've rebased it onto current staging.

I don't think I have the resources to test this effectively. Do we send this to staging and see what hydra shows up?

@ofborg ofborg bot requested review from WilliButz, globin, kalbasit and mmahut March 30, 2022 05:36
@thefloweringash thefloweringash marked this pull request as ready for review March 30, 2022 08:26
@thefloweringash
Copy link
Member Author

thefloweringash commented May 30, 2022

I'm also seeing crashes in the repl on aarch64-darwin in python3.7, python3.9 and python3.11 when resizing my terminal with backtraces like:

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	       0x1bf13ad98 __pthread_kill + 8
1   libsystem_pthread.dylib       	       0x1bf16fee0 pthread_kill + 288
2   libsystem_c.dylib             	       0x1bf0aa340 abort + 168
3   libsystem_malloc.dylib        	       0x1bef8c8c0 malloc_vreport + 552
4   libsystem_malloc.dylib        	       0x1bef8ff34 malloc_report + 64
5   libsystem_malloc.dylib        	       0x1bef7ecf4 free + 300
6   ???                           	       0x10497cd98 call_readline + 720
7   ???                           	       0x104bfba50 PyOS_Readline + 228
8   ???                           	       0x104bfe384 tok_nextc + 268
9   ???                           	       0x104bfc324 tok_get + 160
10  ???                           	       0x104bfc264 _PyTokenizer_Get + 20
11  ???                           	       0x104bd1a88 _PyPegen_fill_token + 68
12  ???                           	       0x104bd695c _PyPegen_parse + 736
13  ???                           	       0x104bd2a68 _PyPegen_run_parser + 20
14  ???                           	       0x104bd2cf0 _PyPegen_run_parser_from_file_pointer + 284
15  ???                           	       0x104d47ffc PyRun_InteractiveOneObjectEx + 428
16  ???                           	       0x104d47628 _PyRun_InteractiveLoopObject + 168
17  ???                           	       0x104d474c4 _PyRun_AnyFileObject + 76
18  ???                           	       0x104d47de8 PyRun_AnyFileExFlags + 68
19  ???                           	       0x104d66f6c Py_RunMain + 2352
20  ???                           	       0x104d67154 pymain_main + 344
21  ???                           	       0x104d671e0 Py_BytesMain + 56
22  dyld                          	       0x10468d08c start + 520

Updating readline seems to make this not happen.

@tjni tjni mentioned this pull request Nov 29, 2022
13 tasks
@SuperSandro2000
Copy link
Member

@ofborg eval

@trofi
Copy link
Contributor

trofi commented Dec 18, 2022

clisp does not support readline8:

*** - FFI::FIND-FOREIGN-VARIABLE: foreign variable
  #<FOREIGN-VARIABLE "rl_readline_state" #x00007FFFF7FBC640> does not have
  the required size or alignment

Proposed fix as #206745

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

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants