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

Automatic Python venv activation doesn't account for Framework builds on macOS #1354

Closed
bryanwweber opened this issue Jan 4, 2024 · 5 comments
Labels

Comments

@bryanwweber
Copy link

Describe the bug
Python on macOS supports a Framework build mode to bundle it as a macOS application. This is needed to access Mac GUI components properly, for example, for matplotlib plots to use Mac GUI windows. In a Framework build, the Python executable is located at ~/.local/share/mise/installs/python/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/bin/python. However, the virtualenv function of the Python core plugin assumes that the symlink binary is at ~/.local/share/mise/installs/python/3.10.13/bin/python and warns that the virtualenv is incorrect. Note that this is true for any recent Python, I happen to be using 3.10.

To Reproduce

env PYTHON_CONFIGURE_OPTS="--enable-framework" mise install -v python@3.10
mise use python@3.10

edit .mise.toml to use the virtualenv setting, then allow mise to create the virtualenv and you get the output:

$ cd project
mise setting up virtualenv at: ~/project/.venv
mise failed to get virtualenv: expected venv ~/project/.venv/bin/python to point to ~/.local/share/mise/installs/python/3.10/bin/python.
Try deleting the venv at ~/project/.venv.

Expected behavior

No warning is produced and the virtualenv is activated.

mise doctor output

mise version:
  2024.1.4 macos-arm64 (bed9669 2024-01-04)

build:
  Target: aarch64-apple-darwin
  Features: DEFAULT, NATIVE_TLS, OPENSSL
  Built: Thu, 4 Jan 2024 01:52:10 +0000
  Rust Version: rustc 1.75.0 (82e1608df 2023-12-21)
  Profile: release

shell:
  /bin/zsh
  zsh 5.9 (x86_64-apple-darwin23.0)

mise data directory:
  ~/.local/share/mise

mise environment variables:
  MISE_SHELL=zsh

settings:
  {
    "experimental": true,
    "color": true,
    "always_keep_download": false,
    "always_keep_install": false,
    "legacy_version_file": true,
    "legacy_version_file_disable_tools": [],
    "plugin_autoupdate_last_check_duration": "7d",
    "trusted_config_paths": [],
    "log_level": "info",
    "trace": false,
    "debug": false,
    "verbose": false,
    "quiet": false,
    "asdf_compat": false,
    "jobs": 4,
    "shorthands_file": null,
    "disable_default_shorthands": false,
    "disable_tools": [],
    "raw": false,
    "yes": false,
    "task_output": null,
    "not_found_auto_install": true,
    "ci": false,
    "env_file": null,
    "cd": null
  }

config files:
  ~/.config/mise/config.toml
  ~/project/.mise.toml

plugins:
  bun      (core)
  deno     (core)
  erlang   (core)
  go       (core)
  java     (core)
  node     (core)
  pdm      https://github.com/1oglop1/asdf-pdm#e83033a
  python   (core)
  ruby     (core)

toolset:
  python@3.10.13, go@1.21.5, node@20, pdm@latest

No problems found

Additional context

The problematic line is here: https://github.com/jdx/mise/blob/581b6e8aa56476d8d184c2cae2bd7657c8690143/src/plugins/core/python.rs#L153C39-L153C39

@bryanwweber bryanwweber added the bug label Jan 4, 2024
@jdx
Copy link
Owner

jdx commented Jan 4, 2024

potential "solution" here: #1357

@bryanwweber
Copy link
Author

Thanks for that discussion! I voted for the third option (add a setting, default off).

That said, I don't think that's quite the same here, because I don't think this is a problem with venv creation. Since you're planning to retain the automatic activation feature, I think it'd still be useful to keep the "is this a mise-managed Python" check, which is where the problem really is in this case. You'd get the same warning with the code as-is if a user just ran python -m venv .venv with a framework build of Python.

There are two fixes to this logic that I can think of quickly. The first is that I think it'd be enough to check that the symlinked Python in the venv folder shares a common root (up to the first version number). For example, ~/.local/share/mise/installs/python/3.10.13 is common in the framework build and the unix-y build. Alternatively, it could check for these two locations specifically (bin/python and Libraries/Frameworks/.../bin/python), since those should be fairly stable at this point in Python's lifetime.

@jdx
Copy link
Owner

jdx commented Jan 4, 2024

is this a mise-managed Python

Not really, a use-case I'm trying to add is using the system python. Though I could detect if it should be mise-managed and do the check in that case.

I'll have to think about it. What you said makes sense as a solution. Part of my thinking here is that this logic has been a very common source of problems and since I'm not a python dev I'm not sure if it'll ever be great. "Doing less" might be the smarter way to go here but I'm still trying to decide.

@bryanwweber
Copy link
Author

bryanwweber commented Jan 4, 2024

is this a mise-managed Python

Not really, a use-case I'm trying to add is using the system python. Though I could detect if it should be mise-managed and do the check in that case.

Edit: I just reread your post and noticed the comma between "really" and "a", which totally inverts the meaning of the sentence 😅 Anyways, the last paragraph below is still relevant!

Sorry, I didn't mean to suggest adding support for the system Python 😄 Since this is just a check to make sure users are getting what they expected, I think it's reasonable to assume that the presence of .mise.toml configured to auto-activate a virtual environment means that the user wants a mise-managed Python. That takes at least 4 intentional steps, I think... (install mise, allow experimental settings, have .mise.toml, configure the venv setting; although the latter two could be pulled in from a git clone). If I as a user accidentally created a venv with the system python, I think it'd be useful to be warned about that (maybe mise wasn't activated when I wrote python -m venv, etc.).

Of course, someone will come along and decide they're using mise just for the auto-activation feature but with Python coming from somewhere else, and they want to turn off the warning...

Anyhow, I fully support whatever you want to do here. I totally get not being a Python dev, and the conventions around virtual environments (and conda and poetry and...) are just bananas and always changing. Deleting this whole feature definitely makes sense to me from a maintenance standpoint!

@jdx
Copy link
Owner

jdx commented Jan 7, 2024

this check is now gone and venv creation is behind a setting so this should be resolved

@jdx jdx closed this as completed Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants