Skip to content

Commit

Permalink
Avoid using editable tag in lockfile for non-package dependencies (#6728
Browse files Browse the repository at this point in the history 
)

## Summary

Use a dedicated source type for non-package requirements. Also enables
us to support non-package `path` dependencies _and_ removes the need to
have the member `pyproject.toml` files available when we sync _and_
makes it explicit which dependencies are virtual vs. not (as evidenced
by the snapshot changes). All good things!
  • Loading branch information
@charliermarsh
charliermarsh authored Aug 28, 2024
1 parent 8fdb3a8 commit 56cc0c9
Show file tree
Hide file tree
Showing 23 changed files with 407 additions and 74 deletions.
9 changes: 9 additions & 0 deletions crates/distribution-types/src/cached.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct CachedDirectUrlDist {
pub url: VerbatimUrl,
pub path: PathBuf,
pub editable: bool,
pub r#virtual: bool,
pub hashes: Vec<HashDigest>,
}

Expand All @@ -57,13 +58,15 @@ impl CachedDist {
hashes,
path,
editable: false,
r#virtual: false,
}),
Dist::Built(BuiltDist::Path(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
path,
editable: false,
r#virtual: false,
}),
Dist::Source(SourceDist::Registry(_dist)) => Self::Registry(CachedRegistryDist {
filename,
Expand All @@ -76,27 +79,31 @@ impl CachedDist {
hashes,
path,
editable: false,
r#virtual: false,
}),
Dist::Source(SourceDist::Git(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
path,
editable: false,
r#virtual: false,
}),
Dist::Source(SourceDist::Path(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
path,
editable: false,
r#virtual: false,
}),
Dist::Source(SourceDist::Directory(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
path,
editable: dist.editable,
r#virtual: dist.r#virtual,
}),
}
}
Expand Down Expand Up @@ -124,6 +131,7 @@ impl CachedDist {
url: dist.url.raw().clone(),
install_path: path,
editable: dist.editable,
r#virtual: dist.r#virtual,
})))
} else {
Ok(Some(ParsedUrl::try_from(dist.url.to_url())?))
Expand Down Expand Up @@ -161,6 +169,7 @@ impl CachedDirectUrlDist {
hashes,
path,
editable: false,
r#virtual: false,
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions crates/distribution-types/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ pub struct DirectorySourceDist {
pub install_path: PathBuf,
/// Whether the package should be installed in editable mode.
pub editable: bool,
/// Whether the package should be built and installed.
pub r#virtual: bool,
/// The URL as it was provided by the user.
pub url: VerbatimUrl,
}
Expand Down Expand Up @@ -404,6 +406,7 @@ impl Dist {
url: VerbatimUrl,
install_path: &Path,
editable: bool,
r#virtual: bool,
) -> Result<Dist, Error> {
// Convert to an absolute path.
let install_path = path::absolute(install_path)?;
Expand All @@ -421,6 +424,7 @@ impl Dist {
name,
install_path,
editable,
r#virtual,
url,
})))
}
Expand Down Expand Up @@ -458,6 +462,7 @@ impl Dist {
url.verbatim,
&directory.install_path,
directory.editable,
directory.r#virtual,
),
ParsedUrl::Git(git) => {
Self::from_git_url(name, url.verbatim, git.url, git.subdirectory)
Expand Down
1 change: 1 addition & 0 deletions crates/distribution-types/src/resolution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ impl From<&ResolvedDist> for Requirement {
install_path: sdist.install_path.clone(),
url: sdist.url.clone(),
editable: sdist.editable,
r#virtual: sdist.r#virtual,
},
},
ResolvedDist::Installed(dist) => RequirementSource::Registry {
Expand Down
7 changes: 6 additions & 1 deletion crates/pypi-types/src/parsed_url.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
editable: false,
r#virtual: false,
})
} else {
ParsedUrl::Path(ParsedPathUrl {
Expand Down Expand Up @@ -101,6 +102,7 @@ impl UnnamedRequirementUrl for VerbatimParsedUrl {
url: verbatim.to_url(),
install_path: verbatim.as_path()?,
editable: false,
r#virtual: false,
})
} else {
ParsedUrl::Path(ParsedPathUrl {
Expand Down Expand Up @@ -208,15 +210,17 @@ pub struct ParsedDirectoryUrl {
/// The absolute path to the distribution which we use for installing.
pub install_path: PathBuf,
pub editable: bool,
pub r#virtual: bool,
}

impl ParsedDirectoryUrl {
/// Construct a [`ParsedDirectoryUrl`] from a path requirement source.
pub fn from_source(install_path: PathBuf, editable: bool, url: Url) -> Self {
pub fn from_source(install_path: PathBuf, editable: bool, r#virtual: bool, url: Url) -> Self {
Self {
url,
install_path,
editable,
r#virtual,
}
}
}
Expand Down Expand Up @@ -370,6 +374,7 @@ impl TryFrom<Url> for ParsedUrl {
url,
install_path: path.clone(),
editable: false,
r#virtual: false,
}))
} else {
Ok(Self::Path(ParsedPathUrl {
Expand Down
30 changes: 30 additions & 0 deletions crates/pypi-types/src/requirement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,14 @@ impl From<Requirement> for pep508_rs::Requirement<VerbatimParsedUrl> {
RequirementSource::Directory {
install_path,
editable,
r#virtual,
url,
} => Some(VersionOrUrl::Url(VerbatimParsedUrl {
parsed_url: ParsedUrl::Directory(ParsedDirectoryUrl {
url: url.to_url(),
install_path,
editable,
r#virtual,
}),
verbatim: url,
})),
Expand Down Expand Up @@ -360,6 +362,8 @@ pub enum RequirementSource {
install_path: PathBuf,
/// For a source tree (a directory), whether to install as an editable.
editable: bool,
/// For a source tree (a directory), whether the project should be built and installed.
r#virtual: bool,
/// The PEP 508 style URL in the format
/// `file:///<path>#subdirectory=<subdirectory>`.
url: VerbatimUrl,
Expand All @@ -379,6 +383,7 @@ impl RequirementSource {
ParsedUrl::Directory(directory) => RequirementSource::Directory {
install_path: directory.install_path.clone(),
editable: directory.editable,
r#virtual: directory.r#virtual,
url,
},
ParsedUrl::Git(git) => RequirementSource::Git {
Expand Down Expand Up @@ -435,11 +440,13 @@ impl RequirementSource {
Self::Directory {
install_path,
editable,
r#virtual,
url,
} => Some(VerbatimParsedUrl {
parsed_url: ParsedUrl::Directory(ParsedDirectoryUrl::from_source(
install_path.clone(),
*editable,
*r#virtual,
url.to_url(),
)),
verbatim: url.clone(),
Expand Down Expand Up @@ -515,11 +522,14 @@ impl RequirementSource {
RequirementSource::Directory {
install_path,
editable,
r#virtual,
url,
..
} => Ok(Self::Directory {
install_path: relative_to(&install_path, path)
.or_else(|_| std::path::absolute(install_path))?,
editable,
r#virtual,
url,
}),
}
Expand Down Expand Up @@ -582,6 +592,8 @@ enum RequirementSourceWire {
Directory { directory: PortablePathBuf },
/// Ex) `source = { editable = "/home/ferris/iniconfig" }`
Editable { editable: PortablePathBuf },
/// Ex) `source = { editable = "/home/ferris/iniconfig" }`
Virtual { r#virtual: PortablePathBuf },
/// Ex) `source = { specifier = "foo >1,<2" }`
Registry {
#[serde(skip_serializing_if = "VersionSpecifiers::is_empty", default)]
Expand Down Expand Up @@ -668,12 +680,17 @@ impl From<RequirementSource> for RequirementSourceWire {
RequirementSource::Directory {
install_path,
editable,
r#virtual,
url: _,
} => {
if editable {
Self::Editable {
editable: PortablePathBuf::from(install_path),
}
} else if r#virtual {
Self::Virtual {
r#virtual: PortablePathBuf::from(install_path),
}
} else {
Self::Directory {
directory: PortablePathBuf::from(install_path),
Expand Down Expand Up @@ -760,6 +777,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
Ok(Self::Directory {
install_path: directory,
editable: false,
r#virtual: false,
url,
})
}
Expand All @@ -769,6 +787,17 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
Ok(Self::Directory {
install_path: editable,
editable: true,
r#virtual: false,
url,
})
}
RequirementSourceWire::Virtual { r#virtual } => {
let r#virtual = PathBuf::from(r#virtual);
let url = VerbatimUrl::from_path(&r#virtual, &*CWD)?;
Ok(Self::Directory {
install_path: r#virtual,
editable: false,
r#virtual: true,
url,
})
}
Expand Down Expand Up @@ -825,6 +854,7 @@ mod tests {
source: RequirementSource::Directory {
install_path: PathBuf::from(path),
editable: false,
r#virtual: false,
url: VerbatimUrl::from_absolute_path(path).unwrap(),
},
origin: None,
Expand Down
1 change: 1 addition & 0 deletions crates/requirements-txt/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1861,6 +1861,7 @@ mod test {
},
install_path: "/foo/bar",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down
3 changes: 3 additions & 0 deletions crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-bare-url.txt.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -72,6 +73,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -125,6 +127,7 @@ RequirementsTxt {
},
install_path: "/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down
6 changes: 6 additions & 0 deletions crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-editable.txt.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -81,6 +82,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -137,6 +139,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -193,6 +196,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -249,6 +253,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -298,6 +303,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/editable[d",
editable: true,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down
3 changes: 3 additions & 0 deletions ...es/requirements-txt/src/snapshots/requirements_txt__test__parse-windows-bare-url.txt.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -72,6 +73,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down Expand Up @@ -125,6 +127,7 @@ RequirementsTxt {
},
install_path: "<REQUIREMENTS_DIR>/scripts/packages/black_editable",
editable: false,
virtual: false,
},
),
verbatim: VerbatimUrl {
Expand Down
Loading

0 comments on commit 56cc0c9

Please sign in to comment.