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

_.python.venv does not correctly prepend the venv path if a virtualenv is active (fish shell) #2718

Open
albireox opened this issue Oct 9, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@albireox
Copy link

albireox commented Oct 9, 2024

Describe the bug

When one already has a virtualenv active, a .mise.toml file with a _.python.venv directive will add the commanded venv to PATH but always after the already active one. For example if I have a $PATH like

/Users/gallegoj/.local/share/virtualenv/gallegoj/bin /Users/gallegoj/.cargo/bin /usr/local/bin /Users/gallegoj/.local/bin /usr/bin /bin

where /Users/gallegoj/.local/share/virtualenv/gallegoj/bin is an activated environment, and cd to a directory with a .mise.toml

[env]
_.python.venv = ".venv"

the resulting $PATH will be

/Users/gallegoj/.local/share/virtualenv/gallegoj/bin /Users/gallegoj/Documents/Code/sdss5/sdss-sep/.venv/bin /Users/gallegoj/.cargo/bin /usr/local/bin /Users/gallegoj/.local/bin /usr/bin /bin

instead of the expected

/Users/gallegoj/Documents/Code/sdss5/sdss-sep/.venv/bin /Users/gallegoj/.local/share/virtualenv/gallegoj/bin /Users/gallegoj/.cargo/bin /usr/local/bin /Users/gallegoj/.local/bin /usr/bin /bin

I've only tested this in fish.

mise doctor output

version: 2024.10.1 macos-x64 (7184e60 2024-10-07)
activated: yes
shims_on_path: no

build_info:
  Target: x86_64-apple-darwin
  Features: DEFAULT, NATIVE_TLS, OPENSSL
  Built: Mon, 7 Oct 2024 19:08:40 +0000
  Rust Version: rustc 1.81.0 (eeb90cda1 2024-09-04)
  Profile: release

shell:
  /usr/local/bin/fish
  fish, version 3.7.1-2332-g4c43819d3

dirs:
  data: ~/.local/share/mise
  config: ~/.config/mise
  cache: ~/Library/Caches/mise
  state: ~/.local/state/mise
  shims: ~/.local/share/mise/shims

config_files:
  ~/Documents/Code/sdss5/sdss-sep/.python-version
  ~/Documents/Code/sdss5/sdss-sep/.mise.toml

backends:
  cargo
  core
  go
  npm
  pipx
  spm
  ubi
  vfox

plugins:

toolset:
  python@sdss-sep  (missing)

env_vars:
  MISE_SHELL=fish

settings:
  activate_aggressive = false
  all_compile = false
  always_keep_download = false
  always_keep_install = false
  asdf = true
  asdf_compat = false
  cache_prune_age = "30d"
  cargo_binstall = true
  ci = false
  color = true
  debug = false
  disable_default_shorthands = false
  disable_hints = []
  disable_tools = []
  experimental = false
  go_default_packages_file = "~/.default-go-packages"
  go_download_mirror = "https://dl.google.com/go"
  go_repo = "https://github.com/golang/go"
  go_set_gopath = false
  go_set_goroot = true
  go_skip_checksum = false
  http_timeout = 30
  jobs = 4
  legacy_version_file = true
  legacy_version_file_disable_tools = []
  libgit2 = true
  log_level = "info"
  not_found_auto_install = true
  paranoid = false
  pin = false
  pipx_uvx = false
  plugin_autoupdate_last_check_duration = "7d"
  python_default_packages_file = "~/.default-python-packages"
  python_pyenv_repo = "https://github.com/pyenv/pyenv.git"
  python_venv_auto_create = false
  quiet = false
  raw = false
  trace = false
  trusted_config_paths = []
  use_versions_host = true
  verbose = false
  vfox = false
  yes = false

  [node]

  [ruby]
  default_packages_file = "~/.default-gems"
  ruby_build_repo = "https://github.com/rbenv/ruby-build.git"
  ruby_install = false
  ruby_install_repo = "https://github.com/postmodern/ruby-install.git"

  [status]
  missing_tools = "if_other_versions_installed"
  show_env = false
  show_tools = false
No warnings found
No problems found
@albireox albireox added the bug Something isn't working label Oct 9, 2024
@hxii
Copy link

hxii commented Oct 30, 2024

Seconding this issue:

