Skip to content

Normalize index URLs to remove trailing slash #14245

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
Merged
jtfmumm merged 2 commits into from
Jun 27, 2025
Merged

Normalize index URLs to remove trailing slash #14245

jtfmumm merged 2 commits into from
Jun 27, 2025

Conversation

@jtfmumm
Copy link
Contributor

@jtfmumm jtfmumm commented Jun 24, 2025 •
edited
Loading

This PR updates IndexUrl parsing to normalize non-file URLs by removing trailing slashes. It also normalizes registry source URLs when using them to validate the lockfile.

Prior to this change, when writing an index URL to the lockfile, uv would use a trailing slash if present in the provided URL and no trailing slash otherwise. This can cause surprising behavior. For example, uv lock --locked will fail when a package is added with an --index value without a trailing slash and then uv lock --locked is run with a pyproject.toml version of the index URL that contains a trailing slash. This PR fixes this and adds a test for the scenario.

It might be safe to normalize file URLs in the same way, but since slashes have a well-defined meaning in the context of files and directories, I chose not to normalize them here.

Closes #13707.

@jtfmumm jtfmumm added the bug Something isn't working label Jun 24, 2025
@jtfmumm jtfmumm force-pushed the jtfm/normalize-index-url-trailing-slash branch from 644f360 to 89f7c18 Compare June 24, 2025 16:18
@jtfmumm jtfmumm force-pushed the jtfm/normalize-index-url-trailing-slash branch from 89f7c18 to 0458081 Compare June 24, 2025 16:20
jtfmumm
jtfmumm commented Jun 24, 2025
View reviewed changes
crates/uv/tests/it/lock.rs
/// Add a package with an `--index` URL with no trailing slash. Run `uv lock --locked`
/// with a `pyproject.toml` with that same URL but with a trailing slash.
#[test]
fn lock_with_inconsistent_trailing_slash() -> Result<()> {
Copy link
Contributor Author

@jtfmumm jtfmumm Jun 24, 2025

Choose a reason for hiding this comment

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

Here's the new test

@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 24, 2025 16:25 — with GitHub Actions Inactive
@konstin konstin requested a review from charliermarsh June 24, 2025 18:59
@charliermarsh
Copy link
Member

charliermarsh commented Jun 24, 2025

In the "Is this lockfile up-to-date check?", should we also strip trailing slashes?

@jtfmumm
Copy link
Contributor Author

jtfmumm commented Jun 25, 2025

In the "Is this lockfile up-to-date check?", should we also strip trailing slashes?

Updated and added a test for a pre-existing lockfile with trailing slashes in registry sources.

@jtfmumm jtfmumm force-pushed the jtfm/normalize-index-url-trailing-slash branch from 5db67b0 to 8fcffbb Compare June 25, 2025 08:50
@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 25, 2025 08:57 — with GitHub Actions Inactive
charliermarsh
charliermarsh reviewed Jun 27, 2025
View reviewed changes
crates/uv-distribution-types/src/index_url.rs
Some(_) => {
// Ex) `https://pypi.org/simple`
VerbatimUrl::parse_url(path)?
Some(scheme) => {
Copy link
Member

@charliermarsh charliermarsh Jun 27, 2025

Choose a reason for hiding this comment

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

Could this be Some(Scheme::file), instead of the nested if?

Copy link
Contributor Author

@jtfmumm jtfmumm Jun 27, 2025

Choose a reason for hiding this comment

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

That was what I initially wanted to do, but there numerous other Scheme variants for things like bzr+file

charliermarsh
charliermarsh reviewed Jun 27, 2025
View reviewed changes
crates/uv-distribution-types/src/file.rs
self.as_ref()
.strip_suffix('/')
.map(SmallString::from)
.unwrap_or_else(|| self.0.clone()),
Copy link
Member

@charliermarsh charliermarsh Jun 27, 2025

Choose a reason for hiding this comment

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

I think this and without_fragment could both return Cow? But that can be a separate PR if you want to try it.

Copy link
Contributor Author

@jtfmumm jtfmumm Jun 27, 2025

Choose a reason for hiding this comment

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

Makes sense to me. I'll open as a separate PR

@jtfmumm jtfmumm merged commit 5754f2f into main Jun 27, 2025
87 checks passed
@jtfmumm jtfmumm deleted the jtfm/normalize-index-url-trailing-slash branch June 27, 2025 15:11
@jtfmumm jtfmumm mentioned this pull request Jun 27, 2025
Merged
charliermarsh pushed a commit that referenced this pull request Jun 27, 2025
@jtfmumm
Return Cow from UrlString::with_ methods (#14319)
f892b85 
A minor performance improvement as a follow-up to #14245 (and an
accompanying test).
@BrewTestBot BrewTestBot mentioned this pull request Jun 27, 2025
Merged
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jun 29, 2025
@tmeijn
build(deps): update astral-sh/uv to v0.7.16
7aeb568 
This MR contains the following updates:

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

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

[Compare Source](astral-sh/uv@0.7.15...0.7.16)

##### Python

- Add Python 3.14.0b3

See the
[`python-build-standalone` release notes](https://github.com/astral-sh/python-build-standalone/releases/tag/20250626)
for more details.

##### Enhancements

- Include path or URL when failing to convert in lockfile ([#&#8203;14292](astral-sh/uv#14292))
- Warn when `~=` is used as a Python version specifier without a patch version ([#&#8203;14008](astral-sh/uv#14008))

##### Preview features

- Ensure preview default Python installs are upgradeable ([#&#8203;14261](astral-sh/uv#14261))

##### Performance

- Share workspace cache between lock and sync operations ([#&#8203;14321](astral-sh/uv#14321))

##### Bug fixes

- Allow local indexes to reference remote files ([#&#8203;14294](astral-sh/uv#14294))
- Avoid rendering desugared prefix matches in error messages ([#&#8203;14195](astral-sh/uv#14195))
- Avoid using path URL for workspace Git dependencies in `requirements.txt` ([#&#8203;14288](astral-sh/uv#14288))
- Normalize index URLs to remove trailing slash ([#&#8203;14245](astral-sh/uv#14245))
- Respect URL-encoded credentials in redirect location ([#&#8203;14315](astral-sh/uv#14315))
- Lock the source tree when running setuptools, to protect concurrent builds ([#&#8203;14174](astral-sh/uv#14174))

##### Documentation

- Note that GCP Artifact Registry download URLs must have `/simple` component ([#&#8203;14251](astral-sh/uv#14251))

### [`v0.7.15`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0715)

[Compare Source](astral-sh/uv@0.7.14...0.7.15)

##### Enhancements

- Consistently use `Ordering::Relaxed` for standalone atomic use cases ([#&#8203;14190](astral-sh/uv#14190))
- Warn on ambiguous relative paths for `--index` ([#&#8203;14152](astral-sh/uv#14152))
- Skip GitHub fast path when rate-limited ([#&#8203;13033](astral-sh/uv#13033))
- Preserve newlines in `schema.json` descriptions ([#&#8203;13693](astral-sh/uv#13693))

##### Bug fixes

- Add check for using minor version link when creating a venv on Windows ([#&#8203;14252](astral-sh/uv#14252))
- Strip query parameters when parsing source URL ([#&#8203;14224](astral-sh/uv#14224))

##### Documentation

- Add a link to PyPI FAQ to clarify what per-project token is ([#&#8203;14242](astral-sh/uv#14242))

##### Preview features

- Allow symlinks in the build backend ([#&#8203;14212](astral-sh/uv#14212))

</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:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
@charliermarsh charliermarsh mentioned this pull request Jun 29, 2025
Merged
charliermarsh added a commit that referenced this pull request Jun 29, 2025
@charliermarsh
Drop trailing slashes when converting index URL from URL (#14346)
734b228 
## Summary

In #14245, we started normalizing index URLs by dropping the trailing
slash in the lockfile. We added tests to ensure that this didn't cause
existing lockfiles to be invalidated, but we missed one of the
constructors (specifically, the path that's used with
`tool.uv.sources`).
@jtfmumm jtfmumm mentioned this pull request Jul 4, 2025
Closed
zanieb added a commit that referenced this pull request Jul 8, 2025
@zanieb
Revert "Normalize index URLs to remove trailing slash (#14245)"
da46fa8 
This reverts commit 5754f2f.
@zanieb zanieb mentioned this pull request Jul 8, 2025
Merged
zanieb added a commit that referenced this pull request Jul 9, 2025
@zanieb
Revert normalization of trailing slashes on index URLs (#14511)
2709c44 
Reverts:

- #14349
- #14346
- #14245

Retains the test cases. Includes a `find-links` test case.

Supersedes

- #14387
- #14503

We originally got a report at
#13707 that inclusion of a
trailing slash on an index URL was causing lockfile churn despite having
no semantic meaning and resolved the issue by adding normalization that
stripped trailing slashes at parse time.

We then discovered that, while there are not semantic differences for
trailing slashes on Simple API index URLs, there are differences for
some flat (or find links) indexes. As reported in
#14367, the change in
#14245 caused a regression for at
least one user.

We attempted to fix the regression via a few approaches.

#14387 attempted to differentiate
between Simple API and flat index URL parsing, but failed to account for
the `Deserialize` implementation, which always assumed Simple API-style
index URLs and incorrectly trimmed trailing slashes in various cases
where we deserialized the `IndexUrl` type from a file. I attempted to
resolve this by performing a larger refactor, but it ended up being
quite painful. In particular, the `Index` type was a blocker — we don't
know the `IndexUrl` variant until we've parsed the `IndexFormat` and
having a multi-stage deserializer is not appealing but adding a new
intermediate type (i.e., `RawIndex`) is painful due to the pervasiveness
of `Index`. Given that we've regressed behavior here and there's not a
straight-forward fix, we're reverting the normalization entirely.

#14503 attempted to perform
normalization at compare-time, but that means we'd fail to invalidate
the lockfile when the a trailing slash was added or removed and given
that a trailing slash has semantic meaning for a find-links URL... we'd
have another correctness problem.

After this revert, we'll retain all index URLs verbatim. The downside to
this approach is that we'll be adding a bunch of trailing slashes back
to lockfiles that we previously normalized out, and we'll be reverting
our fix for users with inconsistent trailing slashes on their index
URLs. Users affected by the original motivating issue should use
consistent trailing slashes on their URLs, as they do frequently have
semantic meaning. We may want to revisit normalization and type aware
index URL parsing as part of a larger change.

Closes  #14367
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

@konstin konstin konstin approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

uv lock keeps trailing / on Linux, but omits on macOS

4 participants

@jtfmumm @charliermarsh @konstin