Force use of macOS's builtin manpath

[zgracem]

Description

This PR modifies __fish_apropos to call macOS's own /usr/bin/manpath directly.

Fixes an issue (which I didn't bother reporting separately) where, if the user has installed and configured the man-db package from Homebrew, /usr/local/bin/manpath will complain every time __fish_apropos runs:

manpath: warning: $MANPATH set, ignoring /usr/local/etc/man_db.conf

The >/dev/null 2>&1 later on line 43 doesn't suppress the message because of the subshell, I guess? Anyway, this prevents the needless warning and ensures more predictable behaviour.

TODOs:

• Changes to fish usage are reflected in user documentation/manpages.
• Tests have been added for regressions fixed
• User-visible changes noted in CHANGELOG.rst

[mqudsi]

Thanks for the fix (and the description of what is going on). Have you fully considered whether or not the second-order ramifications of this change are necessarily intended? Usually not using a fully specified path to a binary lets a user shim/replace the executable (and the behavior) with a replacement installed to a $PATH entry of higher precedence.

To be clearer, the options I see would be

a) what you did, putting in more effort to try and use the explicit system-provided binary,
b) silencing the warning by adding the null redirection in the "subshell",
c) detecting that man-db is installed and skipping all the huge macOS workarounds we have to do to please the system-provided binaries in the first place (cf: we use mandb on Ubuntu just fine and don't need the workarounds, so why not do that on macOS if we can?) This should provide better/more comprehensive results in the corner cases that our MANPATH workaround doesn't (which is really why mandb is complaining).

[zgracem]

I guess I should have been less terse and explained more of my thinking. As you might have guessed from the fact that I noticed this problem at all, I am one of those users who replaces executables in my PATH, and I understand the general pitfalls, edge cases, etc. I'll try to show more of my work.

Your option (b) was the first thing I considered, but because I don't know every error manpath might produce, I didn't want to mute it unconditionally for fear of discarding any actually-useful messages.

I didn't consider option (c) because the need for this particular workaround seems to be macOS switching to a read-only whatis database, and none of the executables in Homebrew's man-db package can help with that.

And I don't think this is a case of the MANPATH workaround failing: as far as I can tell, that workaround only applies to the apropos call on line 39. The call to (either version of) manpath on line 43 uses whatever value of MANPATH is available in the environment, if any, before falling back to its default configuration files.

Given no options or arguments, macOS's /usr/bin/manpath and Homebrew's /usr/local/bin/manpath behave identically when MANPATH is set, except for the error message about redundant configuration. (The manual page for the macOS version says it should complain under the same circumstances, but it does not.)

The larger consideration is what happens when MANPATH is not set: no error message, but /usr/bin/manpath reads from /etc/man.conf, and Homebrew's from /usr/local/etc/man_db.conf, so they will produce different results:

diff >/dev/null (/usr/local/bin/manpath 2>/dev/null | psub) (/usr/bin/manpath | psub); and echo same
#=> same
set --erase --global MANPATH
diff >/dev/null (/usr/local/bin/manpath 2>/dev/null | psub) (/usr/bin/manpath | psub); or echo nope
#=> nope

I also considered your implicit question, "If the user prefers a third-party binary, shouldn't we let them use it?" My conclusion ("No, we shouldn't") was based on two assumptions. First, allowing arbitrary user configuration to influence the behaviour of an undocumented internal fish function which does most of its work in a detached background process and only runs once a week sounds, to me anyway, like a potential nightmare to support. It was certainly a pain for me to diagnose.

Second, speaking as one of those PATH-manipulating power users, if a commandline utility is interfacing with OS-specific stuff outside of my direct view, I actually prefer it not to use my custom third-party tools, which I typically install because they're platform-agnostic—most of the time I reserve them for interactive use, so I can keep an eye out for unexpected behaviour.

Anyway! I hope that makes my line of thinking a little clearer. Feel free to discard this PR if you don't agree. I'd prefer not to maintain my own copy of __fish_apropos just to silence one error message, but I chose the life of an edge-caser and sometimes that comes with the territory, hahaha.

[emilazy]

Another option here is to pass -q to manpath, which silences warnings but hopefully not fatal errors (if there even are any to silence). As another one of those pesky binary-overriders I think I would nominally prefer using the tool from PATH (mine is overriden by nixpkgs' man-db), but in this case it's being fed into /usr/libexec/makewhatis; the fixed use of the host's man implementation is already there.

(As for special-casing man-db to skip additional logic... maybe? /usr/bin/apropos ls is really slow but apropos ls isn't, but they do give different results. And my $XDG_CACHE_HOME/fish/whatis is empty even though /usr/libexec/makewhatis -o whatis (manpath | string split :) produces a nonempty file, so I don't really know what's going on there. Perhaps the logic could be gated on apropos resolving to the system version and the manpath and apropos paths made absolute in that case, but I'm not confident about the implications.)

[zgracem]

I can't believe I didn't notice -q was portable. 🤦🏼‍♀️ That might be the lowest-impact solution here, at least to my "manpath please stfu" issue. But this—

in this case it's being fed into /usr/libexec/makewhatis; the fixed use of the host's man implementation is already there

—is a good point in favour of the absolute-path approach.

[mqudsi]

Ok, I can accept this line of reasoning.

Merged w/ thanks.