Fixing a neovim bug in Termux

andrew@paon.wtf

Termux

These days when I travel, I bring my Galaxy Tab S7 instead of a laptop. It’s a great travel companion: it’s lightweight, the external keyboard is big enough to be comfortable, it has a big bright display for movies and games, the battery lasts a long time. And most importantly: it has Termux.

Termux is a terminal emulator for Android. It allows me to work on my tablet in much the same way that I work on my Linux machine at home. It has its own package manager, which lets me install the essentials: neovim, git, tmux.

On a recent trip I observed, then diagnosed and fixed a bug in Termux. It was an interesting process, and I learned a lot about how the package manager works, so I’m sharing it here.

The bug

Neovim provides a Man command to open manpages in a neovim buffer. I use this feature all the time at home. But I noticed that it wasn’t working for me:

:Man mktemp
man.lua: "no manual entry for man"

I run into these kinds of rough edges occasionally when working in Termux and usually I ignore them. I would have ignored this too, but this time I knew I had just been looking at the same manpage in Termux.

The previous time, I opened it outside of neovim:

bash $ man mktemp
# man page opened

This is particularly interesting because I have this bit of customization for man in my .bashrc:

export MANPAGER='nvim +Man!'

From the man manpage:

MANPAGER, PAGER
  If $MANPAGER or $PAGER is set ($MANPAGER is used in preference), its value is
  used as the name of the program used to display the manual page.

I’m using neovim to open my manpages even when I run man from bash! So why does one approach work but not the other?

Debugging

We have one method of opening manpages that works: man command in bash

And we have one method that doesn’t work: :Man command in neovim.

The goal now is to figure out what the two approaches are doing differently. From the manpage excerpt above it seems like when we call man mktemp, the man program:

1. finds the location of the manpage file, then
2. delegates to an external program, specified by MANPAGER, to display the document

What is :Man doing differently? We know that neovim is able to display the manpage fine when it’s called by man, so the difference likely lies in how neovim finds the location of the manpage. How does neovim find manpage locations?

First, let’s figure out where :Man is defined. We’ve already seen the filename “man.lua” in the error message, so let’s find that file. Where is the file? I know it’s part of neovim (rather than an external plugin), so it’s probably under $VIMRUNTIME.

:e $VIMRUNTIME

I ran fzf from the directory I landed in and saw a couple files called man.lua:

runtime/lua/man.lua
runtime/plugin/man.lua

I’m not an expert on vim plugins, but I suspected the Man command definition itself would be found under plugin/.

I opened it, and sure enough:

vim.api.nvim_create_user_command('Man', function(params)
  local man = require('man')
  if params.bang then
    man.init_pager()
  else
    local ok, err = pcall(man.open_page, params.count, params.smods, params.fargs)
    if not ok then
      vim.notify(man.errormsg or err, vim.log.levels.ERROR)
    end
  end
end, {
  bang = true,
  bar = true,
  addr = 'other',
  nargs = '*',
  complete = function(...)
    return require('man').man_complete(...)
  end,
})

What do we see?

• nvim_create_user_command('Man', ...): this is the definition of the :Man command
• if params.bang: this probably refers to the exclamation point we see in the MANPAGER setting above.

If “bang” is set, we call man.init_pager. If it’s not set, we call man.open_page. Ok, we are getting a handle on the two separate codepaths.

Let’s see what open_page does. It’s defined in lua/man.lua, which I’ve saved a full copy of here (license.)

Partway down open_page we see our error from before ('no manual entry for ' .. name.) This error is displayed if find_path returns nil.

  local path = M.find_path(sect, name)
  if path == nil then
    path = M.find_path(sect, spaces_to_underscores(name))
    if path == nil then
      man_error('no manual entry for ' .. name) -- <--- our error!
    end
  end

Next we look in find_path, which tries several variations of calls to another function called get_path. Let’s look at get_path (reproduced here with fewer comments.)

local FIND_ARG = '-w'

local function get_path(sect, name, silent)
  name = name or ''
  sect = sect or ''

  local cmd ---@type string[]
  if sect == '' then
    cmd = { 'mandoc', FIND_ARG, name }
  else
    cmd = { 'mandoc', FIND_ARG, sect, name }
  end

  local lines = system(cmd, silent)
  local results = vim.split(lines or {}, '\n', { trimempty = true })

  if #results == 0 then
    return
  end

  -- `man -w /some/path` will return `/some/path` for any existent file, which
  -- stops us from actually determining if a path has a corresponding man file.
  -- Since `:Man /some/path/to/man/file` isn't supported anyway, we should just
  -- error out here if we detect this is the case.
  if sect == '' and #results == 1 and results[1] == name then
    return
  end

  -- find any that match the specified name
  ---@param v string
  local namematches = vim.tbl_filter(function(v)
    local tail = fn.fnamemodify(v, ':t')
    return string.find(tail, name, 1, true)
  end, results) or {}
  local sectmatches = {}

  if #namematches > 0 and sect ~= '' then
    ---@param v string
    sectmatches = vim.tbl_filter(function(v)
      return fn.fnamemodify(v, ':e') == sect
    end, namematches)
  end

  return fn.substitute(sectmatches[1] or namematches[1] or results[1], [[\n\+$]], '', '')
end

get_path uses a command called mandoc, which isn’t familiar to me, to find the path of the manual file.

We can add some print statements here to get some visibility

  local cmd ---@type string[]
  if sect == '' then
    cmd = { 'mandoc', FIND_ARG, name }
  else
    cmd = { 'mandoc', FIND_ARG, sect, name }
  end

+ vim.print(cmd)
  local lines = system(cmd, silent)
  local results = vim.split(lines or {}, '\n', { trimempty = true })
+ vim.print(cmd)

Restart neovim, run :Man mktemp, and see:

cmd:
{ "mandoc", "-w", "man" }
results:
{}
cmd:
{ "mandoc", "-w", "mktemp" }
results:
{}
cmd:
{ "mandoc", "-w", "mktemp" }
results:
{}

get_path is called three times: once looking for man ( maybe for setup purposes? ) and twice for mktemp. Results are empty for all three commands.

What happens if we run this command manually?

bash $ mandoc -w mktemp
mandoc: mktemp: BADARG: bad command line argument: No such file or directory

This is kind of a strange result. I know I can open the mktemp manpage, so why can’t mandoc find it? Let’s look at the mandoc manpage.

NAME
  mandoc - format manual pages

“Format manual pages.” Weird that the formatter for manual pages is also used to find manual pages. I searched for the -w flag and … didn’t find it. There is an uppercase -W flag that controls error verbosity, but no -w flag mentioned.

This is pretty strange. At this point I have a few hypotheses in mind:

1. The mandoc manpage is out of date; that’s why it doesn’t mention -w
2. There is a bug in mandoc that is possibly fixed in a later version than what we have in Termux.
3. There is a bug in neovim that is possibly fixed in a later version than what we have in Termux.

I decided to look into #3 first, because I’m more familiar with neovim than I am with mandoc.

First, I searched for my error message in the neovim issue tracker on github. I didn’t find anything relevant. Not a great sign, but I decided to check the source file for any recent changes.

It’s been a few weeks between doing this investigation and writing it up, but at the time I was looking at commit 4c05b1a6ab32880bcc556cf4bf67f1f3edf0c480. Here’s the log from that time for man.lua:

 $ git log man.lua
commit 209ed16f57a73518296c3dce0a1ef3975181318d
Author: Vadim A. Misbakh-Soloviov <msva@users.noreply.github.com>
Date:   Fri May 5 11:15:39 2023 +0700

    fix(man.lua): return support of all sections

    Current behaviour of `:Man` is to only work with "number" sections.
    This is caused by wrong assumptions about man sections naming.

    Also, there was similar assumption about length of section dirs
    in `paths` variable.

    fixes #23485

    Signed-off-by: Vadim Misbakh-Soloviov <git@mva.name>

