The marker display code assumes that all versions are normalized, in
that all trailing zeroes are stripped. This is not the case for
tilde-equals and equals-star versions, where the trailing zeroes (before
the `.*`) are semantically relevant. This would cause path
dependent-behavior where we would get a different marker string
depending on whether a version with or without a trailing zero was added
to the cache first.
To handle both equals-star and tilde-equals when converting
`python_version` to `python_full_version` markers, we have to merge the
version normalization (i.e. trimming the trailing zeroes) and the
conversion both to `python_full_version` and to `Ranges`, while special
casing equals-star and tilde-equals.
To avoid churn in lockfiles, we only trim in the conversion to `Ranges`
for markers, but keep using untrimmed versions for requires-python.
(Note that this behavior is technically also path dependent, as versions
with and without trailing zeroes have the same Hash and Eq. E.q.,
`requires-python == ">= 3.10.0"` and `requires-python == ">= 3.10"` in
the same workspace could lead to either value in `uv.lock`, and which
one it is could change if we make unrelated (performance) changes.
Always trimming however definitely changes lockfiles, a churn I wouldn't
do outside another breaking or lockfile-changing change.) Nevertheless,
there is a change for users who have `requires-python = "~= 3.12.0"` in
their `pyproject.toml`, as this now hits the correct normalization path.
Fixes#14231Fixes#14270
This PR implements a few review follow-ups from #13560. In particular,
it
* Makes `DisplaySafeUrlRef` implement `Copy` so that it can be passed by
value.
* Updates `to_string_with_credentials` methods with
`displayable_with_credentials`, returning an `impl Display` instead of
`String` for greater flexibility.
* Updates the `DisplaySafeUrl` and `DisplaySafeUrlRef` `Debug`
implementations to match the underlying `Url` `Debug` implementation
(with the exception that credentials are masked).
* Replaces an unnecessary `DisplaySafeUrl::from(Url::from_file_path`
with `DisplaySafeUrl::from_file_path`
Prior to this PR, there were numerous places where uv would leak
credentials in logs. We had a way to mask credentials by calling methods
or a recently-added `redact_url` function, but this was not secure by
default. There were a number of other types (like `GitUrl`) that would
leak credentials on display.
This PR adds a `DisplaySafeUrl` newtype to prevent leaking credentials
when logging by default. It takes a maximalist approach, replacing the
use of `Url` almost everywhere. This includes when first parsing config
files, when storing URLs in types like `GitUrl`, and also when storing
URLs in types that in practice will never contain credentials (like
`DirectorySourceUrl`). The idea is to make it easy for developers to do
the right thing and for the compiler to support this (and to minimize
ever having to manually convert back and forth). Displaying credentials
now requires an active step. Note that despite this maximalist approach,
the use of the newtype should be zero cost.
One conspicuous place this PR does not use `DisplaySafeUrl` is in the
`uv-auth` crate. That would require new clones since there are calls to
`request.url()` that return a `&Url`. One option would have been to make
`DisplaySafeUrl` wrap a `Cow`, but this would lead to lifetime
annotations all over the codebase. I've created a separate PR based on
this one (#13576) that updates `uv-auth` to use `DisplaySafeUrl` with
one new clone. We can discuss the tradeoffs there.
Most of this PR just replaces `Url` with `DisplaySafeUrl`. The core is
`uv_redacted/lib.rs`, where the newtype is implemented. To make it
easier to review the rest, here are some points of note:
* `DisplaySafeUrl` has a `Display` implementation that masks
credentials. Currently, it will still display the username when there is
both a username and password. If we think is the wrong choice, it can
now be changed in one place.
* `DisplaySafeUrl` has a `remove_credentials()` method and also a
`.to_string_with_credentials()` method. This allows us to use it in a
variety of scenarios.
* `IndexUrl::redacted()` was renamed to
`IndexUrl::removed_credentials()` to make it clearer that we are not
masking.
* We convert from a `DisplaySafeUrl` to a `Url` when calling `reqwest`
methods like `.get()` and `.head()`.
* We convert from a `DisplaySafeUrl` to a `Url` when creating a
`uv_auth::Index`. That is because, as mentioned above, I will be
updating the `uv_auth` crate to use this newtype in a separate PR.
* A number of tests (e.g., in `pip_install.rs`) that formerly used
filters to mask tokens in the test output no longer need those filters
since tokens in URLs are now masked automatically.
* The one place we are still knowingly writing credentials to
`pyproject.toml` is when a URL with credentials is passed to `uv add`
with `--raw`. Since displaying credentials is no longer automatic, I
have added a `to_string_with_credentials()` method to the `Pep508Url`
trait. This is used when `--raw` is passed. Adding it to that trait is a
bit weird, but it's the simplest way to achieve the goal. I'm open to
suggestions on how to improve this, but note that because of the way
we're using generic bounds, it's not as simple as just creating a
separate trait for that method.
Rustfmt introduces a lot of formatting changes in the 2024 edition. To
not break everything all at once, we split out the set of formatting
changes compatible with both the 2021 and 2024 edition by first
formatting with the 2024 style, and then again with the currently used
2021 style.
Notable changes are the formatting of derive macro attributes and lines
with overly long strings and adding trailing semicolons after statements
consistently.
## Summary
This crate is for standards-compliant types, but this is explicitly a
type that's custom to uv. It's also strange because we kind of want to
reference `IndexUrl` on the registry type, but that's in a crate that
_depends_ on `uv-pypi-types`, which to me is a sign that this is off.
## Summary
A revival of an old idea (#9344) that I have slightly more confidence in
now. I abandoned this idea because (1) it couldn't capture that, e.g.,
`platform_system == 'Windows' and sys_platform == 'foo'` (or some other
unknown value) are disjoint, and (2) I thought that Android returned
`"android"` for one of `sys_platform` or `platform_system`, which
would've made this logic incorrect.
However, it looks like Android... doesn't do that? And the values here
are almost always in a small, known set. So in the end, the tradeoffs
here actually seem pretty good.
Vis-a-vis our current solution, this can (e.g.) _simplify out_
expressions like `sys_platform == 'win32' or platform_system ==
'Windows'`.
## Summary
After #9524, I noticed two other dependencies were misaligned.
Since the previous PR has been merged, I was thinking I could submit
those two misses.
Of course, open to any comments/decline!
Thanks!! 🙂
## Test Plan
All units tests are still passing on my side. Let's see with the
pull-request CI again 😄
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
While working on potential bug fixes with temporary files on Windows (I
think I am currently ecountering the same issue as #2810)
I noticed that sub-workspaces were not all having the same `tempfile`
version. And they were not relying on the cargo root project dependency.
I don't know at all if it was done on purpose or not.
(I also wanted to override the root dependency with a local source but
it was not possible due to sub-workspaces not relying on the same).
The root lockfile already pinned to the `3.14.0`. Some sub-workspaces
were depending on the `3.12.0`, some others on the `3.14.0`. So I
updated the root `Cargo.toml` to the `3.14.0`.
Feel free to decline if it was done on purpose! No worries at all
🙂
Thanks!
<!-- What's the purpose of the change? What does it do, and why? -->
## Test Plan
All units tests are still passing on my side. Let's see with the
pull-request CI 😄
## Summary
Like pip, we now allow the semicolon to directly proceed the URL (but
require that it's either preceded or followed by a space):
```
# OK
./test.whl; sys_platform == 'darwin'
# OK
./test.whl ;sys_platform == 'darwin'
# Error
./test.whl;sys_platform == 'darwin'
```
Closes https://github.com/astral-sh/uv/issues/8831.
As per
https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html
Before that, there were 91 separate integration tests binary.
(As discussed on Discord — I've done the `uv` crate, there's still a few
more commits coming before this is mergeable, and I want to see how it
performs in CI and locally).
