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.

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

iron out how to represent path dependencies in the universal lock file #3506

Closed
1 of 2 tasks
Tracked by #3347
BurntSushi opened this issue May 10, 2024 · 5 comments
Closed
1 of 2 tasks
Tracked by #3347
Assignees
Labels
preview Experimental behavior

Comments

@BurntSushi
Copy link
Member

BurntSushi commented May 10, 2024

We want to support path dependencies in our lock file. But the data model for path dependencies in our lock file is, at the time of writing, not quite right. I believe what we ought to have for a path dependency is a single file URL to a directory, source distribution or wheel. But the data model currently allows for a source distribution and zero or more wheels, but specifically does not allow for a directory.

I think the current data model is mostly just a reaction to the types that we have after resolution. Namely, I think we have two related types. The first is for wheel path dependencies:

/// A built distribution (wheel) that exists in a local directory.
#[derive(Debug, Clone)]
pub struct PathBuiltDist {
pub filename: WheelFilename,
pub url: VerbatimUrl,
pub path: PathBuf,
}

And the second is for source distribution path dependencies:

/// A source distribution that exists in a local directory.
#[derive(Debug, Clone)]
pub struct PathSourceDist {
pub name: PackageName,
pub url: VerbatimUrl,
pub path: PathBuf,
pub editable: bool,
}

My understanding is that the second is also used when we have a path dependency to a directory.

This all matters for the lock file because if a path dependency points to a wheel or a source dist, then we want to record a hash for it. But if it comes from a directory, then we very specifically do not want to record a hash.

So to fix this issue, I think we need to:

  • Adjust the PathSourceDist type so that it indicates whether it came from a real source distribution or a directory. Possibly one way to do this is to split the type in two, with one type representing source distributions and the other representing a directory.
  • Adjust the Lock data model so that distributions with a path source kind are more limited than what they are today. I think we actually do not want to encode the file path itself, and instead derive that from the pyproject.toml. (This would match what Cargo does.)
@charliermarsh
Copy link
Member

I can look at the first bullet there, splitting PathSourceDist.

charliermarsh added a commit that referenced this issue May 13, 2024
## Summary

I think this is overall good change because it explicitly encodes (in
the type system) something that was previously implicit. I'm not a huge
fan of the names here, open to input.

It covers some of #3506 but I
don't think it _closes_ it.
@charliermarsh
Copy link
Member

What does the second bullet mean exactly? Can you flesh it out a bit more? Perhaps I can help with it.

@charliermarsh
Copy link
Member

Is it that the path gets omitted entirely when it's part of the workspace?

@BurntSushi
Copy link
Member Author

Is it that the path gets omitted entirely when it's part of the workspace?

Yeah exactly. Cargo rebuilds the path from the Cargo.toml instead, as I understand it. So in order to get the path of a path dependency in a lock file, I think there is some interaction point between what's in pyproject.toml and what's in the lock file.

(There is at least one other interaction point as well: finding the relevant root package in the lock file at which to start a graph traversal.)

@konstin
Copy link
Member

konstin commented Jun 11, 2024

Fixed by #4205, we can revisit this later if we want to omit paths

@konstin konstin closed this as completed Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Experimental behavior
Projects
None yet
Development

No branches or pull requests

3 participants