commit c8d1d8b2546c5974f4ce75fc87f918bc8cfe8e56
Author: zeertzjq <zeertzjq@outlook.com>
Date:   Tue Apr 11 09:35:01 2023 +0800

    fix(man.lua): don't continue on command error (#23009)

    Fix #21169

commit 304477ff3504373a336c83127654e65eddfa2ef9
Author: Justin M. Keyes <justinkz@gmail.com>
Date:   Tue Mar 7 15:13:09 2023 +0100

    fix(man.lua): tests, naming

commit 160a019ffa104eebd65f4037729954d98aca6ad0
Author: Eriks Muhins <chad@eriksmuhins.com>
Date:   Fri Mar 3 21:44:13 2023 +0200

    feat(man.lua): support spaces in manpage names

    Problem:
    :Man command errors if given more than two arguments. Thus, it is
    impossible to open man pages that contain spaces in their names.

    Solution:
    Adjust :Man so that it tries variants with spaces and underscores, and
    uses the first found.

commit 2d99830706a8560ac6f4668b4a384afc776cc7f4
Author: Lewis Russell <lewis6991@gmail.com>
Date:   Tue Feb 21 12:19:09 2023 +0000

    refactor(man): add type annotations

commit 9ce44a750c2a65082962effe6ce4d185b7698d73
Author: Lewis Russell <lewis6991@gmail.com>
Date:   Wed Feb 1 17:21:42 2023 +0000

    fix(man): use italics for `<bs>_` (#22086)

    fix(man): use italics for <bs>_

    Even though underline is strictly what this should be. <bs>_ was used by
    nroff to indicate italics which wasn't possible on old typewriters so
    underline was used. Modern terminals now support italics so lets use
    that now.

There are a few commits that could be related. Let’s see what get_path looks like at HEAD.

local function get_path(sect, name, silent)
  name = name or ''
  sect = sect or ''

  local cmd ---@type string[]
  if sect == '' then
    cmd = { 'man', FIND_ARG, name }
  else
    cmd = { 'man', FIND_ARG, sect, name }
  end

  local lines = system(cmd, silent)
  local results = vim.split(lines or {}, '\n', { trimempty = true })

  if #results == 0 then
    return
  end

  -- `man -w /some/path` will return `/some/path` for any existent file, which
  -- stops us from actually determining if a path has a corresponding man file.
  -- Since `:Man /some/path/to/man/file` isn't supported anyway, we should just
  -- error out here if we detect this is the case.
  if sect == '' and #results == 1 and results[1] == name then
    return
  end

  -- find any that match the specified name
  ---@param v string
  local namematches = vim.tbl_filter(function(v)
    local tail = fn.fnamemodify(v, ':t')
    return string.find(tail, name, 1, true)
  end, results) or {}
  local sectmatches = {}

  if #namematches > 0 and sect ~= '' then
    ---@param v string
    sectmatches = vim.tbl_filter(function(v)
      return fn.fnamemodify(v, ':e') == sect
    end, namematches)
  end

  return fn.substitute(sectmatches[1] or namematches[1] or results[1], [[\n\+$]], '', '')
end

There is a glaring difference here between what I see in the repo and what I see in my installation. Do you see it?

  -- neovim repo:
  if sect == '' then
    cmd = { 'man', FIND_ARG, name }
  else
    cmd = { 'man', FIND_ARG, sect, name }
  end

  -- termux installation:
  if sect == '' then
    cmd = { 'mandoc', FIND_ARG, name }
  else
    cmd = { 'mandoc', FIND_ARG, sect, name }
  end

man instead of mandoc. What happens if I try running that command instead?

bash $ man -w mktemp
/data/data/com.termux/files/usr/share/man/man1/mktemp.1.gz

So man -w prints the path for me while mandoc -w prints an error. I edited man.lua locally to use man instead of mandoc, restarted neovim, ran :Man mktemp, and – it works!

This is great - I have a fix that seems to work for me. It’s a little awkward to have to change installation files locally, though. And I’m lucky that this change was in an interpreted lua file rather than a compiled C program.

Fixing the bug

Maybe this bug was fixed in a recent version of neovim and I just need to wait a couple weeks for it to percolate into termux? Was the code changed recently?

bash $ git blame man.lua
[output trimmed]
2afcdbd63a (Lewis Russell             2022-09-02 15:20:29 +0100 308)   if sect == '' then
b126b11840 (Lewis Russell             2022-10-11 13:25:51 +0100 309)     cmd = { 'man', FIND_ARG, name }
2afcdbd63a (Lewis Russell             2022-09-02 15:20:29 +0100 310)   else
b126b11840 (Lewis Russell             2022-10-11 13:25:51 +0100 311)     cmd = { 'man', FIND_ARG, sect, name }
2afcdbd63a (Lewis Russell             2022-09-02 15:20:29 +0100 312)   end

Not super recently, but maybe these commits are relevant. But when I drilled down into them, I found none were related to mandoc.

In fact, no commit to this file ever contained the word mandoc:

bash $ git log -Smandoc man.lua
[empty]

Where did mandoc come from? If it’s not in the neovim source, I guess it must be added by the termux maintainers. I didn’t know how package management works for termux, but after clicking around on the homepage I found https://github.com/termux/termux-packages.

In that repository, under packages/neovim, I found the culprit:

--- a/runtime/lua/man.lua
+++ b/runtime/lua/man.lua
@@ -271,9 +271,9 @@
   -- inconsistently supported. Instead, call -w with a section and a name.
   local cmd
   if sect == '' then
-    cmd = { 'man', FIND_ARG, name }
+    cmd = { 'mandoc', FIND_ARG, name }
   else
-    cmd = { 'man', FIND_ARG, sect, name }
+    cmd = { 'mandoc', FIND_ARG, sect, name }
   end

   local lines = system(cmd, silent)

This is a patch file that is applied to the neovim codebase before building the neovim package for Termux. It replaces man with mandoc.

This change was made in commit ebeb10fa03691ed000e3259edc83e7f9f229116f with no explanation.

At this point I felt as though I had done my due diligence and filed a bug against termux-packages https://github.com/termux/termux-packages/issues/16720. I explained the bug and my diagnosis: this patch was probably added in error.

After a brief conversation, I let the issue sit for a couple weeks. There was no movement on it in that time, so I asked if I could send a PR to fix the bug. One of the maintainers gave permission, so I removed the patch (and the corresponding patch from packages/neovim-nightly) in https://github.com/termux/termux-packages/pull/16821.

My PR was merged one week ago. Today, I ran pkg ugprade on Termux to see if the fix has made it into production.

:Man mktemp
man.lua: "no manual entry for mktemp"

Well, it’s not there yet. At least I don’t have a trip planned for a little while :)

Conclusion

This was a pretty satisfying debugging progress. In the span of an hour or so sitting in a public library in Orlando, I identified a bug and tracked it down through a series of twists and turns. I learned a bit more about how neovim works, and I learned a lot about package management works – at least for termux.

I have a few lingering thoughts after the whole experience.

Length of outage

This bug was introduced on September 23, 2022. I observed it eight months later on May 19, 2023 and merged a fix on June 2. As of June 8, the bug is still in production. I haven’t found any other bugs mentioning this issue, although there was another PR introduced (but not merged) by the original patch author on May 20 to fix the issue.

Am I the only person that ran into this issue in all that time? I guess there just aren’t a lot of Android users that read their manpages in neovim.

The supply chain

It’s a bit scary that the original patch, which visibly breaks the only functionality it could possibly affect, made it into my system at all. If a patch like this was added in error, with no explanation, how easy would it be for a malicious actor to include something worse? How closely should I be looking at the termux-patched builds that make up my system?

When are patches necessary?

Specifically for neovim, and more specifically for lua plugins, I’m surprised to see Termux including patches at all. The neovim maintainers ought to be using cross-platform APIs; if they aren’t, is that a Termux bug or a neovim bug?

For now, the other patches to neovim seem pretty reasonable: they are in C code and make changes relevant to termux’s non-standard filesystem hierarchy.