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

fix: prepend virtualenv path rather than append #1751

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

kalvinnchau
Copy link
Contributor

prepend virtualenv path rather than append

When using the

_.python.venv = { path = "{{env.MISE_DATA_DIR}}/python/venv", create = true }

Syntax with nested directories, I found that which python would report the most globally available .mise.toml virtualenv python rather than the more locally specific .mise.toml. The VIRTUAL_ENV environment variable would be set correctly however, causing a bit of confusion initially.

background

A minimal example:

❯ tree -a mise-example
mise-example
├── .mise.toml
└── sub
    ├── .mise.toml
    └── nest
        └── .mise.toml
# mise-example
[tools]
python = "3.11"

[env]
_.python.venv = { path = "{{env.MISE_DATA_DIR}}/python/venv/example", create = true }

# mise-example.sub
[tools]
python = "3.9"

[env]
_.python.venv = { path = "{{env.MISE_DATA_DIR}}/python/venv/sub", create = true }

# mise-example.sub.nest
[tools]
python = "3.12"

[env]
_.python.venv = { path = "{{env.MISE_DATA_DIR}}/python/venv/nest", create = true }

And the output of PATH from the mise-example/sub/nest directory:

❯ echo $PATH

/Users/knc/.local/share/mise/python/venv/example/bin:/Users/knc/.local/share/mise/python/venv/sub/bin:/Users/knc/.local/share/mise/python/venv/nest/bin:...

[DEBUG] ARGS: /Users/knc/.local/bin/mise hook-env -s zsh
[DEBUG] Config {
    Config Files: [
        "~/stage/mise-example/sub/nest/.mise.toml",
        "~/stage/mise-example/sub/.mise.toml",
        "~/stage/mise-example/.mise.toml",
        "~/.config/mise/config.toml",
    ],
    Env: {
        "MISE_DATA_DIR": "/Users/knc/.local/share/mise",
        "MISE_POETRY_AUTO_INSTALL": "1",
        "VIRTUAL_ENV": "/Users/knc/.local/share/mise/python/venv/nest",
    },
    Path Dirs: [
        "/Users/knc/.local/share/mise/python/venv/example/bin",
        "/Users/knc/.local/share/mise/python/venv/sub/bin",
        "/Users/knc/.local/share/mise/python/venv/nest/bin",
    ],
}

changes

This change inserts the found virtualenv to the front of the env_paths instead, resulting in the Path Dirs in the DEBUG output to be flipped and showing the correct python version when running which python:

[DEBUG] Config {
...
    Path Dirs: [
        "/Users/kchau/.local/share/mise/python/venv/nest/bin",
        "/Users/kchau/.local/share/mise/python/venv/sub/bin",
        "/Users/kchau/.local/share/mise/python/venv/example/bin",
    ],
}

I added an e2e test to create a subdirectory to verify this, as well as a unit test. I verified the new e2e test fails on the current main without the change:

mise creating venv at: ~/.mise/e2e/venv
mise all runtimes are installed
mise creating venv at: ~/.mise/e2e/subvenv
Expected '/root/.mise/e2e/subvenv/bin/python' but got '/root/.mise/e2e/venv/bin/python'
::error file=e2e/test_python::E2E Test Failed (code: 1)

I'm not sure I completely like the unit test having to rely on the cwd and / directories to pass - but I didn't see any test scaffolding / mocks to ensure a Path exists. Let me know how you'd want this to change if anything.

prepend all virtualenv PATHs so the most nested directory is at the
front of the vector.

otherwise the most globally configured virtualenv takes precedence

add unit tests and e2e tests to verify this change
@jdx jdx merged commit 5c9e82e into jdx:main Mar 15, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants