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.

Sign up for GitHub

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

Jump to bottom

Report marker diagnostics during parsing, rather than evaluation #9338

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Report marker diagnostics during parsing, rather than evaluation #9338

merged 2 commits into from
Nov 26, 2024

Conversation

charliermarsh
Copy link
Member

Summary

I want to move towards a more normalized marker representation within the marker tree, which means that the things we warn against will disappear by the time we get to evaluation. I think it makes more sense to show these warnings when we create the tree, rather than when we evaluate it.

@charliermarsh charliermarsh added the error messages Messaging when something goes wrong label Nov 21, 2024
@charliermarsh charliermarsh force-pushed the charlie/eval branch 2 times, most recently from 55a569d to 5eca3e9 Compare November 23, 2024 00:21
@charliermarsh
Copy link
Member Author

Ah shoot, I accidentally merged #9341 into this branch.

@charliermarsh
Copy link
Member Author

Please review the first commit, but ignore the second.

konstin
konstin approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the much better place

crates/uv-pep508/src/marker/parse.rs
len,
input: cursor.to_string(),
})
.inspect(|value| match value {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ? to remove the inspect

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that better? Then I have to assign and return the value

crates/uv-pep508/src/verbatim_url.rs Outdated
@@ -9,8 +9,6 @@ use std::sync::LazyLock;
use thiserror::Error;
use url::{ParseError, Url};

use uv_fs::{normalize_absolute_path, normalize_url_path};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to comment that, but this one is an explicit import to make exporting and rewriting to https://github.com/konstin/pep508_rs/blob/d155787317425e8ab897beef55a947e8ef28f9d8/src/verbatim_url.rs#L12-L13 easier

@charliermarsh
Report marker diagnostics during parsing, rather than evaluation
96b269a
@charliermarsh
Add a lowered representation for markers (#9341)
274b7db 
## Summary

This PR introduces a set of parallel structs to `MarkerValueString`,
`MarkerValueExtra`, and `MarkerValueVersion` that remove various pieces
of information and representations that shouldn't be available in the
marker algebra. To start, I've _just_ removed the invalid extra
component from `MarkerValueExtra` -- there are no other changes to the
representation. So, throughout the marker algebra, we can't represent
and thus don't have to worry about clauses with invalid extras.

The subsequent changes I plan to make are:

1. Removing `python_version`, since we exclusively use
`python_version_full` in the algebra.
2. Removing the deprecated aliases, such that we re-map to the correct
marker value.
3. Consolidating `sys_platform` and `platform_release` (the original
motivation).
@charliermarsh charliermarsh enabled auto-merge (squash) November 26, 2024 14:06
@charliermarsh charliermarsh merged commit 1068630 into main Nov 26, 2024
64 checks passed
@charliermarsh charliermarsh deleted the charlie/eval branch November 26, 2024 14:15
@BrewTestBot BrewTestBot mentioned this pull request Nov 27, 2024
Merged
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Nov 28, 2024
@tmeijn
build(deps): update astral-sh/uv to v0.5.5
576feaf 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.5.4` -> `0.5.5` |

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.5.5`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#055)

[Compare Source](astral-sh/uv@0.5.4...0.5.5)

##### Enhancements

-   Add aliases for build backend requests ([#&#8203;9294](astral-sh/uv#9294))
-   Avoid displaying empty paths ([#&#8203;9312](astral-sh/uv#9312))
-   Allow constraints in `uv tool upgrade` ([#&#8203;9375](astral-sh/uv#9375))
-   Remove conflict between `--no-sync` and `--frozen` in `uv run` ([#&#8203;9400](astral-sh/uv#9400))
-   Respect dependency sources in overrides and constraints ([#&#8203;9455](astral-sh/uv#9455))
-   Show an interpreter-focused message for `--target` and `--prefix` ([#&#8203;9373](astral-sh/uv#9373))
-   Add `--no-extra` flag and setting ([#&#8203;9387](astral-sh/uv#9387))
-   Add `uv export --prune` ([#&#8203;9389](astral-sh/uv#9389))
-   Add dedicated error message for musl install attempts ([#&#8203;9430](astral-sh/uv#9430))
-   Add various grammar changes to conflict error messages ([#&#8203;9369](astral-sh/uv#9369))
-   Annotate default groups in conflict error messages ([#&#8203;9368](astral-sh/uv#9368))
-   Report marker diagnostics during parsing, rather than evaluation ([#&#8203;9338](astral-sh/uv#9338))
-   Use consistent formatting for build system errors ([#&#8203;9340](astral-sh/uv#9340))
-   Use rich diagnostics for build failures ([#&#8203;9335](astral-sh/uv#9335))

##### Preview features

-   Improve build backend excludes ([#&#8203;9281](astral-sh/uv#9281))
-   Include PEP 639 `license-files` metadata during `uv publish` ([#&#8203;9442](astral-sh/uv#9442))

##### Performance

-   Initialize rayon lazily ([#&#8203;9435](astral-sh/uv#9435))
-   Migrate to PubGrub's arena for package names ([#&#8203;9448](astral-sh/uv#9448))

##### Bug fixes

-   Allow dependency groups to include the containing package ([#&#8203;9385](astral-sh/uv#9385))
-   Allow syncing to empty virtual environment directories ([#&#8203;9427](astral-sh/uv#9427))
-   Allow system Python discovery with `--target` and `--prefix` ([#&#8203;9371](astral-sh/uv#9371))
-   Don't warn when `--output-file` is empty ([#&#8203;9417](astral-sh/uv#9417))
-   Fix Python interpreter discovery on non-glibc hosts ([#&#8203;9005](astral-sh/uv#9005))
-   Fix `tool.uv.dependency-metadata.[].version` schema ([#&#8203;9468](astral-sh/uv#9468))
-   Only respect preferences across the same indexes ([#&#8203;9302](astral-sh/uv#9302))
-   Re-compile when `--compile` is passed to an install operation ([#&#8203;9378](astral-sh/uv#9378))
-   Remove `--upgrade`, `--no-upgrade`, and `--upgrade-package` from `uv tool upgrade` ([#&#8203;9318](astral-sh/uv#9318))
-   Remove dev dependencies in `--all-groups --no-dev` ([#&#8203;9300](astral-sh/uv#9300))
-   Surface extras and group conflicts in `uv export` ([#&#8203;9365](astral-sh/uv#9365))
-   Treat deprecated aliases as equivalent in marker algebra ([#&#8203;9342](astral-sh/uv#9342))
-   Treat less compatible tags as lower priority in resolver ([#&#8203;9339](astral-sh/uv#9339))

##### Documentation

-   Avoid referencing `scikit-build` (instead of `scikit-build-core`) ([#&#8203;9320](astral-sh/uv#9320))
-   Expand entry points documentation ([#&#8203;9329](astral-sh/uv#9329))
-   Fix example `pyproject.toml` in project concept documentation ([#&#8203;9298](astral-sh/uv#9298))
-   Fix header level of "Conflicting dependencies" page ([#&#8203;9330](astral-sh/uv#9330))
-   Touch-up the extension module guide ([#&#8203;9293](astral-sh/uv#9293))
-   Update the dependencies documentation ([#&#8203;9359](astral-sh/uv#9359))
-   Reference `--no-progress` option in related environment variable ([#&#8203;9357](astral-sh/uv#9357))

</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=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@konstin konstin konstin approved these changes

Assignees
No one assigned
Labels
error messages Messaging when something goes wrong
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@charliermarsh @konstin