mise
mise copied to clipboard
jdx
rtx-activate: less aggressive PATH modifications
Currently, when using rtx activate, rtx will prepend PATH entries to the very front of PATH, e.g.:
PATH="~/.local/share/rtx/installs/java/xx/bin:/usr/bin:..."
However this might be more aggressive than it should be. This means it's basically impossible to override rtx and allow the user to add custom PATH entries ahead of runtimes rtx adds with rtx activate.
Shims are one solution, but rtx activate could be a bit softer here and instead inject the PATH entries at the location of the rtx bin itself. This means if PATH was initially the following:
PATH="~/bin:~/.local/share/rtx/bin:/usr/bin"
then after running rtx activate it could change to this:
PATH="~/bin:~/.local/share/rtx/installs/java/xx/bin:~/.local/share/rtx/bin:/usr/bin"
Note that ~/bin is before the install directories.
Now while I could see use-cases where users want this, it also might be undesirable since the place that rtx is added to PATH matters. Most users will probably want it at the front or near the front regardless. For that reason, we probably want this to be an option (though I'm not sure what default is best).
FYI this is likely a somewhat challenging task. The way rtx sets up env vars is complex and it's also some of the oldest code in the codebase where I was still learning rust. That said, I think this line is what will need to be modified:
https://github.com/jdx/rtx/blob/58b0e0e56f24fb12fae8d36cdf245dcec0808796/src/cli/hook_env.rs#L92
I actually just ran into this and as far as I could tell there was no way for me to change this in my own config. The use case I had was chef-workstation and the bundled gems as part of this. By default rtx would try and load gems that were loaded into my default ruby version.
I would suspect there are a lot of edge cases here because of the various setups people have. For example, I could see a situation for my own setup where based on the project I was in (some projects actually do manage these gems directly) i would want rtx to manage the dependencies however by default its much simpler to just use the gems bundled with chef-workstation.
It also messes up any Python virtual environment in cases where I’m not using the plugin’s built-in virtual environment handling. I have a separate listener that reacts to $PWD change and activates the environment if there is one. But even without — if I do it manually — RTX will change $PATH eventually, and virtual environment binaries will fall behind.
How eventually? It’s a mystery. It can occur right on the following prompt or after a few commands.
I can’t stress enough how often I installed something by PIP as a global package instead of an environment one because of this issue.
The obvious solution would be to have the .rtx.toml in each directory with a virtual environment, with the venv key explicitly configured. But I don’t want excessive entities without any apparent reason for it. It worked perfectly before.
My solution was to write yet another listener that reacts to the prompt event and checks if the first element of $PATH resembles a virtual environment path, then re-activates the environment if necessary. But it’s a crutch solution for sure.
@Aeron you might have better luck not using rtx activate and just sticking with shims
Well, yeah, but Python is not the only plugin I use. It’s not the “Hey, fix my specific case” demand, by no means. But if it treats $PATH that way, it could be a problem not only for Python’s virtual environment.
If RTX needs to listen for shell’s prompt and preexec events and force its way regularly, then it probably should emit custom events. Therefore, users can have more control over it for their narrow cases.
Otherwise, it would be a million specific cases to deal with. And it’s not like RTX has (or should have) a monopoly on the last word for $PATH look. At least, that’s what I think.
Yet, in my specific case, it would be wonderful if RTX could respect not only .rtx.toml in $PWD but also existing $VIRTUAL_ENV. I believe it’s a normal behavior for any virtual environment handler.
@Aeron I'm not 100% but it sounds like you think my plan of inserting runtimes into PATH at the location of the rtx bin would be insufficient and you'd prefer more control via events?
I feel like inserting instead of just always prepending to the front would work, though it might in some cases require installing rtx to a dedicated directory (one reason I think it should be behind a config setting of some kind).
Let me know if I got what you said correct though, I wasn't 100%.
In general, virtualenv stuff needs to be improved. (For users using rtx virtualenv activation as well as those that are not) I know it's a well-liked feature but has notable gaps.
Oh, no-no, I don’t think your plan would not succeed, but as you said, it is a somewhat challenging task. Events are an option, alternative solution, or option B, perhaps easier and faster to implement. If anything, I agree with you and the general idea.
I like this solution, it may also be worth considering an addition to rtx doctor here that will list any possible overrides that could be impacted ability to use rtx installed tools. I can imagine this will help with people debugging own issues, as well as helping resolve any opened issues of people having problems with PATHs.
Python people probably want $VIRTUAL_ENV paths before the rtx activate paths. I know I work around this by changing the events my rtx activate and auto venv activator trigger on.
Hey jdx thanks a lot for the work on this issue, this has been a recurring issue for me working with python ! Looks like the path is updated the old way in hook-env.rs
The following bit of code
let new_path = join_paths([installs.clone(), env::PATH.clone()].concat())?
.to_string_lossy()
.to_string();
could be replaced with
let mut path_env = PathEnv::from_iter(env::PATH.clone());
for p in installs.clone() {
path_env.add(p);
}
let new_path = path_env.join().to_string_lossy().to_string();
Do you want me to open a PR for this ?
One idea I had is maybe we could keep track of any paths added to the front since activate was first launched. I think we can compare the difference between env::var("PATH") and env::PRISTINE_ENV["PATH"] which would tell us that
I tried out that idea I had and it seems to work. Would anyone mind trying out by installing from HEAD?
Not sure that the fix solves the issue (at least for my use case) Here's the top of my path just after activating a python virtualenv
And the same command a few minutes later
(compiled from master)
I'm confused, isn't 9160c4f the commit that fixed it ? I used the cargo install rtx-cli --git https://github.com/jdx/rtx --branch main command to install it
oh apologies. You're running in a new shell and everything? does echo $__RTX_ORIG_PATH print anything?
it seems to work for me in fish at least:
~ ❯ python -m venv .venv
~ ❯ echo $PATH
/Users/jdx/.rtx/installs/ruby/3.2.2/bin ...
~ ❯ source .venv/bin/activate.fish
~ ❯ echo $PATH
/Users/jdx/.venv/bin /Users/jdx/.rtx/installs/ruby/3.2.2/bin ...
~ ❯ cd ~/src/rtx
rtx main ❯ echo $PATH
/Users/jdx/.venv/bin /Users/jdx/.rtx/installs/tiny/1.1.0/bin ...
ah yeah and I had shims remaining as a test for the first version of the fix, would that affect it ?
ok so I tried removing the shims directory but it did not change the result. Also adjusted the order of initialization in my config.fish file, and I can see the $__RTX_ORIG_PATH variable. Do you have a consistent way to trigger the hook ? would be easier for testing.
Also, food for thoughts, the way poetry shell does its thing is by creating a subshell, then sourcing th virtualenv file; could that affect the behaviour of rtx ?
let's try this to make sure it's working at all:
$ rtx activate fish | source # remove this from your config.fish temporarily and just run it in a subshell so we can isolate
$ set -gx PATH FOO $PATH # this adds "FOO" (an invalid path entry, but easy to spot), it should stay at the very front because it's not in $__RTX_ORIG_PATH
$ echo $PATH # verify FOO is in front
$ cd ... # enter some directory with a .tool-versions or .rtx.toml file
$ echo $PATH # verify the new tools in this directory are picked up, but "FOO" is still in front of PATH
one thing that might be happening: if something is modifying PATH aside from simply adding something to the front then it won't work. I might need to try messing with poetry here.
Also, are you using the automatic venv activation for python or poetry? Or just running source .venv/bin/activate.fish?
Commands above work as expected
I use the poetry shell command to activate manually my virtualenv. I'm going to try to reproduce the error with poetry today
Was able to reproduce consistently in a VM, here's the log
[ec2-user@ip-10-62-56-74 test-project]$ rtx doctor
rtx version:
2023.12.25 linux-arm64 (e1f4173 2023-12-13)
build:
Target: aarch64-unknown-linux-gnu
Features: DEFAULT, NATIVE_TLS, OPENSSL
Built: Wed, 13 Dec 2023 06:55:22 +0000
Rust Version: rustc 1.74.1 (a28077b28 2023-12-04)
Profile: release
shell:
/bin/bash
GNU bash, version 5.2.15(1)-release (aarch64-amazon-linux-gnu)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
rtx data directory:
/home/ec2-user/.local/share/rtx
rtx environment variables:
RTX_SHELL=bash
settings:
{"always_keep_download": "false", "always_keep_install": "false", "asdf_compat": "false", "disable_default_shorthands": "false", "disable_tools": "[]", "experimental": "false", "jobs": "4", "legacy_version_file": "true", "legacy_version_file_disable_tools": "[]", "plugin_autoupdate_last_check_duration": "7d", "raw": "false", "trusted_config_paths": "[]", "verbose": "false", "yes": "false"}
config files:
/home/ec2-user/.tool-versions
/home/ec2-user/test-project/.rtx.toml
plugins:
bun (core)
deno (core)
go (core)
java (core)
node (core)
python (core)
ruby (core)
toolset:
node@20, [email protected]
No problems found
[ec2-user@ip-10-62-56-74 test-project]$ poetry shell
Spawning shell within /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12
. /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin/activate
[ec2-user@ip-10-62-56-74 test-project]$ . /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin/activate
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ tr ':' '\n' <<< "$PATH"
/home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin
/home/ec2-user/.local/share/rtx/installs/node/20/bin
/home/ec2-user/.local/share/rtx/installs/python/3.12/bin
/home/ec2-user/.local/share/rtx/bin
/home/ec2-user/.cargo/bin
/home/ec2-user/.local/bin
/home/ec2-user/bin
/usr/local/sbin
/usr/local/bin
/usr/sbin
/usr/bin
/sbin
/bin
/var/lib/snapd/snap/bin
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ rtx use node@20
rtx ~/test-project/.rtx.toml node@20
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ tr ':' '\n' <<< "$PATH"
/home/ec2-user/.local/share/rtx/installs/node/20/bin
/home/ec2-user/.local/share/rtx/installs/python/3.12/bin
/home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin
/home/ec2-user/.local/share/rtx/bin
/home/ec2-user/.cargo/bin
/home/ec2-user/.local/bin
/home/ec2-user/bin
/usr/local/sbin
/usr/local/bin
/usr/sbin
/usr/bin
/sbin
/bin
/var/lib/snapd/snap/bin
Please let me know if there is any flaw in my approach
update: this has been released in 2023.12.25 but it's under experimental so set RTX_EXPERIMENTAL=1 to test it
Added the export at the beginning, should be enough to enable ?
[ec2-user@ip-10-62-56-74 test-project]$ RTX_EXPERIMENTAL=1
[ec2-user@ip-10-62-56-74 test-project]$ rtx doctor
rtx version:
2023.12.25 linux-arm64 (e1f4173 2023-12-13)
build:
Target: aarch64-unknown-linux-gnu
Features: DEFAULT, NATIVE_TLS, OPENSSL
Built: Wed, 13 Dec 2023 06:55:22 +0000
Rust Version: rustc 1.74.1 (a28077b28 2023-12-04)
Profile: release
shell:
/bin/bash
GNU bash, version 5.2.15(1)-release (aarch64-amazon-linux-gnu)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
rtx data directory:
/home/ec2-user/.local/share/rtx
rtx environment variables:
RTX_SHELL=bash
settings:
{"always_keep_download": "false", "always_keep_install": "false", "asdf_compat": "false", "disable_default_shorthands": "false", "disable_tools": "[]", "experimental": "false", "jobs": "4", "legacy_version_file": "true", "legacy_version_file_disable_tools": "[]", "plugin_autoupdate_last_check_duration": "7d", "raw": "false", "trusted_config_paths": "[]", "verbose": "false", "yes": "false"}
config files:
/home/ec2-user/.tool-versions
/home/ec2-user/test-project/.rtx.toml
plugins:
bun (core)
deno (core)
go (core)
java (core)
node (core)
python (core)
ruby (core)
toolset:
node@20, [email protected]
No problems found
[ec2-user@ip-10-62-56-74 test-project]$ poetry shell
Spawning shell within /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12
. /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin/activate
[ec2-user@ip-10-62-56-74 test-project]$ . /home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin/activate
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ tr ':' '\n' <<< "$PATH"
/home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin
/home/ec2-user/.local/share/rtx/installs/node/20/bin
/home/ec2-user/.local/share/rtx/installs/python/3.12/bin
/home/ec2-user/.local/share/rtx/bin
/home/ec2-user/.cargo/bin
/home/ec2-user/.local/bin
/home/ec2-user/bin
/usr/local/sbin
/usr/local/bin
/usr/sbin
/usr/bin
/sbin
/bin
/var/lib/snapd/snap/bin
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ rtx use node@20
rtx ~/test-project/.rtx.toml node@20
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$ tr ':' '\n' <<< "$PATH"
/home/ec2-user/.local/share/rtx/installs/node/20/bin
/home/ec2-user/.local/share/rtx/installs/python/3.12/bin
/home/ec2-user/.cache/pypoetry/virtualenvs/test-project-G_hDmf5f-py3.12/bin
/home/ec2-user/.local/share/rtx/bin
/home/ec2-user/.cargo/bin
/home/ec2-user/.local/bin
/home/ec2-user/bin
/usr/local/sbin
/usr/local/bin
/usr/sbin
/usr/bin
/sbin
/bin
/var/lib/snapd/snap/bin
(test-project-py3.12) [ec2-user@ip-10-62-56-74 test-project]$
no, it needs to be there when you call rtx activate. With fish you can use a universal variable: set -Ux RTX_EXPERIMENTAL 1. You can also use a setting with rtx settings set experimental 1
the way it works is when you call rtx activate it will save PATH at that moment to __RTX_ORIG_PATH. Later, when rtx calls rtx hook-env before the prompt is displayed, it checks that variable and if it matches the end of the current PATH it will take everything added after __RTX_ORIG_PATH was set and prepend it before rtx bins.
So effectively what this should do is make it so if you do this:
set PATH A $PATH
rtx activate fish | source
set PATH B $PATH
then PATH will be the following:
B:[RTXBINS]:A:...
what might be happening (I need to look), is that poetry is modifying the PATH in a way that changes the things that were set in __RTX_ORIG_PATH. If that does not match the suffix of PATH then rtx will just ignore it and prepend things to the beginning. In other words it might be doing something like this:
set PATH A $PATH
rtx activate $PATH
set PATH B $PATH
set PATH $PATH C
in that case, because "C" is added to the end of PATH rtx should generate this with nothing prepended:
[RTXBINS]:B:A:...:C
That's a use-case I might need to think about a bit more. Maybe instead of saving the entire suffix I could just find the first entry in PATH and prepend things before that.