Skip to content

Commit

Permalink
Fix messages for unavailable packages when range is plural (#6221)
Browse files Browse the repository at this point in the history 
Not in love with the implementation, but it seems like the easiest path
forward for now.
  • Loading branch information
@zanieb
zanieb authored Aug 19, 2024
1 parent 865e9e0 commit 5c2781c
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 44 deletions.
92 changes: 73 additions & 19 deletions crates/distribution-types/src/prioritized_distribution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,62 +70,116 @@ pub enum IncompatibleDist {
Unavailable,
}

impl IncompatibleDist {
pub fn singular_message(&self) -> String {
match self {
Self::Wheel(incompatibility) => match incompatibility {
IncompatibleWheel::NoBinary => format!("has {self}"),
IncompatibleWheel::Tag(_) => format!("has {self}"),
IncompatibleWheel::Yanked(_) => format!("was {self}"),
IncompatibleWheel::ExcludeNewer(ts) => match ts {
Some(_) => format!("was {self}"),
None => format!("has {self}"),
},
IncompatibleWheel::RequiresPython(..) => format!("requires {self}"),
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => format!("has {self}"),
IncompatibleSource::Yanked(_) => format!("was {self}"),
IncompatibleSource::ExcludeNewer(ts) => match ts {
Some(_) => format!("was {self}"),
None => format!("has {self}"),
},
IncompatibleSource::RequiresPython(..) => {
format!("requires {self}")
}
},
Self::Unavailable => format!("has {self}"),
}
}

pub fn plural_message(&self) -> String {
match self {
Self::Wheel(incompatibility) => match incompatibility {
IncompatibleWheel::NoBinary => format!("have {self}"),
IncompatibleWheel::Tag(_) => format!("have {self}"),
IncompatibleWheel::Yanked(_) => format!("were {self}"),
IncompatibleWheel::ExcludeNewer(ts) => match ts {
Some(_) => format!("were {self}"),
None => format!("have {self}"),
},
IncompatibleWheel::RequiresPython(..) => format!("require {self}"),
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => format!("have {self}"),
IncompatibleSource::Yanked(_) => format!("were {self}"),
IncompatibleSource::ExcludeNewer(ts) => match ts {
Some(_) => format!("were {self}"),
None => format!("have {self}"),
},
IncompatibleSource::RequiresPython(..) => {
format!("require {self}")
}
},
Self::Unavailable => format!("have {self}"),
}
}
}

impl Display for IncompatibleDist {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::Wheel(incompatibility) => match incompatibility {
IncompatibleWheel::NoBinary => {
f.write_str("has no source distribution and using wheels is disabled")
f.write_str("no source distribution and using wheels is disabled")
}
IncompatibleWheel::Tag(tag) => match tag {
IncompatibleTag::Invalid => f.write_str("has no wheels with valid tags"),
IncompatibleTag::Invalid => f.write_str("no wheels with valid tags"),
IncompatibleTag::Python => {
f.write_str("has no wheels with a matching Python implementation tag")
}
IncompatibleTag::Abi => {
f.write_str("has no wheels with a matching Python ABI tag")
f.write_str("no wheels with a matching Python implementation tag")
}
IncompatibleTag::Abi => f.write_str("no wheels with a matching Python ABI tag"),
IncompatibleTag::Platform => {
f.write_str("has no wheels with a matching platform tag")
f.write_str("no wheels with a matching platform tag")
}
},
IncompatibleWheel::Yanked(yanked) => match yanked {
Yanked::Bool(_) => f.write_str("was yanked"),
Yanked::Bool(_) => f.write_str("yanked"),
Yanked::Reason(reason) => write!(
f,
"was yanked (reason: {})",
"yanked (reason: {})",
reason.trim().trim_end_matches('.')
),
},
IncompatibleWheel::ExcludeNewer(ts) => match ts {
Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"),
Some(_) => f.write_str("published after the exclude newer time"),
None => f.write_str("no publish time"),
},
IncompatibleWheel::RequiresPython(python, _) => {
write!(f, "requires Python {python}")
write!(f, "Python {python}")
}
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => {
f.write_str("has no usable wheels and building from source is disabled")
f.write_str("no usable wheels and building from source is disabled")
}
IncompatibleSource::Yanked(yanked) => match yanked {
Yanked::Bool(_) => f.write_str("was yanked"),
Yanked::Bool(_) => f.write_str("yanked"),
Yanked::Reason(reason) => write!(
f,
"was yanked (reason: {})",
"yanked (reason: {})",
reason.trim().trim_end_matches('.')
),
},
IncompatibleSource::ExcludeNewer(ts) => match ts {
Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"),
Some(_) => f.write_str("published after the exclude newer time"),
None => f.write_str("no publish time"),
},
IncompatibleSource::RequiresPython(python, _) => {
write!(f, "requires Python {python}")
write!(f, "Python {python}")
}
},
Self::Unavailable => f.write_str("has no available distributions"),
Self::Unavailable => f.write_str("no available distributions"),
}
}
}
Expand Down
17 changes: 12 additions & 5 deletions crates/uv-resolver/src/pubgrub/report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,21 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
UnavailableReason::Package(reason) => {
// While there may be a term attached, this error applies to the entire
// package, so we show it for the entire package
format!("{}{reason}", Padded::new("", &package, " "))
format!(
"{}{}",
Padded::new("", &package, " "),
reason.singular_message()
)
}
UnavailableReason::Version(reason) => {
let set = self.simplify_set(set, package);
format!(
"{}{reason}",
Padded::new("", &self.compatible_range(package, &set), " ")
)
let range = self.compatible_range(package, &set);
let reason = if range.plural() {
reason.plural_message()
} else {
reason.singular_message()
};
format!("{}{reason}", Padded::new("", &range, " "))
}
}
}
Expand Down
73 changes: 55 additions & 18 deletions crates/uv-resolver/src/resolver/availability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,47 @@ pub(crate) enum UnavailableVersion {
Offline,
}