Paul_Glushak@Paul ~> pwd
/Users/Paul_Glushak
Paul_Glushak@Paul ~> echo $PATH | sed 's/ /\n/g'
/Users/Paul_Glushak/dev/forter-dev-cli/.venv/bin
/Users/Paul_Glushak/.local/share/mise/installs/pipx/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/python/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/poetry/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/usage/latest/bin
/Users/Paul_Glushak/.local/bin
/opt/homebrew/bin
...

Then cd somewhere else

Paul_Glushak@Paul ~> z boku
Paul_Glushak@Paul ~/d/boku (main)> cat .mise.toml
───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: .mise.toml
───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ [tools]
   2   │ python = "latest"
   3   │
   4   │ [env]
   5   │ _.python.venv = { path = ".venv", create = true }
───────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Paul_Glushak@Paul ~/d/boku (main)> echo $PATH | sed 's/ /\n/g'
/Users/Paul_Glushak/dev/forter-dev-cli/.venv/bin
/Users/Paul_Glushak/dev/boku/.venv/bin
/Users/Paul_Glushak/.local/share/mise/installs/python/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/pipx/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/poetry/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/usage/latest/bin
/Users/Paul_Glushak/.local/bin
/opt/homebrew/bin
...
Paul_Glushak@Paul ~/d/boku (main)>

It looks like the incorrect path value exists in both $PATH as well as $MISE_SHELL:

MISE_SHELL=fish set -gx __MISE_ORIG_PATH /Users/Paul_Glushak/dev/forter-dev-cli/.venv/bin /Users/Paul_Glushak/.local/share/mise/installs/pipx/latest/bin ...

@jdx
Copy link
Owner

jdx commented Nov 1, 2024

this is by design, mise doesn't modify PATH to override things after it was activated to prevent it from aggressively modifying PATH. The activate_aggressive setting modifies this behavior.

@hxii
Copy link

hxii commented Nov 5, 2024

@jdx am I understanding correctly that the aggressive setting will prepend the new path, while the default should overwrite the old one? If so, the issue is that it’s not being overwritten.
Perhaps I misunderstood?

@Its-Alex
Copy link
Contributor

Its-Alex commented Nov 12, 2024

@jdx Hello 👋 Sorry to disturb you, but have you an update on this? 🙏

From your point of view, how are we supposed to use the venv directory correctly in this case? Because by default a venv directory is created but never used 😕

@syhol
Copy link
Contributor

syhol commented Nov 18, 2024

Not an expert but hope I can lend a thought here.

the aggressive setting will prepend the new path

Yes, but not just the new path, but all mise paths:

e.g.
initial state:

/Users/Paul_Glushak/dev/forter-dev-cli/.venv/bin <-- venv not managed by mise
/Users/Paul_Glushak/dev/boku/.venv/bin
/Users/Paul_Glushak/.local/share/mise/installs/python/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/pipx/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/poetry/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/usage/latest/bin
/Users/Paul_Glushak/.local/bin
/opt/homebrew/bin

to

/Users/Paul_Glushak/dev/boku/.venv/bin
/Users/Paul_Glushak/.local/share/mise/installs/python/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/pipx/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/poetry/latest/bin
/Users/Paul_Glushak/.local/share/mise/installs/usage/latest/bin
/Users/Paul_Glushak/dev/forter-dev-cli/.venv/bin <-- venv not managed by mise
/Users/Paul_Glushak/.local/bin
/opt/homebrew/bin

while the default should overwrite the old one

Depends what you mean by "old one". I don't believe mise ever overwrites/deletes any paths other than the ones it manages.

If mise was managing both then it would unload the old one and load the new one

Like this
~/Code/sandboxmkdir py1 py2

~/Code/sandboxcat > py1/mise.toml
[tools]
python = "latest"
[env]
_.python.venv = { path = ".venv", create = true }

~/Code/sandboxcp py1/mise.toml py2/

~/Code/sandboxmise trust py1/mise.toml; mise trust py2/mise.toml;
mise trusted /Users/syhol/Code/sandbox/py1/mise.toml
mise trusted /Users/syhol/Code/sandbox/py2/mise.toml

~/Code/sandboxcd py1

Code/sandbox/py1 via 🐍 v3.11.10 (.venv)echo $PATH | sed 's/ /\n/g'
/Users/syhol/Code/sandbox/py1/.venv/bin
/Users/syhol/.local/share/mise/installs/python/3.11.10/bin
...

Code/sandbox/py1 via 🐍 v3.11.10 (.venv)cd ../py2

Code/sandbox/py2 via 🐍 v3.11.10 (.venv)echo $PATH | sed 's/ /\n/g'
/Users/syhol/Code/sandbox/py2/.venv/bin
/Users/syhol/.local/share/mise/installs/python/3.11.10/bin
...

If /Users/Paul_Glushak/dev/forter-dev-cli/.venv or /Users/gallegoj/.local/share/virtualenv/gallegoj/bin is not managed by mise, then I would say its not mise's responsibility to deactivate them. Is there a precedent for this behaviour with any other tool? I don't mean to be dismissive, I'm just curious where your expectations come from.

For some reason my install of mise seems to always do the "aggressive setting" behaviour even though I haven't enabled it, so I can't recreate your behaviour with an unmanaged uv venv . but it sounds like what you're experiencing is actually intended mise behaviour.

My recommendation is to do one of the following:

  • run deactivate on "non mise managed" envs to remove the old env path when entering a mise managed env
  • run mise settings set activate_aggressive true (from jdx's comment) and you might need to restart your shell after changing this setting for it to take effect, I'm not sure.
  • Use mise to manage all venvs on your system 😁 but I know thats not always possible

I hope I've understood your problem properly, let me know if I've gotten anything wrong.

@Its-Alex
Copy link
Contributor

Hello 👋 @syhol

Thanks for your reply, concerning this point:

If /Users/Paul_Glushak/dev/forter-dev-cli/.venv or /Users/gallegoj/.local/share/virtualenv/gallegoj/bin is not managed by mise, then I would say its not mise's responsibility to deactivate them. Is there a precedent for this behaviour with any other tool? I don't mean to be dismissive, I'm just curious where your expectations come from.

If we use _.python.venv = ".venv" mise is supposed to be responsible of this virtual environment no? If no, what is the best way to managed it? If yes why is it not used by default without using activate_aggressive=true?

@jdx
Copy link
Owner

jdx commented Nov 19, 2024

I don't think I understand this use-case. It sounds like you've activated a venv after mise, then are wanting mise to override that venv. Why activate the other one at all? I would just use mise for both of them.

mise is careful not to make modifications to PATH that overwrite changes after it was started. Otherwise it would be impossible to do something like export PATH="somenewpath:$PATH" because mise would just stomp on top of it. As far as mise is concerned, venv is just modifying env vars like PATH so it's not special. This is the behavior that activate_aggressive disables.

@syhol
Copy link
Contributor

syhol commented Nov 19, 2024

mise is actually activating the environment, check echo $VIRTUAL_ENV in this situation and you should see the correct directory. jdx explains why the path is not matching your expectation.

However, to your credit, I checked what happens when you have 2 venvs (both not managed by mise) and activate them one after the other. The second activation did remove the path from the first.

uv's venv clobbering old venv paths
# Code/sandbox/unmanaged-venvuv venv .venv
Using Python 3.13.0 interpreter at: /opt/homebrew/opt/[email protected]/bin/python3.13
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate.fish

# Code/sandbox/unmanaged-venvsource .venv/bin/activate.fish

# Code/sandbox/unmanaged-venv via 🐍 v3.13.0 (unmanaged-venv)echo $PATH | sed 's/ /\n/g'
/Users/simon/Code/sandbox/unmanaged-venv/.venv/bin
/Users/simon/.local/share/mise/installs/node/21.4.0/bin
...

# ~/Code/sandbox/unmanaged-venv via 🐍 v3.13.0 (unmanaged-venv)mkdir ../unmanaged-venv-2

# ~/Code/sandbox/unmanaged-venv via 🐍 v3.13.0 (unmanaged-venv)cd ../unmanaged-venv-2/

# Code/sandbox/unmanaged-venv-2 via 🐍 v3.13.0 (unmanaged-venv)uv venv .venv
Using Python 3.13.0 interpreter at: /opt/homebrew/opt/[email protected]/bin/python3.13
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate.fish

# Code/sandbox/unmanaged-venv-2 via 🐍 v3.13.0 (unmanaged-venv)source .venv/bin/activate.fish

# Code/sandbox/unmanaged-venv-2 via 🐍 v3.13.0 (unmanaged-venv-2)echo $PATH | sed 's/ /\n/g'
/Users/syhol/Code/sandbox/unmanaged-venv-2/.venv/bin
/Users/syhol/.local/share/mise/installs/node/21.4.0/bin
...

@jdx would you be open to matching this behaviour? If you check to see what .venv/bin/activate.fish does, it calls the deactivate function before activating the new venv. The deactivate function uses a env var called $_OLD_VIRTUAL_PATH to reset the PATH:

venv deactivate fish function (runs on activate to clear out the old venv)

Here is the source of the virtualenv version https://github.com/pypa/virtualenv/blob/main/src/virtualenv/activation/fish/activate.fish

and a very similar source for uv:
https://github.com/astral-sh/uv/blob/main/crates/uv-virtualenv/src/activator/activate.fish

function deactivate -d 'Exit virtualenv mode and return to the normal environment.'
    # reset old environment variables
    if test -n "$_OLD_VIRTUAL_PATH"
        # https://github.com/fish-shell/fish-shell/issues/436 altered PATH handling
        if test (echo $FISH_VERSION | head -c 1) -lt 3
            set -gx PATH (_fishify_path "$_OLD_VIRTUAL_PATH")
        else
            set -gx PATH $_OLD_VIRTUAL_PATH
        end
        set -e _OLD_VIRTUAL_PATH
    end

    if test -n "$_OLD_VIRTUAL_PYTHONHOME"
        set -gx PYTHONHOME "$_OLD_VIRTUAL_PYTHONHOME"
        set -e _OLD_VIRTUAL_PYTHONHOME
    end

    if test -n "$_OLD_FISH_PROMPT_OVERRIDE"
       and functions -q _old_fish_prompt
        # Set an empty local `$fish_function_path` to allow the removal of `fish_prompt` using `functi
ons -e`.
        set -l fish_function_path

        # Erase virtualenv's `fish_prompt` and restore the original.
        functions -e fish_prompt
        functions -c _old_fish_prompt fish_prompt
        functions -e _old_fish_prompt
        set -e _OLD_FISH_PROMPT_OVERRIDE
    end

    set -e VIRTUAL_ENV
    set -e VIRTUAL_ENV_PROMPT

    if test "$argv[1]" != 'nondestructive'
        # Self-destruct!
        functions -e pydoc
        functions -e deactivate
        functions -e _bashify_path
        functions -e _fishify_path
    end
end

I can't see this behaviour in any formal spec but that does seem to be the practical standard for venv activation. Also it is what many (at least 3) people expect to happen. What do you think?

@jdx
Copy link
Owner

jdx commented Nov 19, 2024

I would be, though I don't think we have any mechanism for removing entries from PATH. I don't think that would be simple to achieve.

@jdx
Copy link
Owner

jdx commented Nov 19, 2024

what would probably need to be done is to have a special env var sort of like how we use MISE_ADD_PATH called MISE_RM_PATH that the plugin gets from here:

hm.insert("MISE_ADD_PATH".into(), bin.to_string_lossy().into());

however we wouldn't be able to use that in the same place MISE_ADD_PATH is consumed I don't think. This logic might need to go into hook-env directly to make the removal there. That might be tricky since the logic does a lot of dancing around undoing what mise did. What we might do here is just remove it forever since we won't ever need it again.

That only would solve mise activate though. For shims and mise x I think this code would need to perform the removal:

mise/src/toolset/mod.rs

Lines 491 to 505 in 7ba9eab

pub fn env_with_path(&self, config: &Config) -> Result<BTreeMap<String, String>> {
let mut path_env = PathEnv::from_iter(env::PATH.clone());
for p in config.path_dirs()?.clone() {
path_env.add(p);
}
let mut env = self.env(config)?;
if let Some(path) = env.get(&*PATH_KEY) {
path_env.add(PathBuf::from(path));
}
for p in self.list_paths() {
path_env.add(p);
}
env.insert(PATH_KEY.to_string(), path_env.to_string());
Ok(env)
}

@syhol
Copy link
Contributor

syhol commented Nov 19, 2024

Are you suggesting you set the old ${VIRTUAL_ENV}/bin to MISE_RM_PATH? It's a bit different from what venv does but it should be a fine solution. Alternatively, could you call get_pristine_env while overriding PATH with _OLD_VIRTUAL_PATH? I'm not sure how that would work with ToolSet.env though.

Re: shims and mise x - Since env_with_path adds the paths to the start, the new venv path would take precedence anyway. Removing the old venv path would just avoid exposing any extra executables.

just remove it forever since we won't ever need it again

yeah that should be fine.

@hxii
Copy link

hxii commented Nov 26, 2024

I don't think I understand this use-case. It sounds like you've activated a venv after mise, then are wanting mise to override that venv. Why activate the other one at all? I would just use mise for both of them.

mise is careful not to make modifications to PATH that overwrite changes after it was started. Otherwise it would be impossible to do something like export PATH="somenewpath:$PATH" because mise would just stomp on top of it. As far as mise is concerned, venv is just modifying env vars like PATH so it's not special. This is the behavior that activate_aggressive disables.

It could be that I missed something while configuring mise.

mise doctor output

version: 2024.11.29 macos-arm64 (2024-11-25)
activated: yes
shims_on_path: no

build_info:
  Target: aarch64-apple-darwin
  Features: DEFAULT, NATIVE_TLS
  Built: Mon, 25 Nov 2024 04:03:52 +0000
  Rust Version: rustc 1.82.0 (f6e511eec 2024-10-15) (Homebrew)
  Profile: release

shell:
  /opt/homebrew/bin/fish
  fish, version 3.7.1

dirs:
  cache: ~/Library/Caches/mise
  config: ~/.config/mise
  data: ~/.local/share/mise
  shims: ~/.local/share/mise/shims
  state: ~/.local/state/mise

config_files:
  ~/.config/mise/config.toml
  ~/dev/boku/.mise.toml

backends:
  aqua
  asdf
  cargo
  core
  go
  npm
  pipx
  spm
  ubi
  vfox

plugins:
  pipx    https://github.com/yozachar/asdf-pipx.git#31db618
  poetry  https://github.com/mise-plugins/mise-poetry.git#431c335
  usage   https://github.com/jdx/mise-usage.git#fe3888a

toolset:
  asdf:[email protected]
  asdf:[email protected]
  asdf:[email protected]
  core:[email protected]

env_vars:
  MISE_SHELL=fish

settings:
  activate_aggressive = false
  all_compile = false
  always_keep_download = false
  always_keep_install = false
  asdf_compat = false
  cache_prune_age = "30d"
  ci = false
  color = true
  debug = false
  disable_backends = []
  disable_default_registry = false
  disable_hints = []
  disable_tools = []
  exec_auto_install = true
  experimental = true
  fetch_remote_versions_cache = "1h"
  fetch_remote_versions_timeout = "10s"
  go_default_packages_file = "~/.default-go-packages"
  go_download_mirror = "https://dl.google.com/go"
  go_repo = "https://github.com/golang/go"
  go_set_gopath = false
  go_set_goroot = true
  go_skip_checksum = false
  http_timeout = "30s"
  ignored_config_paths = []
  jobs = 4
  legacy_version_file = true
  legacy_version_file_disable_tools = []
  libgit2 = true
  lockfile = true
  log_level = "info"
  not_found_auto_install = true
  paranoid = false
  pin = false
  plugin_autoupdate_last_check_duration = "7d"
  quiet = false
  raw = false
  task_run_auto_install = true
  task_skip = []
  trace = false
  trusted_config_paths = []
  unix_default_file_shell_args = ["sh"]
  unix_default_inline_shell_args = [
      "sh",
      "-c",
  ]
  use_file_shell_for_executable_tasks = false
  use_versions_host = true
  verbose = false
  windows_default_file_shell_args = [
      "cmd",
      "/c",
  ]
  windows_default_inline_shell_args = [
      "cmd",
      "/c",
  ]
  windows_executable_extensions = [
      "exe",
      "bat",
      "cmd",
      "com",
      "ps1",
      "vbs",
  ]
  yes = false

  [aqua]
  cosign = true
  slsa = true

  [cargo]
  binstall = true

  [node]

  [npm]
  bun = false

  [pipx]
  uvx = false

  [python]
  default_packages_file = "~/.default-python-packages"
  pyenv_repo = "https://github.com/pyenv/pyenv.git"
  venv_auto_create = false
  venv_stdlib = false

  [ruby]
  default_packages_file = "~/.default-gems"
  ruby_build_repo = "https://github.com/rbenv/ruby-build.git"
  ruby_install = false
  ruby_install_repo = "https://github.com/postmodern/ruby-install.git"

  [status]
  missing_tools = "if_other_versions_installed"
  show_env = false
  show_tools = false

No problems found

Just to clarify, in my case the venv in both directories are created by mise:

Paul_Glushak@Paul ~/d/boku (main)> catp .mise.toml
[tools]
python = "latest"

[env]
_.python.venv = { path = ".venv", create = true }
Paul_Glushak@Paul ~/d/boku (main)> catp ~/dev/forter-dev-cli/.mise.toml
[tools]
poetry = {version='latest', pyproject='pyproject.toml'}
python = '3.10'

[env]
_.python.venv = { path = '.venv', create = true }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants