Read functions and completions directly from the fish binary

[faho]

Description

This builds on the previous "installable" builds that embedded the assets (functions, completions, man pages etc)
and wrote them out on start or whenever you ran fish --install, and turns them into "embed-data" builds.

These will just read the assets directly from the binary when needed.

When autoloading, these are considered last (except for completions, they're considered before the generated completions), so overriding them via $fish_function_path or $fish_complete_path works like before.

That means, for users, this creates a fully capable single-file fish that does not depend on any other files on the filesystem. Link it statically and you can copy it wherever.

Internals

Some scripts use the new status list-files and status get-file commands to get the files.
The fish --install option is removed - if you need to extract the files you can easily script that with the status commands.

• Webconfig is written to a temporary directory where it can be opened by python
• The manpage completion generator is extracted to a temporary directory when you run fish_update_completions.
  This is not done automatically at first start to avoid creating directories. Tbh the manpage completions aren't all that useful anymore.
• funced knows to load these files
• our man wrapper and builtin --help write out the man pages when needed

As before, the way to do this is to build with the on-by-default "embed-data" feature (which is disabled by cmake, so nothing changes for packagers).

Why?

The idea is that you can copy a single fish binary to a system and run it without having to touch that system's files or care about keeping them up-to-date.

This also makes fish more resilient to misinstallation - there is no more data directory to keep in-sync, uninstallation is just removing the binary. Completions in other packages can no longer conflict with our completions.

It is no longer possible to break a lot of fish by setting $fish_function_path wrong.

Compatibility concerns

$__fish_data_dir is set to empty. This is to avoid embed-data builds reading outdated assets from old installable builds and because it is no longer a useful directory.

That means scripts that go through that directory, e.g. to load another file, are affected will need to be changed to use the status commands.
I've done that for our things (and hopefully got everything), but third-party plugins would also have to be adjusted.

We could also keep $__fish_data_dir set to the old directory,
but that would not help - if anything does source $__fish_data_dir/completions/git.fish that breaks if there is no git.fish there,
and not having a git.fish there is the point. So there is necessary breakage, and at that point it's easier to make both versions work if the directory is empty.

This concern is why I'm not pushing for this to become the default right now - it's still "experimental" so peope have time to adjust their plugins.

An additional point is that $__fish_data_dir also contains the vendor directories. If we did make this the default also for packaged builds, we would have to do something about that. I'm not entirely sure what, one possibility is to always check /usr/share/fish and /usr/local/share/fish, but that can obviously run into problems if the versions diverge (new fish on an LTS system).

Installable builds never read the vendor directories so nothing changes in that respect.

Future

This could be the default way to build fish, and it could also be used to remove cmake.

The way to build fish for a package could be

# Configure this one with environment variables like $PREFIX, where necessary.
cargo build --release
# html docs can only be built after fish_indent is built
# we could make this easier by making it find fish automatically and using the indent builtin
FISH_BUILD_VERSION=$(target/release/fish --version) PATH="$PWD/target/release:$PATH" sphinx-build doc_src -M html
cargo test
python3 tests/test_driver.py target/release/fish -f /tmp

# create vendor directories, __fish_build_paths.fish, maybe the man pages separately
# copy the files wherever needed to make a package

As before, embed-data builds do not contain html documentation (because we need fish_indent to build them) or translations (because gettext needs them in a file).

help will open the online docs and translations aren't very useful at the moment.

TODO

• Check docs for fish_data_dir
• Another pass over functions/completions
• Add a feature flag so plugins can do if status test-feature embed-data instead of trying status list-files?
• Or make the status commands transparently open files when needed?
• Rename the status commands to be more explicit about which files they touch? "status list-fish-asset-files"? "status list-embedded-files"?
• CHANGELOG

Proposed CHANGELOG wording:

The "self-installable" builds have been changed to read their datafiles directly from the fish binary instead of having to write them out on start. The --install option was removed, instead there are more granular status list-files and status get-file FILE commands that can be used to get at these files.

That means it is now possible (combined with the changes to make fish_key_reader and fish_indent builtins) to build fish as a single file that you can copy to a compatible system and just run it, including as a different user. As before, this is the default when building fish via cargo (e.g. cargo install).

We're considering making this the default in future because it makes it easier to install fish.

Note to developers of fish integration or plugins: These builds will have $__fish_data_dir set to empty because it would not contain any files. If you want to get a file, consider falling back to status get-file path/to/file (where "path/to/file" would be the path relative to the datadir, for instance "completions/git.fish") or if you want to load other completions just running them with complete -C"foo" >/dev/null.

the old description

## Description

This is a rough draft. In particular, for some reason I have not figured out, cargo test gets stuck at completing echo (ls .

If anyone sees why that is (it works in actual usage and the scripted tests!), any help would be appreciated. (edit: figured it out, it's a bug that's uncovered by this and now worked around)

Anyway, what does it do?

The worst part of self-installable fish currently is that it needs to write out its functions and completions before being in any sensible meaning of the word "functional". Without installing you've got a paper (well, bit-) weight.

So this will instead read these files right from the binary, if necessary.

So if you try to autoload e.g. the fish_prompt function, it will go through the regular $fish_function_path lookup, and if it doesn't find anything, load the version of fish_prompt.fish that was embedded in the binary. Same for completions.

Note that this is not entirely the same thing as currently, when using $__fish_data_dir - it is always last, and cannot be skipped. Not being skippable is probably fine, we might want to add a special "embedded" entry for this that you can add wherever you want (or just interpret the data dir as that). There's a hack to skip the generated completions.

Installing now only writes out the docs and webconfig (with the themes!). It skips functions and completions entirely.

TODOs:

• This isn't gated on the "installable" feature for now
• Some things still need to be installed - webconfig, the man pages etc. This no longer prompts on start to install them. Is that okay?
• builtin --help could be made to work by reading the docs from the binary and piping them to groff
• What about $__fish_data_dir? It's no longer really a thing for $fish_function_path
• We could make embedding the functions and completions the default configuration even via cmake - no real need to have files on disk we need to keep in sync.
• Aforementioned hang
• Cleanup
• Tests

[faho]

• Completing functions doesn't work completely because it'll just check $fish_function_path
• Highlighting functions that aren't already loaded fails
• the build won't be rerun if only a function changed

[faho]

I think the hang might be pre-existing.

It hangs doing

completions = do_complete(
    L!("echo (ls test/complete_test/testfil"),
    CompletionRequestOptions::default(),
);

And specifically and only that one. Now, ls has a completion script, which is loaded, and that calls ls, which is also a function, which is also loaded.

Only... without embedding the functions, these aren't ever loaded because $fish_complete_path and $fish_function_path aren't actually defined here, so it will use the default file completions. That's also why, when you comment out that specific one, some other test_complete tests will fail.

When the functions are embedded, it hangs in running an external command - the first would be uname, but comment that out and it hangs running command ls (inside the ls function). It leaves defunct processes, so it seems to not be reaping them correctly from inside the cargo test environment.

[zanchey]

We did try this in about 2013, IIRC, though the motivation was performance rather than portability.

[faho]

Yup, it's #15 - which was disabled in fishfish beta 2 (which later became fish 2.0).

The big downside there was that "internalized" functions could not be overridden.

This solution doesn't have that problem - it does the normal lookup, and only if that fails it'll load the embedded scripts. That means it always runs as if $fish_function_path/$fish_complete_path had the embedded scripts as the last entry (with the one exception of generated_completions, which I'll grant is a hack).

I don't believe performance will really be measurably better - we still look at all the paths (including the empty $__fish_data_dir path for now), and we still need to convert the embedded scripts to wstring.

The benefit I'm excited by really is that it makes fish easier to install and more resilient - there's no more "what if someone set $fish_function_path wrong or erased /usr/share/fish", there's no more issue with make install leaving outdated scripts.

Which would be cool even if you install fish via a package manager, which is why I think this could be a default.

[zanchey]

Yeah, that makes senses, and it's a great idea

[faho]

Alright, we have a few places where we kinda wanna get the file from a function. Both __fish_print_help and fish_delta could benefit from this. I'm not sure how to do the latter without, and the former would be tricky.

So I am tempted to add a status get-file command that will print these files - status get-file functions/fish_prompt.fish would print the default fish_prompt.fish.

This would be weird if it were possible to not have embedded files at all, because either it would print files from the filesystem (in which case we've built a cat), or it wouldn't be available in some configurations.

[faho]

Okay, I now made it so it asks to update the extracted assets if they exist. That means we allow an install without (e.g. just copying fish onto a machine that you ssh to), but if they are used we prevent them from going stale.

I don't know if that still requires confirmation, we could just write them out when needed.

[faho]

I added two special commands:

• status get-file, to get a single embedded file's contents
• status list-files, to list embedded files (starting with a string - this could also do wildcard matching)

These allow us to make a lot of things work without extracting:

• fish_config (the CLI version) (prompts TODO)
• __fish_print_help (and thereby builtin --help)
• fish_delta
• funced - if we didn't have status get-file it couldn't correctly edit e.g. fish_git_prompt (which has multiple functions)

That means the only remaining features that need files to be extracted are:

• webconfig
• the man page completion generator

That's because these have multiple files that need to be passed to external tools. webconfig has a bunch of js, the completion generator consists of two python files.

We could potentially extract webconfig on-demand, make the man generator a single file and remove fish --install entirely. In that case webconfig would need to be taught to call fish to print embedded files.

status isn't really the ideal place to put these subcommands but I don't know a better one.

Remaining TODOs:

• fish_config prompt
• docs for status get-file/list-files

[faho]

Alright, made webconfig extract on demand.

[faho]

I think the only good thing to do for the manpage completion generator is to make it a single file. Otherwise we would have to extract it on startup, which is awkward.

Also: This PR gets us further away from being able to use gettext. It's currently not functional for installable builds - gettext needs the files in a very specific directory structure. And it needs them as files. Since our translations are currently barely functional at best (for french and german), that's fine.

But if we wanted to have translations, this is another reason to switch away.

I also noticed that this fixes the issue where packagers place completion scripts in /usr/share/fish/completions. If we no longer install anything there, there's no conflict.

[faho]

Alright, I've restructured this a bit.

It is now optional again, using the same "installable" feature. We could rename that to "embedded", because that's now what it is.

The "--install" option is gone because it is no longer necessary (if you really must, you can use status list-files/status get-file to extract these files).

The manpage generator won't be run automatically on startup, but if you run fish_update_completions it will be extracted to a temporary directory.
I decided on this because this is nicer when you move a fish binary to another system just to run it there. It feels weird to clutter any system you run with a bunch of completion scripts that you probably won't use, and run a python script to do so.

To reiterate, the remaining limitations of "embedded" builds:

• html docs aren't included
• translations won't work until we move off gettext

I've decided to keep $__fish_data_dir set for now, because I'm not sure how many things will break if it's empty.
It is still there, and if you put something there it will be used.

I would like more testing before we make this the default configuration.

Other than that, it's pretty much ready now, so I'm marking this as no longer a draft.

[faho]

I've updated the description to explain what this does now and what I would want to do with it.

I believe this is now in a mergeable state and want to do so. If anyone wants to review, now's the time.

[krobelus]

My cargo install --path . --debug && ~/.cargo/bin/fish tries to source files from /usr/share/fish/functions
This is because I have this hack in my config:

set -a fish_complete_path /usr/share/fish/vendor_completions.d /usr/share/fish/completions
set -a fish_function_path /usr/share/fish/vendor_functions.d/ /usr/share/fish/functions

of course the added "/usr/share/fish/functions" is wrong, I'm not sure why I added it

I think the vendor_completions.d used to be necessary for running installations to ~/.local,
but they should not be necessary if I set XDG_DATA_DIRS.. probably there's a reason I still have it

[faho]

The installable builds never read the vendor stuff, yes.

I'm not sure if it is a good idea to do that or not, because these builds could be much newer than what the vendor stuff is prepared for.

For now, I'm explicitly punting on that for later - if we end up making this style of build the default, we'll have to handle it, but until then it's the status quo.

[krobelus]

This concern is why I'm not pushing for this to become the default right now - it's still
"experimental" so people have time to adjust their plugins.

Yeah it would be nice if we can directly notify the most important plugins
(or better, give them fixes, even if they're untested).
I haven't checked how many are affected.

Installable builds never read the vendor directories so nothing changes in that respect.

It seems ~/.local/share/fish/vendor_completions.d is still in fish_complete_path when I run an embed-data build with:

env -u XDG_DATA_DIRS HOME=$(mktemp -d) fish

so I assume it's not completely ignored?

[faho]

That's $XDG_DATA_HOME.

[faho]

Ah, no, it's $__fish_user_data_dir.

We really added too much guff here.

[faho]

Okay, I don't know what to do with that one. I missed it being introduced.

We can either set it to empty too, or keep it (it's in your home dir, so it's more likely that it's up-to-date and usable).