-
Notifications
You must be signed in to change notification settings - Fork 682
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
Avoid displaying "failed to download" on build failures for local source distributions #6075
Conversation
…rce distributions
crates/uv/tests/pip_install.rs
Outdated
error: Failed to build: `project @ file://[TEMP_DIR]/path_dep` | ||
Caused by: Failed to build: `project @ file://[TEMP_DIR]/path_dep` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we have a duplicate "Failed to build" here because of uv-distribution::Error::Build
— I'm exploring a way to avoid that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 6cf1a94
It becomes a little ambiguous where we failed for some errors, but I think it's a net improvement.
6cf1a94
to
381eb40
Compare
error: Failed to download and build: `anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz` | ||
Caused by: Failed to build: `anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz` | ||
Caused by: Build backend failed to determine metadata through `prepare_metadata_for_build_wheel` with exit status: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is arguably more ambiguous now, but I think dropping the duplicate package URL is nice. I'm not sure how to improve this in the general case? I think we'd need to implement display for Error::FetchAndBuild
to include a bit more logic which seems wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have the URL in the line just above? It looks like it was repeated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's an improvement not to repeat the URL. The ambiguity here is that it's not clear if the following error, e.g., "Build backend failed to determine metadata", is a build or download error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Significantly better.
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.2.36` -> `0.2.37` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.2.37`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0237) [Compare Source](astral-sh/uv@0.2.36...0.2.37) ##### Performance - Avoid cloning requirement for unchanged markers ([#​6116](astral-sh/uv#6116)) ##### Bug fixes - Fix loading of cached metadata for Git distributions with subdirectories ([#​6094](astral-sh/uv#6094)) ##### Error messages - Add env var to `--link-mode=copy` warning ([#​6103](astral-sh/uv#6103)) - Avoid displaying "failed to download" on build failures for local source distributions ([#​6075](astral-sh/uv#6075)) - Improve display of available package ranges ([#​6118](astral-sh/uv#6118)) - Use "your requirements" consistently in resolver error messages ([#​6113](astral-sh/uv#6113)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Especially with workspace members (e.g., this new test case), I find it very confusing that we say we failed to download these distributions.