-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add a GitHub Actions warning to the uv venv
behavior change
#14669
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
Draft
zanieb
wants to merge
28
commits into
main
Choose a base branch
from
zb/venv-warn-ci
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes #13057 Sets `UV_TOOL_BIN_DIR` to `/usr/local/bin` for all derived images to allow `uv tool install` to work out of the box. Note, when the default image user is overwritten (e.g. `USER <UID>`) by a less privileged one, an alternative writable location would now need to be set by downstream consumers to prevent issues, hence I'm labeling this as a breaking change for 0.8.x release. Relates to astral-sh/uv-docker-example#55 Each image was tested to work with uv tool with `UV_TOOL_BIN_DIR` set to `/usr/local/bin` with the default root user and alternative non-root users to confirm breaking nature of the change.
While reviewing #14107, @oconnor663 pointed out a bug where we allow `uv python pin --rm` to delete the global pin without the `--global` flag. I think that shouldn't be allowed? I'm not 100% certain though.
Right now, `--python-platform linux` to defaults to `manylinux_2_17`. Defaulting to `manylinux_2_17` causes some problems for users, since it means we can't use (e.g.) `manylinux_2_28` wheels, and end up having to build from source. cibuildwheel made `manylinux_2_28` their default in pypa/cibuildwheel#1988, and there's a lot of discussion in pypa/cibuildwheel#1772 and pypa/cibuildwheel#2047. In short, the `manylinux_2014` image is EOL, and the vast majority of consumers now run at least glibc 2.28 (https://mayeut.github.io/manylinux-timeline/):  Note that this only changes the _default_. Users can still compile against `manylinux_2_17` by specifying it.
If `--workspace` is provided, we add all paths as workspace members. If `--no-workspace` is provided, we add all paths as direct path dependencies. If neither is provided, then we add any paths that are under the workspace root as workspace members, and the rest as direct path dependencies. Closes #14524.
If a user specifies `-e /path/to/dir` and `/path/to/dir` in a `uv pip install` command, we want the editable to "win" (rather than erroring due to conflicting URLs). Unfortunately, this behavior meant that when you requested a package as editable and non-editable in conflicting groups, the editable version was _always_ used. This PR modifies the requisite types to use `Option<bool>` rather than `bool` for the `editable` field, so we can determine whether a requirement was explicitly requested as editable, explicitly requested as non-editable, or not specified (as in the case of `/path/to/dir` in a `requirements.txt` file). In the latter case, we allow editables to override the "unspecified" requirement. If a project includes a path dependency twice, once with `editable = true` and once without any `editable` annotation, those are now considered conflicting URLs, and lead to an error, so I've marked this change as breaking. Closes #14139.
We weren't following our usual "destructure all the options" pattern in this function, and several "this isn't actually read from uv.toml" fields slipped through the cracks over time since folks forgot it existed. Fixes part of #14308, although we could still try to make the warning in FilesystemOptions more accurate? You could argue this is a breaking change, but I think it ultimately isn't really, because we were already silently ignoring these fields. Now we properly error.
e.g., this could occur for a socket
f38d5ac
to
108a816
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In an attempt to increase the visibility of this warning in CI users, emit it as a GitHub Actions annotation. This begins establishing general infrastructure for emitting CI provider-specific warnings.
We don't need this for 0.8, it's more important for when this change is breaking.