Skip to content

Commit

Permalink
Annotate sources of requirements (#3269)
Browse files Browse the repository at this point in the history
## Summary

Fixes #1343. This is kinda a first
draft at the moment, but does at least mostly work locally (barring some
bits of the test suite that seem to not work for me in general).

## Test Plan

Mostly running the existing tests and checking the revised output is
sane

## Outstanding issues

Most of these come down to "AFAIK, the existing tools don't support
these patterns, but `uv` does" and so I'm not sure there's an existing
good answer here! Most of the answers so far are "whatever was easiest
to build"

- [x] ~~Is "-r pyproject.toml" correct? Should it show something else or
get skipped entirely~~ No it wasn't. Fixed in
3044fa8
- [ ] If the requirements file is stdin, that just gets skipped. Should
it be recorded?
- [ ] Overrides get shown as "--override<override.txt>". Correct?
- [x] ~~Some of the tests (e.g.
`dependency_excludes_non_contiguous_range_of_compatible_versions`) make
assumptions about the order of package versions being outputted, which
this PR breaks. I'm not sure if the text is fairly arbitrary and can be
replaced or whether the behaviour needs fixing?~~ - fixed by removing
the custom pubgrub PartialEq/Hash
- [ ] Are all the `TrackedFromStr` et al changes needed, or is there an
easier way? I don't think so, I think it's necessary to track these sort
of things fairly comprehensively to make this feature work, and this
sort of invasive change feels necessary, but happy to be proved wrong
there :)
- [x] ~~If you have a requirement coming in from two or more different
requirements files only one turns up. I've got a closed-source example
for this (can go into more detail if needed), mostly consisting of a
complicated set of common deps creating a larger set. It's a rarer case,
but worth considering.~~ 042432b
- [ ] Doesn't add annotations for `setup.py` yet
- This is pretty hard, as the correct location to insert the path is
`crates/pypi-types/src/metadata.rs`'s `parse_pkg_info`, which as it's
based off a source distribution has entirely thrown away such matters as
"where did this package requirement get built from". Could add "`built
package name`" as a dep, but that's a little odd.
  • Loading branch information
palfrey authored May 9, 2024
1 parent 367958e commit bc963d1
Show file tree
Hide file tree
Showing 42 changed files with 1,287 additions and 323 deletions.
52 changes: 52 additions & 0 deletions crates/distribution-types/src/annotation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use serde::{Deserialize, Deserializer, Serialize};
use std::path::PathBuf;
use uv_fs::Simplified;

/// Source of a dependency, e.g., a `-r requirements.txt` file.
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize)]
pub enum SourceAnnotation {
/// A `pyproject.toml` file.
PyProject {
path: PathBuf,
project_name: Option<String>,
},
/// A `-c constraints.txt` file.
Constraint(PathBuf),
/// An `--override overrides.txt` file.
Override(PathBuf),
/// A `-r requirements.txt` file.
Requirement(PathBuf),
}

impl<'de> Deserialize<'de> for SourceAnnotation {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
Ok(SourceAnnotation::Requirement(PathBuf::from(s)))
}
}

impl std::fmt::Display for SourceAnnotation {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Requirement(path) => {
write!(f, "-r {}", path.user_display())
}
Self::Constraint(path) => {
write!(f, "-c {}", path.user_display())
}
Self::Override(path) => {
write!(f, "--override {}", path.user_display())
}
Self::PyProject { path, project_name } => {
if let Some(project_name) = project_name {
write!(f, "{} ({})", project_name, path.user_display())
} else {
write!(f, "{}", path.user_display())
}
}
}
}
}
2 changes: 2 additions & 0 deletions crates/distribution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use pep440_rs::Version;
use pep508_rs::{Pep508Url, Scheme, VerbatimUrl};
use uv_normalize::PackageName;

pub use crate::annotation::*;
pub use crate::any::*;
pub use crate::buildable::*;
pub use crate::cached::*;
Expand All @@ -62,6 +63,7 @@ pub use crate::resolved::*;
pub use crate::specified_requirement::*;
pub use crate::traits::*;

mod annotation;
mod any;
mod buildable;
mod cached;
Expand Down
2 changes: 2 additions & 0 deletions crates/distribution-types/src/requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub struct Requirement {
pub extras: Vec<ExtraName>,
pub marker: Option<MarkerTree>,
pub source: RequirementSource,
pub path: Option<PathBuf>,
}

impl Requirement {
Expand Down Expand Up @@ -62,6 +63,7 @@ impl Requirement {
extras: requirement.extras,
marker: requirement.marker,
source,
path: requirement.path,
})
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/distribution-types/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ impl From<&ResolvedDist> for Requirement {
extras: vec![],
marker: None,
source,
path: None,
}
}
}
2 changes: 2 additions & 0 deletions crates/distribution-types/src/specified_requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ pub struct UnresolvedRequirementSpecification {
pub requirement: UnresolvedRequirement,
/// Hashes of the downloadable packages.
pub hashes: Vec<String>,
/// Path of the source of the requirement
pub path: Option<String>,
}

/// A requirement read from a `requirements.txt` or `pyproject.toml` file.
Expand Down
17 changes: 16 additions & 1 deletion crates/pep508-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use std::fmt::{Debug, Display, Formatter};
use std::hash::{Hash, Hasher};
#[cfg(feature = "pyo3")]
use std::ops::Deref;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;

#[cfg(feature = "pyo3")]
Expand Down Expand Up @@ -148,6 +148,16 @@ pub struct Requirement<T: Pep508Url = VerbatimUrl> {
/// `requests [security,tests] >= 2.8.1, == 2.8.* ; python_version > "3.8"`.
/// Those are a nested and/or tree.
pub marker: Option<MarkerTree>,
/// The source file containing the requirement.
pub path: Option<PathBuf>,
}

impl Requirement {
/// Set the source file containing the requirement.
#[must_use]
pub fn with_source(self, path: Option<PathBuf>) -> Self {
Self { path, ..self }
}
}

impl<T: Pep508Url + Display> Display for Requirement<T> {
Expand Down Expand Up @@ -482,6 +492,7 @@ impl<T: Pep508Url> Requirement<T> {
extras,
version_or_url,
marker,
path,
} = self;
Requirement {
name,
Expand All @@ -494,6 +505,7 @@ impl<T: Pep508Url> Requirement<T> {
Some(VersionOrUrl::Url(url)) => Some(VersionOrUrl::Url(U::from(url))),
},
marker,
path,
}
}
}
Expand Down Expand Up @@ -1017,6 +1029,7 @@ fn parse_pep508_requirement<T: Pep508Url>(
extras,
version_or_url: requirement_kind,
marker,
path: None,
})
}

Expand Down Expand Up @@ -1158,6 +1171,7 @@ mod tests {
operator: MarkerOperator::LessThan,
r_value: MarkerValue::QuotedString("2.7".to_string()),
})),
path: None,
};
assert_eq!(requests, expected);
}
Expand Down Expand Up @@ -1383,6 +1397,7 @@ mod tests {
extras: vec![],
marker: None,
version_or_url: Some(VersionOrUrl::Url(Url::parse(url).unwrap())),
path: None,
};
assert_eq!(pip_url, expected);
}
Expand Down
11 changes: 10 additions & 1 deletion crates/pep508-rs/src/unnamed.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fmt::{Display, Formatter};
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
Expand Down Expand Up @@ -30,6 +30,8 @@ pub struct UnnamedRequirement {
/// `requests [security,tests] >= 2.8.1, == 2.8.* ; python_version > "3.8"`.
/// Those are a nested and/or tree.
pub marker: Option<MarkerTree>,
/// The source file containing the requirement.
pub path: Option<PathBuf>,
}

impl UnnamedRequirement {
Expand All @@ -41,6 +43,12 @@ impl UnnamedRequirement {
true
}
}

/// Set the source file containing the requirement.
#[must_use]
pub fn with_source(self, path: Option<PathBuf>) -> Self {
Self { path, ..self }
}
}

impl Display for UnnamedRequirement {
Expand Down Expand Up @@ -159,6 +167,7 @@ fn parse_unnamed_requirement(
url,
extras,
marker,
path: None,
})
}

Expand Down
Loading

0 comments on commit bc963d1

Please sign in to comment.