Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow building fish as self-installable #10367

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

faho
Copy link
Member

@faho faho commented Mar 12, 2024

Description

Here's the teaser:

cargo install --git https://github.com/faho/fish-shell --branch fish-installer
~/.cargo/bin/fish --install
~/.cargo/bin/fish

This is how you can get a running fish without root permissions.

You can also use it to build a 100% statically linked fish that you can then copy to another system (with the same architecture and operating system). I've been able to do that for x86_64 linux, and cross-compile one for aarch64 linux. (using musl because glibc doesn't do static linking)

I believe it would be possible to allow this as an option, and to add a static build to fishshell.com so people can just download a fish, without compilation.

I believe this is superior to appimages, which I have recently discovered require libfuse2, which I didn't have installed, along with other dependencies like a compatible libc (so you would have to build on an old glibc system, and additionally a musl one, and that for each architecture)

Supersedes #6475.


What this does is embed the entirety of share/ - all the functions, completions and fish_config, and then write it out when you run fish --install (or fish --install=noconfirm to skip the confirmation).

It writes them out to ~/.local/share/fish/install, so uninstallation is cargo uninstall fish; rm -r ~/.local/share/fish/install.

Technically, this is done by adding a Cargo feature, "installable", that's enabled by default (so cargo install will do it by default). When it is enabled, rust_embed will be added as a dependency. Default features are disabled in the cmake build so nothing changes there. (a slight wrinkle is that, if we did add another feature, we would have to turn it on in cmake)

This about doubles the fish binary size (6M to 12M unstripped, 4M to 8.7M stripped). I have run benchmarks where it had little impact (7% increase in --no-config startup time). I do not think this is important. Binaries of similar size on my system: ctest, emacs, agg, gdb, perf. Bigger binaries include: kitten, clang-tidy, qemu, clangd. I have never thought of the size of any of them.

TODO:

  • Documentation
  • Tests
  • The version currently only includes the commit hash (e.g. "4ec25d894"). I believe that's because cargo only does a shallow clone. This would have to find another source, e.g. the crate version, but even then it would look different because the number of commits is lost. Would something like "3.7.1-g4ec25d894" be an okay version string?

The following are more "future directions" than things I feel must be included now for this to be useful:

  • Stop including __fish_build_paths.fish.in (this is harmless)
  • Include man pages - these aren't currently included. It might be possible to get them in by generating them in build.rs.
    HTML isn't possible (it needs fish_indent to do the syntax highlighting), but help is always available and will simply open the online version.
  • Include translations
  • Make fish_indent/fish_key_reader available as builtins (and potentially by calling fish as them) so you really have a single-file fish.
  • The data path is currently pretty set as ~/.local/share/fish/install. It would be nice to make this "relocatable" in some manner, so people can download a build and place it in e.g. /usr/local/bin/ or /opt/. (unfortunately cargo installs to ~/.cargo/bin/ by default but ~/.cargo/share/ isn't really a thing, so a "../share" feels off to me?)

@faho faho added this to the fish next-3.x milestone Mar 12, 2024
@faho
Copy link
Member Author

faho commented Mar 13, 2024

One annoyance of this solution: You have to rerun fish --install on upgrade. I experimented with having it check the mtime of share/config.fish and printing a note if it was older than the fish binary. This generally works, but will trigger too often (e.g. if you move the fish binary after installing).

It might be possible to write a file with the version that was installed and check that on startup instead.

@matklad
Copy link

matklad commented Mar 20, 2024

I believe it would be possible to allow this as an option, and to add a static build to fishshell.com so people can just download a fish, without compilation.

Would be prudent to double-check performance of the statically linked version. Four-ish years ago, statically linking with musl could have tanked your performance due to musl's malloc being much slower than glibc one or jemalloc. This issue prevented rust-analyzer from going the fully statically linked way. But I remeber that musl wanted to rewrite the allocator, and, presumably, they had done that since.

@faho
Copy link
Member Author

faho commented Mar 22, 2024

Would be prudent to double-check performance of the statically linked version

Pure "--no-config" startup is faster, our other benchmarks are 10-70% slower (memory usage is ~25% less). Of course those are specifically written to mostly measure time spent in fish. Realistically, this is an entirely usable build and it's hard to notice a difference when just running it. Performance in shells is weird, very often most of the time is spent waiting for external commands.

It's "possible" to link glibc statically, but that will crash when you attempt to do echo ~foo (in libnss_something, via getpwnam_r). (this is why we used to forbid static linking in cmake)

The alternative is to offer a self-installable build dynamically linked against glibc, but that obviously requires a compatible (i.e. same version or newer) glibc to what it was built against. This would still be better than the appimage, because that needs libfuse2 and glibc on the host. These are also simple enough to build that we could do both.

This talk is of course secondary to whether having fish self-installable is useful - it would enable cargo install to do something usable, and it could then also be used to distribute cross-distro single-file binaries, whether they are statically or dynamically linked.

@dingobits
Copy link

The alternative is to offer a self-installable build dynamically linked against glibc, but that obviously requires a compatible (i.e. same version or newer) glibc to what it was built against. This would still be better than the appimage, because that needs libfuse2 and glibc on the host. These are also simple enough to build that we could do both.

Binaries built with newer glibc isn't necessarily incompatible with older glibc. glibc uses symbol versioning to indicate compatibility. I pulled your branch and ran cargo build --release with glibc 2.37, which was released in 2023. readelf -V target/release/fish shows binary requires GLIBC_2.34, which was released in 2021. I don't know how many years of backwards compatibility is most appropriate, but I think building from Debian oldstable or the Ubuntu equivalent should suffice for most people.

@faho faho mentioned this pull request May 2, 2024
@Justinzobel
Copy link

So if I had a fleet of 100 servers on varying versions of Ubuntu LTS I could build it on the oldest server and it should be able to be copied to any of the same version server or higher and it would simply run?

@faho
Copy link
Member Author

faho commented May 2, 2024

I could build it on the oldest server and it should be able to be copied to any of the same version server or higher and it would simply run

If you build dynamically against glibc, yes. If you build statically against musl you can build it anywhere (probably even cross-compile it from macos), at the cost of some performance.

This will give you a per-user fish that needs to install its datafiles by running fish --install, after that fish will be fully capable.

@faho
Copy link
Member Author

faho commented May 3, 2024

Alright, after a rebase this now crashes on startup - do cargo install --path ., run ~/.cargo/bin/fish, and you'll get a segfault.

Backtrace is:

#0  0x0000555555801744 in fish::fork_exec::blocked_signals_for_job ()
#1  0x00005555557ef2ee in fish::fork_exec::spawn::PosixSpawner::new ()
#2  0x00005555557ea0fc in fish::exec::exec_process_in_job ()
#3  0x00005555557e4bdf in fish::exec::exec_job ()
#4  0x00005555556804ea in fish::parse_execution::ParseExecutionContext::run_1_job ()
#5  0x000055555567f378 in fish::parse_execution::ParseExecutionContext::run_job_conjunction ()
#6  0x000055555567f20a in fish::parse_execution::ParseExecutionContext::run_job_list ()
#7  0x00005555556776a4 in fish::parse_execution::ParseExecutionContext::eval_node ()
#8  0x000055555576fd4c in fish::parser::Parser::eval_node ()
#9  0x000055555576d7fe in fish::parser::Parser::eval_with ()
#10 0x00005555557f6ba9 in fish::exec::exec_subshell_internal ()
#11 0x000055555572d2f3 in fish::expand::expand_cmdsubst ()

@krobelus
Copy link
Member

krobelus commented May 3, 2024

debug build might help. This is still pre-fork so that should rule out a class of problems

@faho
Copy link
Member Author

faho commented May 3, 2024

debug build might help.

Debug build gives an empty backtrace (only ??, no symbols).

Edit: Okay, I'm reasonably sure it's some dependency locking issue. If I run cargo build and run the resulting fish, it runs. But if I run cargo install, the resulting fish crashes and every cargo build after that also crashes until I cargo clean.

faho added 6 commits May 7, 2024 17:56
This shells out to __fish_describe_command, but if the install is
incomplete that will trigger the command-not-found handler.
This applies to `cargo install`, cmake should override it by setting $PREFIX.

This together with the previous change makes `cargo install` sort-of
work-ish - you'll have to copy the assets manually, and you'll be
responsible for upgrading them together with fish.
When built with the default "installable" feature:

Run `fish --install` or `fish --install=noconfirm` (for
non-interactive use)

CMake disables the default features so nothing changes for that, but this allows installing via `cargo install`,
and even making a static binary that you can then just upload and have extract itself.
This used to create ~/.local/share/etc/fish, which makes no sense
Otherwise you have all the gunk next to the history etc.

So we isolate it in ~/.local/share/fish/install/.
@faho
Copy link
Member Author

faho commented May 7, 2024

Okay I'm reasonably sure cargo install pulled in a newer libc. Pinning it to 0.2.151 (the last version compatible with rust < 1.71) seems to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants