When validating lockfile, take account of indexes defined as sources in path dependencies#14003
When validating lockfile, take account of indexes defined as sources in path dependencies#14003
Conversation
crates/uv-workspace/src/workspace.rs
Outdated
|
|
||
| // Canonicalize path to compare symlinks and relative paths correctly | ||
| let Ok(canonical_path) = dep_path.canonicalize() else { | ||
| debug!( |
There was a problem hiding this comment.
I've taken a permissive approach here since we are checking for pyproject.toml files defined in path dependencies, and only for the purposes of validation. That's why I've chosen to log problems canonicalizing and parsing them and then to continue.
crates/uv-workspace/src/workspace.rs
Outdated
| } | ||
| Err(e) => { | ||
| debug!( | ||
| "Failed to read `pyproject.toml` in path dependency `{}`: {}", |
| false, | ||
| )) | ||
| }) | ||
| .or(Some(Cow::Borrowed(index_locations))) |
There was a problem hiding this comment.
I'm using a Cow here because the common case will probably be to have no path dependencies. This avoids cloning in that common case
jtfmumm
force-pushed
the
jtfm/custom-indexes-lockfile
branch
from
956031d to
b11727c
Compare
jtfmumm
force-pushed
the
jtfm/custom-indexes-lockfile
branch
from
b11727c to
fbd3700
Compare
crates/uv-workspace/src/workspace.rs
Outdated
| { | ||
| for source_list in sources.inner().values() { | ||
| for source in source_list.iter() { | ||
| if let Source::Path { path, .. } = source { |
There was a problem hiding this comment.
I'm currently only handling the Path case from the issue. We could also fetch Source::Git and Source::Url pyprojects but I wasn't sure if we wanted to do that here.
jtfmumm
force-pushed
the
jtfm/custom-indexes-lockfile
branch
from
fbd3700 to
f2d707b
Compare
| } | ||
|
|
||
| /// Parses a `pyproject.toml` file from a path. | ||
| fn pyproject_toml_from_path(pyproject_path: PathBuf) -> Result<PyProjectToml, WorkspaceError> { |
There was a problem hiding this comment.
We should avoid reparsing pyproject.toml, this is an expensive operation when it runs as part of a noop uv run. See #12096 for context and benchmarking instructions.
Can we reuse the parsed file through using the workspace cache around collect_members_only, or alternatively collect this information after workspace discovery so we can read from a warm cache of workspace members?
jtfmumm
had a problem deploying
to
uv-test-publish
GitHub Actions
Failure
jtfmumm
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
jtfmumm
force-pushed
the
jtfm/custom-indexes-lockfile
branch
from
9db74dd to
9e5c16d
Compare
jtfmumm
temporarily deployed
to
uv-test-registries
GitHub Actions
Inactive
|
Could we instead solve this by adding any explicit index assignments to the lockfile? We already write the normalized requirements ( |
…e explicit indexes (#14876) ## Summary This is an alternative to #14003 that takes advantage of the fact that we already validate that the requirements are up-to-date when validating the lockfile, and the requirements for pinned requirements include the index itself -- so rather than collecting all the explicit indexes upfront, we can just add them to the available list as we iterate over the lockfile's dependency graph. This gets all the tests passing from that PR, but with ~no performance impact and a much less invasive change. It also gets the "circular dependency" test passing, which is marked with a TODO in that PR. Closes #11419.
3 participants
As described in #11419, when an index was defined as a source in a path dependency (and that index ended up in the lockfile), it would cause lockfile validation to incorrectly determine that the file had changed. This PR adds those indexes to the list uv uses to validate.
This adds those path dependency source indexes as a new field on
Workspace(including from transitive path dependencies). It includes cycle detection to handle the case of circular path dependencies.This includes test for explicit indexes in a path dependency, a combination of explicit and non-explicit, path dependencies without indexes defined, nested (transitive) path dependencies, and circular path dependencies. The circular path dependency test does not hang (indicating successful cycle detection) but lockfile validation still indicates the file has changed. This is not a change from
mainso I have marked this as aTODO.Closes #11419