impl Display for UnavailableVersion {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
impl UnavailableVersion {
pub(crate) fn message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => format!("{invalid_dist}"),
UnavailableVersion::MissingMetadata => "not include a `METADATA` file".into(),
UnavailableVersion::InvalidMetadata => "invalid metadata".into(),
UnavailableVersion::InconsistentMetadata => "inconsistent metadata".into(),
UnavailableVersion::InvalidStructure => "an invalid package format".into(),
UnavailableVersion::Offline => "to be downloaded from a registry".into(),
}
}

pub(crate) fn singular_message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => invalid_dist.singular_message(),
UnavailableVersion::MissingMetadata => format!("does {self}"),
UnavailableVersion::InvalidMetadata => format!("has {self}"),
UnavailableVersion::InconsistentMetadata => format!("has {self}"),
UnavailableVersion::InvalidStructure => format!("has {self}"),
UnavailableVersion::Offline => format!("needs {self}"),
}
}

pub(crate) fn plural_message(&self) -> String {
match self {
UnavailableVersion::IncompatibleDist(invalid_dist) => Display::fmt(invalid_dist, f),
UnavailableVersion::MissingMetadata => {
f.write_str("does not include a `METADATA` file")
}
UnavailableVersion::InvalidMetadata => f.write_str("has invalid metadata"),
UnavailableVersion::InconsistentMetadata => f.write_str("has inconsistent metadata"),
UnavailableVersion::InvalidStructure => f.write_str("has an invalid package format"),
UnavailableVersion::Offline => f.write_str("needs to be downloaded from a registry"),
UnavailableVersion::IncompatibleDist(invalid_dist) => invalid_dist.plural_message(),
UnavailableVersion::MissingMetadata => format!("do {self}"),
UnavailableVersion::InvalidMetadata => format!("have {self}"),
UnavailableVersion::InconsistentMetadata => format!("have {self}"),
UnavailableVersion::InvalidStructure => format!("have {self}"),
UnavailableVersion::Offline => format!("need {self}"),
}
}
}

impl Display for UnavailableVersion {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.message())
}
}

/// The package is unavailable and cannot be used.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(crate) enum UnavailablePackage {
Expand All @@ -75,21 +101,32 @@ pub(crate) enum UnavailablePackage {
}

impl UnavailablePackage {
pub(crate) fn as_str(&self) -> &'static str {
pub(crate) fn message(&self) -> &'static str {
match self {
UnavailablePackage::NoIndex => "not found in the provided package locations",
UnavailablePackage::Offline => "not found in the cache",
UnavailablePackage::NotFound => "not found in the package registry",
UnavailablePackage::MissingMetadata => "not include a `METADATA` file",
UnavailablePackage::InvalidMetadata(_) => "invalid metadata",
UnavailablePackage::InvalidStructure(_) => "an invalid package format",
}
}

pub(crate) fn singular_message(&self) -> String {
match self {
UnavailablePackage::NoIndex => "was not found in the provided package locations",
UnavailablePackage::Offline => "was not found in the cache",
UnavailablePackage::NotFound => "was not found in the package registry",
UnavailablePackage::MissingMetadata => "does not include a `METADATA` file",
UnavailablePackage::InvalidMetadata(_) => "has invalid metadata",
UnavailablePackage::InvalidStructure(_) => "has an invalid package format",
UnavailablePackage::NoIndex => format!("was {self}"),
UnavailablePackage::Offline => format!("was {self}"),
UnavailablePackage::NotFound => format!("was {self}"),
UnavailablePackage::MissingMetadata => format!("does {self}"),
UnavailablePackage::InvalidMetadata(_) => format!("has {self}"),
UnavailablePackage::InvalidStructure(_) => format!("has {self}"),
}
}
}

impl Display for UnavailablePackage {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.write_str(self.as_str())
f.write_str(self.message())
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/cache_prune.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ fn prune_unzipped() -> Result<()> {
and all of:
iniconfig<=0.1
iniconfig>=1.0.0
needs to be downloaded from a registry, we can conclude that iniconfig<1.0.0 cannot be used.
need to be downloaded from a registry, we can conclude that iniconfig<1.0.0 cannot be used.
And because you require iniconfig, we can conclude that your requirements are unsatisfiable.
hint: Pre-releases are available for iniconfig in the requested range (e.g., 0.2.dev0), but pre-releases weren't enabled (try: `--prerelease=allow`)
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/pip_install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,7 @@ fn install_only_binary_all_and_no_binary_all() {
anyio>=3.0.0,<=3.6.2
anyio>=3.7.0,<=3.7.1
anyio>=4.0.0
has no usable wheels and building from source is disabled, we can conclude that all of:
have no usable wheels and building from source is disabled, we can conclude that all of:
anyio<1.1.0
anyio>1.4.0,<2.0.0
anyio>2.2.0,<3.0.0
Expand Down

0 comments on commit 5c2781c

Please sign in to comment.