ncurses: patch wrong st-0.7 terminfo

[typetetris]

Motivation for this change

ncurses 6.1 has a defective st-0.7 terminfo entry according to

tmux/tmux#1264 (comment)

which resulted in tmux crashing if it is used within st and neovim
is started inside. Which is my use case with nix on debian8. On nixos
the issue can't be reproduced as the terminfo database out of
/run/current-system is also used, which contains ncurses 6.0
on my system.

Of course it is a bit unfortunate that a correction to the terminfo database
necessitates a rebuild of so many packages :(

Things done

Applied the fix stated in the comment above and checked that the crash doesn't
occur any more.

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)

• Built on platform(s)

  • NixOS
  • macOS
  • other Linux distributions

• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)

• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  Sadly this ate all my ram and was killed by oom.

• Tested execution of all binary files (usually in ./result/bin/)

• Determined the impact on package closure size (by running nix path-info -S before and after)

• Fits CONTRIBUTING.md.

---

[xeji]

Has this patch been submitted/accepted by ncurses upstream? Is there an upstream ncurses issue we can reference?

which resulted in tmux crashing if it is used within st and neovim
is started inside. Which is my use case with nix on debian8. On nixos
the issue can't be reproduced as the terminfo database out of
/run/current-system is also used, which contains ncurses 6.0
on my system.

Sounds like a rather specific combination of things is needed to trigger this bug. I'm not sure whether this warrants using an unofficial patch for such a crucial package.

[Mic92]

Can you also send this patch upstream?

[typetetris]

Someone else already did this.

[typetetris]

@xeji The corrected attribute is an extension of the terminfo database by tmux, which is described in the manual page of tmux. As the entry for suckless terminal is corrected you need at least tmux and st to reproduce the issue. neovim is just used to trigger the bug, by using a program, which tries to do something, which yields in tmux using the entry.

@xeji @Ma27 I will investigate, wether that patch is already upstream or I try to send it upstream. But here I will need a few hours.

[typetetris]

At least the terminfo database, which can be viewed online, contains the corrected line:

st-0.7|simpleterm 0.7,
    ccc,
    dim=\E[2m,
    initc=\E]4;%p1%d;rgb\:%p2%{255}%*%{1000}%/%2.2X/%p3%{255}%*
          %{1000}%/%2.2X/%p4%{255}%*%{1000}%/%2.2X\E\\,
    kcbt@, kent@, oc=\E]104\007,
    sgr=%?%p9%t\E(0%e\E(B%;\E[0%?%p6%t;1%;%?%p2%t;4%;%?%p1%p3%|
        %t;7%;%?%p4%t;5%;%?%p5%t;2%;%?%p7%t;8%;m,
    Ms=\E]52;%p1%s;%p2%s\007, kDN3=\E[1;3B, kDN5=\E[1;5B,
    kLFT3=\E[1;3D, kLFT5=\E[1;5D, kNXT3=\E[6;3~,
    kNXT5=\E[6;5~, kPRV3=\E[5;3~, kPRV5=\E[5;5~,
    kRIT3=\E[1;3C, kRIT5=\E[1;5C, kUP3=\E[1;3A, kUP5=\E[1;5A,
    use=ecma+strikeout, use=st-0.6,

It is the line with Ms= which reads Ss= in the ncurses-6.1 distribution tarball.

Comparing the file misc/terminfo.src from the release download and the development download you see, that the patch is already upstream:

6263c6297
<       Ss=\E]52;%p1%s;%p2%s\007, kDN3=\E[1;3B, kDN5=\E[1;5B,
---
>       Ms=\E]52;%p1%s;%p2%s\007, kDN3=\E[1;3B, kDN5=\E[1;5B,

Also in the development source download is the following comment:

# 2018-02-24
#	+ correct Ss/Ms interchange in st-0.7 entry (tmux #1264) -TD

So yes, the patch is already upstream.

[Mic92]

Ok. Then please leave a short comment that the patch can be removed with the next upgrade.

[typetetris]

Added the comment, that the patch is only necessary for the 6.1 version of ncurses and needs to be removed, if the ncurses is upgraded.

[dtzWill]

Fixing/upgrading terminfo DB sounds good to me!

• Might make sense to package/ship terminfo separately? Related: (WIP) terminfo-extra: init #41544 .
  Ideally once we move to it, future terminfo updates wouod not trigger mass rebuilds...

Not to hijack this PR but if we're eating an ncurses build anyway... :)

• "While we're at it" :P, why not bump to latest ncurses release? How do we feel about that?

https://invisible-island.net/ncurses/NEWS.html#t20180804

---

IIRC while there are other goodies there is precedent for picking this st fix in particular. I was looking through all this recently re:termite and some issues I was having with screen (and TERM=termite vs TERM=xterm-termite, lol)

---

Anyway LGTM but haven't given any hands-on checking of the result.

[dtzWill]

Previously: #28334

[dtzWill]

Hmm well regarding the original issue, doesn't latest st ships a fixed terminfo? If so this might only be fixing things when using st and ssh'ing to a box with ncurses 6.1 but not at installed?

[Mic92]

Arch also ships ncurses 6.1 for a while. Upgrading makes sense then.

[typetetris]

You lost me. So you suggest to update ncurses to some weekly unreleased version?

Shipping terminfo db separately should be good in my opinion. Shouldn't it be a runtime dependency only always? Why should it be needed for building?

[typetetris]

@dtzWill Regarding the fixed terminfo that ships with st. That doesn't change anything for me, as tmux only reads the terminfo database from ncurses-6.1. I did some strace -f -ff -o tmux.strace tmux inside st, starting neovim which leads to an immediate crash for me (with nixpkg-unstable on debian8). Afterwards inspecting the straces, I see that it sometimes reads the terminfo database of ncurses 6.1 in ~/.nix-profile and sometimes directly in the nix store, but never the fixed terminfo from st.

[Mic92]

Sorry. Somehow I toughed we still have an old version of ncurses. But we already have 6.1. In this case we can just merge this one instead.