Repository navigation
Conversation
… shell On NFS (the 42 machines' sgoinfre) a deleted file that a process still has open stays behind as .nfsXXXX until that process exits, and removing it fails with "Device or resource busy". With any shell started from build/bin still running, fclean's final `rm -rf build` hit exactly that file, so `make re` stopped before compiling anything and left no binary. fclean now accepts a build tree reduced to such files: it says which one it kept and succeeds; the rebuild is unaffected, since the file has a name of its own. Anything else that cannot be removed is retried and fails the target as before. tests/fclean_nfs_test.py cuts the recipe out of the Makefile and runs it next to the repo: with only a held-open .nfs file fclean succeeds and names it; with an unwritable directory it still fails. Off NFS no .nfs file appears and the test reports SKIP. Verified on sgoinfre: `make re` exit 2 before, exit 0 after, with a shell from build/bin running.
ASan reported a leak at exit after an ordinary editing session: 32 bytes plus the typed text, allocated by readline's rl_add_undo. Two paths in readline 8.1 (Ubuntu 22.04's, what the 42 machines link) lose an undo list once per line, and the in-process reader made them the shell's: - Saved line. Walking the history from a typed line with previous-history (Ctrl-P, or Up when prefix search is off) saves that line, with its undo list as data, so Down can bring it back. Ended on the recalled entry (Enter, ^C, ^G), the saved copy outlives the read and the next readline() frees it without its data. hist_rl_reclaim_saved() now runs before each read and takes the list back with public calls only: rl_maybe_save_line() so there is a saved line (unsave rings the bell otherwise), rl_maybe_unsave_line() to move its data into rl_undo_list, rl_free_undo_list() to free it, which also clears any history entry still pointing at it. - Aborted search. Alt-N / Alt-P (stock non-incremental search) reads its string with an undo list of its own; aborting restores the saved line over rl_undo_list. rl_nsearch_reclaim() frees that list in the reader when the byte readline is about to get ends the search: ^G, ^C, Rubout or ^H on an empty string, a failed read. A readline that frees it itself finds NULL. Marked Caveat: the abort keys mirror 8.1's search.c. Both predate #136/#140 (same report on 134ac5a); prefix search made the first one the everyday path on a non-PIE build, where Up was plain previous-history. tests/rl_undo_leak_test.py runs 10 leaking key sequences, each in a fresh shell with detect_leaks=1, plus 3 checks that the keys still work (Up then Down restores the typed line, Alt-P finds, ^G restores). Before: 11 failed; after: 0. SKIP on a binary built without ASan.
On the 42 machines Up stayed plain history and Right / End / ^F / Alt-F / Alt-N never touched the ghost: none of #136's or #140's keys were installed. rl_slot_arm() decides whether a slot still holds readline's stock function by comparing pointers, and took the stock ones by address (&rl_get_previous_history, ...). cc there is clang 12, which links non-PIE, where such an address is the executable's PLT stub; Ubuntu's libreadline is linked -Bsymbolic-functions and keeps its own internal address in the keymaps. The two never compared equal, every slot read as a user binding, and slot_user kept it that way for the session. gcc (CI) and the static release build link PIE, so neither showed it. rl_slot_funcs() now asks readline for its functions by name (rl_named_function), the pointer its keymaps and funmap hold, for the stock side and for the history-search pair that replaces Up / Down. The latter also matters for readline's own rl_last_func check: a search bound by stub address would restart on every Up instead of walking back. Verified on a non-PIE debug build (file: "LSB executable"), both readers: history_prefix_search_test 57 failed -> 148 ok / 1 failed, the one being bind -p against the oracle's bundled readline 8.3 (fails on develop too); autosuggest_test 86 failed -> 346 ok / 0 failed. A PIE build passes before and after, so CI cannot see this without a non-PIE job.
oh-my-zsh's sudo widget runs `builtin which -a "$cmd"` on every Esc Esc once $EDITOR is set, and hellish had no `which` builtin: each toggle printed "builtin: which: not a shell builtin" over the prompt, and an editor reached only through that check (`vim f` with EDITOR set to a second vim on $PATH) became `sudo vim f` instead of `sudo -e f`. `which [-ap] name...` is zsh's (whence -c): alias, reserved word, function, builtin, then the command; -a lists every match, -p paths only; "NAME not found" on stdout and status 1, as zsh. Diffed against zsh 5.8.1 on 20 invocations: identical apart from the error prefix. It resolves in zsh mode only. `which` is also /usr/bin/which, and a bash script's `which`, `type which` and `command -v which` must keep meaning that one, so it sits in a separate table that builtin_func() consults only under zsh_mode(). builtin_func() now takes the shell state for that; every caller already had it, and the compiler found them all. declare -f's body trimming moves to func_text_trimmed() so `which` prints a function with the same text. Caveats in the code: functions print as written rather than deparsed; the reserved words are the POSIX set. tests/omz_sudo_widget_test.py adds the second-vim case and fails on "not a shell builtin" anywhere: 2 failed before, 11 ok after. Golden suite 5363/5363; zsh_builtins, zle and plugin_corpus tests pass. wiki/builtins regenerated (it also picks up `bind`, which was missing).
The welcome panel's "Did you know?" block becomes "What's new in 3.2": one row per new feature, written as the keys that reach it, then the switch for inline hints. The link to wiki/whats-new.md (new, and in the site nav) sits under the logo. The left column is a fixed 43 columns, so the URL shows in full at any width. Every right-hand row now fits in the 32 cells the right column has on an 80-column terminal. The 3.1 rows, and two "Getting started" rows, were written for about 40 and were cut with an ellipsis there. Under LANG=C, the identity line printed a UTF-8 middle dot that header_width() cannot decode, so it stopped counting and the frame went out of line. In that locale it is now a dash. HELLISH_HEADER_REV goes to 4, so users who saw the 3.1 panel see this one. banner_update_test.py had the revision hard-coded as 3; it now reads it from incs/version.h. tests/banner_news_test.py renders the panel at 80 and 120 columns, in UTF-8 and in C. It checks four things: - every framed row is the terminal's width; - no news row is clipped; - the guide link shows in full; - the guide is in mkdocs.yml's nav and names every key the banner shows. On the 3.1.3 build it fails 7 checks, the LANG=C frame among them. Lengthening one row makes it fail at 80 columns only. Also: the autosuggestions entry on interactive.md's roadmap is now marked shipped.
fix(build): make re no longer stops on a file NFS keeps for a running shell
fix(editor): prefix search and suggestion keys on non-PIE builds
fix(editor): free the two undo lists readline 8.1 drops on the floor
fix(zsh): `which` is a builtin in zsh mode, so Esc Esc stops erroring
feat(banner): show what is new in 3.2, readable at 80 columns
… bind
Prefix history search, opt-in inline hints, make <TAB>, bind / bind -x,
bindkey's zsh notation, ${var@P}, $SRANDOM and $EPOCHREALTIME. Also the
fixes that made the hint and prefix keys work on non-PIE builds, two
readline undo-list leaks, zsh-mode which, and make re on NFS.
RELEASE.md has the summary; wiki/whats-new.md, which the banner links
to, has the long form.
release: 3.2.0 — Up finds what you typed, a hint is one key away, and bind
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #
How I verified it
make re(SAFE=1 debug) builds clean, no warningsmake re OPT=1(SAFE=0 optimized) builds clean, no warningsmake testis green (libc heap)cd tests && ./verify_alloc.shgreen on both SAFE=1 and SAFE=0HELLISH_ALLOC_STATS(SAFE=0)
huge args, deep nesting)
make normclean on touched files (no in-function comments, ≤80 cols,no new globals)
Notes / trade-offs