mise icon indicating copy to clipboard operation
mise copied to clipboard

shims

Open jdx opened this issue 1 year ago • 13 comments

Ok to start: I don't want to implement this. I'm creating this ticket as a place to discuss it since there are some areas where we may want to do it. It's come up a few times as at least a potential solution to various problems.

We should be able to build a shim farm in ~/.local/share/rtx/shims just like asdf does. This could be useful if for some reason rtx activate doesn't work correctly. One potential example of this is in IDEs which would be easier to just add that directory to PATH instead of having an extension to integrate with rtx.

This behavior would almost certainly not be the default behavior. Shims would be an alternative way to use rtx.

There are 4 downsides I see with using shims:

  • Breaking which
  • Needing to reshim when installing new bins
  • (almost negligible) performance hit of a few ms
  • More code that we need in rtx

We'd have to figure out how this would work from the user's perspective. Do we have a setting that just makes rtx behave with a shim model globally? (so rtx activate would simply add the shims to PATH)

Or do we do something simpler and just implement rtx reshim which rebuilds the shims when they're changed, leaving it up to the user to add the shims directory to PATH and keep them up to date?

What do we do when installing new bins with tools like pip and npm? Does asdf do anything here or does the user have to manually run asdf reshim? EDIT: the plugins call reshim.

As far as the shims themselves, they should be trivial to write (and these should work today, there just isn't a way to generate them):

$ mkdir -p ~/.local/share/rtx/shims
$ cat >~/.local/share/rtx/shims/node <<EOF
#!/bin/sh
rtx x -- "$(basename $0)" "$@"
EOF
$ chmod +x ~/.local/share/rtx/shims/node
$ ~/.local/share/rtx/shims/node -v
v18.0.0

Hopefully this is enough, if so we wouldn't even need for the shims to be different. They can all have exactly the same contents.

jdx avatar Feb 07 '23 15:02 jdx

A minor note regarding the actual shim: it would probably have to look something like this:

#!/bin/sh
RTX_MISSING_RUNTIME_BEHAVIOR=ignore rtx exec -- "$(basename $0)" "$@"

Or something similar, since otherwise prompts from rtx about e.g. missing versions if using fuzzy matching in the .tool-versions file will break running binaries inside other tools like IDEs.

eproxus avatar Feb 23 '23 10:02 eproxus

What do we do when installing new bins with tools like pip and npm? Does asdf do anything here or does the user have to manually run asdf reshim?

I think they do some magic to add new shims for all installed binaries from packages, at least pip does.

eproxus avatar Feb 23 '23 10:02 eproxus

it won't prompt if it's not connected to a tty so that shouldn't be a problem

jdx avatar Feb 23 '23 13:02 jdx

I'd really like to see shims, as trying to integrate rtx with Emacs looks like it would be pretty daunting right now.

I've not used asdf myself, but I have used rbenv, and it comes with a RubyGems plugin that automatically triggers a call to rbenv rehash after any gems are installed. I'm fairly sure pyenv does the same for pip.

As for the shims themselves, correct me if I'm wrong, but couldn't the shims be even simpler/faster if they were simply symlinks to the rtx binary? That is assuming it's easy to make rtx behave as if the exec command was used when args 0 is that of a shim.

jimeh avatar Feb 25 '23 00:02 jimeh

In terms of emacs, what would you do if you wanted to integrate direnv? rtx has almost identical behavior so whatever works for direnv would work for rtx. That may involve writing an emacs-rtx extension. You could use rtx direnv activate and just use a direnv plugin which would work too. That doesn't mean you need to use direnv everywhere else I don't think.

asdf uses the shims in part as a manifest that lets it do reverse lookups:

$ cat ~/.asdf/shims/node
#!/usr/bin/env bash
# asdf-plugin: nodejs 14.21.2
# asdf-plugin: nodejs 16.19.0
# asdf-plugin: nodejs 18.13.0
# asdf-plugin: nodejs 19.4.0
# asdf-plugin: nodejs lts-fermium
exec /opt/homebrew/Cellar/asdf/0.11.1/libexec/bin/asdf exec "node" "$@" # asdf_allow: ' asdf '

we would need the same for at least 2 reasons: knowing when we can delete shims, and being able to use rtx which node. I suspect there are more reasons I haven't considered yet.

However it doesn't have to be in that file. We could put it next to it in a dotfile or something perhaps. Or maybe somewhere else entirely. Or maybe we can just "reshim" on demand and not store it at all.

These are all questions I'll have to sort out.

jdx avatar Feb 25 '23 00:02 jdx

Hmm, the direnv approach might work in the short term. I'll have to experiment.

The current direnv package for Emacs changes env vars within Emacs based on what buffer/file is in focus. In theory this should be fine most of the time. But there are commands you can run that target specific files/directories without first visiting a file, which in theory could lead to unexpected env var values being used. I'll test it out and see how it goes :)

Are for the reverse lookup, is that truly needed? During a rehash/reshim operation, I'd imagine you'd need to iterate over every executable from every installed version of every tool/runtime/etc, to ensure you've created shims for everything.

At that point, you'd have an absolute list of all current shims that should exist, allowing you to simply remove any shims on disk that isn't part of that list.

Though I can image doing a targeted reshim for just the one version that was just installed/uninstalled might be desirable for performance purposes. Though I would imagine a full reshim of all the things can probably be done more than fast enough.

As for shims being sh scripts or just symlinks, the script variant should be more fast enough in theory if all it does is call something like exec /path/to/rtx exec "$(basename "$0")" -- "$@".

In the case of asdf, the shims adds overhead compared to directly calling the underlying binaries. I saw around 200ms extra time to run go version yesterday. However, I imagine that's mostly from within asdf itself as it figures out what the underlying executable to use and what env vars to pass it.

jimeh avatar Feb 25 '23 15:02 jimeh

Yes, shims would add overhead, but in my testing it is 20-200x faster than asdf. It also does what asdf does and only add overhead to the first call in a process tree by modifying PATH for subprocesses

jdx avatar Feb 25 '23 15:02 jdx

experimental support has been added in #213. They use symlinks as @jimeh suggested, which is definitely cleaner. We'll see how well it works I suppose.

please try it and let me know how it works

jdx avatar Feb 26 '23 22:02 jdx

@jdxcode Awesome, thanks for the quick turnaround on this :D

I've given it a quick try, and it basically does what it says on the tin. But I've had an issue and couple of annoyances:

  • When invoking a shim for a binary not available in the active install, the command just hangs without any output. This scenario should probably print an error to STDERR (ideally with details of installs that it is available in) and return a non-zero exit code. In my case I tested with gopls available in Go 1.20.1, and switched to Go 1.19.6 where I had not installed it.
  • I ran into a File exists (os error 17) error with reshim, due to having shims in place already that symlinked to a different rtx binary path. This is probably desired behavior as is, but maybe a more informative error message could help avoid confusion, or even add a --force flag which will take over any existing shims that are symlinked to other rtx binaries.
  • Docs and error messages calls the setting shim_dir (singular) in a few places, but the actual value I had to use was shims_dir (plural).

jimeh avatar Feb 26 '23 23:02 jimeh

lmk how things look in 1.18.1

jdx avatar Feb 27 '23 01:02 jdx

Invoking shims for missing binaries now errors out as I'd expect, meaning I'm using rtx for the rest of the work day :)

I'll poke around a bit more after work to see if there's any other issues/annoyances.

And out of curiosity I did a quick test, and go version through the shim runs in about 30-35ms vs 10-18ms through the shim. I'd say only 10-20ms slowdown is awesome, and a massive improvement over the ~200ms slowdown I saw with asdf.

jimeh avatar Feb 27 '23 11:02 jimeh

@jdxcode The reason I mentioned it was because I had shims running from inside Sublime LSP that somehow went into an infinite loop consuming all CPU. When adding that to the shim the problem was fixed 🤷

eproxus avatar Feb 27 '23 12:02 eproxus

@eproxus that shouldn't be necessary anymore

jdx avatar Feb 27 '23 13:02 jdx

@jdxcode Lovely! Thanks!

eproxus avatar Feb 28 '23 12:02 eproxus

I tried the experimental shims support today and ran into an issue while testing it.

Previously with asdf, I had some global tool-versions set to system, which I no longer needed with rtx. In some projects, I then set specific versions of a tool.

With the current rtx shims implementation, I'm not able to get the shimmed binary to call the system version.

I tested two things (sample tool terraform):

  1. terraform version set in local project, not set at all in ~/.tool-versions Results in terraform is not a valid shim message when called from ~/
Detailed steps in shell
❯ zi rtx-sys
❯ cat .tool-versions
terraform 1.3.0

❯ rtx reshim
❯ ~/bin/shims/terraform --version
Terraform v1.3.0
on darwin_arm64

Your version of Terraform is out of date! The latest version
is 1.3.9. You can update by downloading from https://www.terraform.io/downloads.html
❯ cd ~
❯ cat .tool-versions
python    3.10
ruby      3.1

❯ ~/bin/shims/terraform --version
[DEBUG] rtx::config: Files: ~/.tool-versions, ~/.config/rtx/config.toml
Installed Plugins: direnv, flutter, golang, helm, hugo, java, kubectl, maven, nodejs, pulumi, python, ruby, structurizr-cli, terraform
[DEBUG] rtx::toolset::builder: [email protected], nodejs@18, java@temurin-17, [email protected], [email protected]
Error:
   0: terraform is not a valid shim

Location:
   src/shims.rs:34

Version:
   1.20.4 macos-arm64 (built 2023-03-03)

Suggestion: Run with RTX_DEBUG=1 for more information.

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
[DEBUG] rtx::config: Files: ~/.tool-versions, ~/.config/rtx/config.toml
Installed Plugins: direnv, flutter, golang, helm, hugo, java, kubectl, maven, nodejs, pulumi, python, ruby, structurizr-cli, terraform

  1. Then I thought, the error might be valid and I just need to add terraform system to my global config to get this working. Unfortunately, it seems rtx has a bug when using system version like terraform system, as I always get the following warning: [WARN] Tool not installed: terraform@system
Detailed steps in shell
# opening new login shell shows following warning with eval "$(rtx activate --quiet zsh)"
[WARN] Tool not installed: terraform@system
❯ which terraform
/opt/homebrew/bin/terraform
❯ rtx global terraform@system
✔ Select versions to install · terraform@system
python    3.10.10
terraform system
[WARN] Tool not installed: terraform@system
❯ rtx reshim
❯ ~/bin/shims/terraform --version
zsh: no such file or directory: /Users/user/bin/shims/terraform
❯ cat ~/.tool-versions
python    3.10.10
terraform  system

# tried with and without asdf compat mode enabled
❯ rtx settings get asdf_compat
true

# rtx exec asks me to select version to install every single time
❯ rtx x terraform@system -- terraform --version
✔ Select versions to install · terraform@system
Terraform v1.3.9
on darwin_arm64

❯ rtx x terraform@system -- terraform --version
✔ Select versions to install · terraform@system
Terraform v1.3.9
on darwin_arm64



I think, when the bug with the system version is fixed, the shims can be used as expected.

amoosbr avatar Mar 03 '23 16:03 amoosbr

I just remembered, that rtx no longer sets the RUBYLIB env variable from the ruby plugin (Reference to related rtx code). With shims, the variable should be set again. It contains a ruby file calling asdf reshim ruby, when you install a new ruby binary bundle.

Initially, RUBYLIB was blacklisted, as the rtx activate hook concatenated RUBYLIB paths when switching directories. Not sure, if this is still the case.

amoosbr avatar Mar 03 '23 17:03 amoosbr

that ruby file isn't going to work. It calls asdf reshim, not rtx reshim. In plugins I do some work to re-route that back to rtx but I wouldn't be able to do that here. I may need to fork the ruby plugin.

jdx avatar Mar 03 '23 19:03 jdx

that ruby file isn't going to work. It calls asdf reshim, not rtx reshim. In plugins I do some work to re-route that back to rtx but I wouldn't be able to do that here. I may need to fork the ruby plugin.

Could the path be modified before executing these and add an asdf shim when we know we’re executing from within rtx?

eproxus avatar Mar 04 '23 11:03 eproxus

I don't like the idea of changing asdf to map to rtx for the user. I don't think there is a way to only do it when this ruby script is called. I think it's totally reasonable to use both tools. I'd rather fork the plugin.

jdx avatar Mar 04 '23 14:03 jdx

I'm going to close this thread. At a later point I'll mark this functionality as non-experimental and probably default the shim farm somewhere since it shouldn't hurt people that don't use it.

If there are issues with ruby or other plugins, please open a separate ticket.

jdx avatar Mar 04 '23 23:03 jdx

Thanks for the first round of system version tools fixing. Unfortunately, I think it still needs another fix for shims mixing system and local versions. I opened #276 with the details.

To shim the rubygem installed binaries, my first thought to just symlink asdf to rtx and export RUBYLIB="$ruby_plugin_dir/rubygems-plugin" myself. Obviously, this didn't work, as rtx expects symlinks to itself as shimmed binaries now. Therefore I copied rubygems_plugin.rb locally, modified the calls from asdf to rtx and exported the path as RUBYLIB. This seems to be working for me. For the general audience, forking is probably the better solution. Or get rtx support upstream, if possible.

Due to using the shims, I also needed to use a modified version of set-java-home.zsh to set "JAVA_HOME" in my zshrc.

amoosbr avatar Mar 06 '23 11:03 amoosbr