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

installation of editable package don't work when adding python markers #3604

Closed
ImpSy opened this issue May 15, 2024 · 6 comments · Fixed by #3622
Closed

installation of editable package don't work when adding python markers #3604

ImpSy opened this issue May 15, 2024 · 6 comments · Fixed by #3622
Assignees
Labels
compatibility Compatibility with a specification or another tool enhancement New feature or improvement to existing functionality

Comments

@ImpSy
Copy link

ImpSy commented May 15, 2024

UV details

version: 0.1.44
command uv pip sync requirements.txt

What

uv currently not work while installing an editable package with python markers

How to reproduce

for the following requirements.txt

-e file:///Users/sebastienmaintrot/libs/aws-client ; python_version >= "3.9" and python_version < "3.11"

uv fails with the following error

error: Failed to build editables
  Caused by: Source distribution not found at: /Users/sebastienmaintrot/Spot/CP/bigdata-python-services/libs/aws-client ; python_version >= "3.9" and python_version < "3.11"
@charliermarsh
Copy link
Member

I don't think pip respects these, as far as I can tell they're just ignored?

For example, installing this file always installs the editable:

-e ./scripts/packages/black_editable ; python_version < "3.9"

@ImpSy
Copy link
Author

ImpSy commented May 15, 2024

I think you're right, the marker used will be the one define in the editable package if any
But in my case, I'm using poetry to generate a cross platform lock and the export to requirements seems to always add them

@charliermarsh
Copy link
Member

I agree we should do something different here (bare minimum: a real error message), but undecided on what. I'm tempted to say we should actually just support them.

See: pypa/pip#8581.

@charliermarsh charliermarsh added enhancement New feature or improvement to existing functionality compatibility Compatibility with a specification or another tool labels May 15, 2024
@charliermarsh
Copy link
Member

I think my preference would be to actually support and respect these. Second choice would be to parse them properly, but ignore them and warn.

@charliermarsh
Copy link
Member

Fixed in #3622.

charliermarsh added a commit that referenced this issue May 16, 2024
## Summary

If a user includes markers after an editable, we now ignore them (rather
than including them in the parsed URL). This matches pip's behavior. In
the future, we could further improve by respecting them, but that
_would_ be a deviation from pip.

For example, given:

```
-e ./scripts/packages/black_editable ; python_version >= "3.9" and python_ver
```

We now split at the first whitespace (just before the `;`), parse
everything before, and throw out everything after.

This logic also extends to extras. So given:

```
-e ./scripts/packages/black_editable[dev, colorama]
```

We'll now parse this as the URL
`./scripts/packages/black_editable[dev,`, and throw out ` colorama]`.
Instead, you need to do:

```
-e ./scripts/packages/black_editable[dev,colorama]
```

(I.e., remove the space.)

This _also_ matches pip's behavior. I could "fix" this but I'm unsure if
I should -- it means requirements files will be parseable by uv that
won't work with pip. Open to input. My gut reaction is that we _should_
properly support `-e ./scripts/packages/black_editable[dev, colorama]`
even if pip would reject it, but `requirements.txt` is
implementation-defined so it'd be a "deviation".

Closes #3604.
@charliermarsh
Copy link
Member

Out now in v0.1.45.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Compatibility with a specification or another tool enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants