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

uv wrongly detects PEP723 style magic comments in strings #12303

Open
samuelcolvin opened this issue Mar 18, 2025 · 15 comments
Open

uv wrongly detects PEP723 style magic comments in strings #12303

samuelcolvin opened this issue Mar 18, 2025 · 15 comments
Labels
bug Something isn't working external The problem is with another package or dependency (not uv)

Comments

@samuelcolvin
Copy link

Summary

code:

print("""
# /// script
# dependencies = ["pydantic", "email-validator"]
# ///
import pydantic

class Model(pydantic.BaseModel):
    email: pydantic.EmailStr

print(Model(email='[email protected]'))
""")

Running this prints:

error: failed to create directory `/Users/samuel/Library/Caches/uv/environments-v2/check-da89a400ed637ca7`: Permission denied (os error 13)

Platform

MacOS 15 ARM

Version

uv 0.6.6 (c1a0bb8 2025-03-12)

Python version

Python 3.12.3

@samuelcolvin samuelcolvin added the bug Something isn't working label Mar 18, 2025
@zanieb
Copy link
Member

zanieb commented Mar 19, 2025

Alas.

Fwiw, this matches the regex in the specification https://regex101.com/r/LNoSNF/1 — though I'm sure this isn't the intent.

I presume we can use a heuristic to avoid this? I don't want to bring in a parser.

pipx has the same behavior

❯ uvx pipx run -v example.py
pipx >(setup:1110): pipx version is 1.7.1
pipx >(setup:1111): Default python interpreter is '/Users/zb/.local/share/uv/python/cpython-3.13.0-macos-aarch64-none/bin/python3.13'
pipx >(_remove_all_expired_venvs:315): Removing expired venv /Users/zb/.local/pipx/.cache/CACHEDIR.TAG
pipx >(create_venv:164): Creating virtual environment
creating virtual environment...
pipx >(run_subprocess:175): running /Users/zb/.local/share/uv/python/cpython-3.13.0-macos-aarch64-none/bin/python3.13 -m venv --without-pip /Users/zb/.local/pipx/.cache/ea304b8314d56fb
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(run_subprocess:175): running /Users/zb/.local/pipx/.cache/ea304b8314d56fb/bin/python -c import sysconfig; print(sysconfig.get_path('purelib'))
pipx >(run_subprocess:175): running /Users/zb/.local/pipx/shared/bin/python -c import sysconfig; print(sysconfig.get_path('purelib'))
pipx >(run_subprocess:175): running /Users/zb/.local/pipx/.cache/ea304b8314d56fb/bin/python --version
pipx >(install_unmanaged_packages:292): Installing pydantic, email-validator
installing pydantic, email-validator...
pipx >(run_subprocess:175): running /Users/zb/.local/pipx/.cache/ea304b8314d56fb/bin/python -m pip --no-input install pydantic email-validator
pipx >(exec_app:375): exec_app: /Users/zb/.local/pipx/.cache/ea304b8314d56fb/bin/python example.py

# /// script
# dependencies = ["pydantic", "email-validator"]
# ///
import pydantic

class Model(pydantic.BaseModel):
    email: pydantic.EmailStr

print(Model(email='[email protected]'))

cc @ofek

@samuelcolvin
Copy link
Author

Presumably the magic comment should appear at the start of the file, perhaps allowing for blank lines? If so, that should be pretty easy to detect.

I agree the original regex here would match on this, and I'm pretty sure my code in mcp-run-python here would do the same (that's what I was testing when I found this).

@zanieb
Copy link
Member

zanieb commented Mar 19, 2025

I think it needs to be able to appear after a shebang, which could be fairly complicated (e.g., #12193 (comment) — which is not quite a perfect example but does capture the possible complexity)

@charliermarsh
Copy link
Member

I need to look back at the comments, but I seem to recall that this came up when the PEP was being discussed… I’ll look when I’m back at my computer.

@ofek
Copy link
Contributor

ofek commented Mar 19, 2025

The magic comment is allowed anywhere in the file because an anticipated use case is modification of the comment such as storing the contents of a lock file/dependency resolution. I don't think anything can be done here. In order to get your example working start immediately after the triple quotes i.e. don't have a leading new line character.

jsirois added a commit to jsirois/pex that referenced this issue Mar 19, 2025
This pre-emptively fixes the issue reported here:
  astral-sh/uv#12303
@ofek
Copy link
Contributor

ofek commented Mar 19, 2025

I very much dislike solutions like the one that was opened in response to this issue: pex-tool/pex#2722

There's nothing preventing such liberties but I think we're in a bad situation if/when providers start getting requests to support parsing Python syntax.

@samuelcolvin What's your use case for running scripts that happen to embed magic comments in multiline literals?

@shauneccles
Copy link

This seems like a "don't shoot yourself in the face" problem.

I'd advocate that this is noted as an issue with the spec and if the spec changes to fix it then great.

@samuelcolvin
Copy link
Author

@samuelcolvin What's your use case for running scripts that happen to embed magic comments in multiline literals?

I don't have a specific use case, as mentioned above I discovered this while debugging our MCP server for writing python that uses PEP 723 style comments for dependencies, so I had some code in a string within a python script.

I was reporting it to be a good citizen, not particularly because I need a fix.

jsirois added a commit to pex-tool/pex that referenced this issue Mar 19, 2025
This pre-emptively fixes the issue reported here:
  astral-sh/uv#12303
@charliermarsh
Copy link
Member

Personally, I think we should probably avoid changing anything here. The spec was designed with the intent of supporting non-Python consumers, and deviating from the spec puts pressure on other tools to support those deviations.

@notatallshaw
Copy link
Collaborator

Accepting the magic comment in any location in the file matches the spec and is obvious to the user what happened after the fact, if sometimes surprising to the user before the fact.

Therefore if a tool rejects a magic comment based on the Python syntax, or some other measure, I would prefer it do it non-silently, give a big warning that a magic comment was detected but rejected due to reasons.

As I recall, there was a long discussion about not having to parse Python, and the construction of this comment was made such that it was unlikely to accidentally result in false positives, but obviously giving an example of the spec itself would be one of those cases impossible to avoid.

@ofek
Copy link
Contributor

ofek commented Mar 20, 2025

Germane to the discussion: is there a plan to embed the lock file within scripts rather than generating a file beside the script? It would be nice to have portable reproducibility (without relying on that resolution date limit option).

@notatallshaw
Copy link
Collaborator

is there a plan to embed the lock file within scripts

I think that's #11064

@zanieb
Copy link
Member

zanieb commented Mar 20, 2025

Worth noting that I see some hiccups with the specification for this purpose too #6318 (comment)

@zanieb
Copy link
Member

zanieb commented Mar 20, 2025

If PEP 751 lands maybe we can have a # /// lock block :)

@ncoghlan
Copy link

I agree with the folks suggesting that nothing needs to change in uv here - the naive parsing in the spec is intentional, not accidental, and Python's string quoting is flexible enough that it isn't necessary to format strings in a way that triggers a misfire when generating code that happens to include PEP 723 comments.

I filed pypa/packaging.python.org#1835 to cover adding a note about this to the "Recommendations" section on the PEP 723 page (I'm open to bikeshedding on what we think the nicest "escaping" mechanism to recommend would be)

@zanieb zanieb added the external The problem is with another package or dependency (not uv) label Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external The problem is with another package or dependency (not uv)
Projects
None yet
Development

No branches or pull requests

7 participants