Skip to content

Commit

Permalink
Reverts
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 13, 2024
1 parent b21c3d0 commit dcc133d
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 61 deletions.
55 changes: 27 additions & 28 deletions crates/platform-tags/src/tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,34 +203,33 @@ impl Tags {
wheel_abi_tags: &[String],
wheel_platform_tags: &[String],
) -> TagCompatibility {
return TagCompatibility::Compatible(TagPriority(NonZeroU32::new(1).unwrap()));
// let mut max_compatibility = TagCompatibility::Incompatible(IncompatibleTag::Invalid);
//
// for wheel_py in wheel_python_tags {
// let Some(abis) = self.map.get(wheel_py) else {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Python));
// continue;
// };
// for wheel_abi in wheel_abi_tags {
// let Some(platforms) = abis.get(wheel_abi) else {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Abi));
// continue;
// };
// for wheel_platform in wheel_platform_tags {
// let priority = platforms.get(wheel_platform).copied();
// if let Some(priority) = priority {
// max_compatibility =
// max_compatibility.max(TagCompatibility::Compatible(priority));
// } else {
// max_compatibility = max_compatibility
// .max(TagCompatibility::Incompatible(IncompatibleTag::Platform));
// }
// }
// }
// }
// max_compatibility
let mut max_compatibility = TagCompatibility::Incompatible(IncompatibleTag::Invalid);

for wheel_py in wheel_python_tags {
let Some(abis) = self.map.get(wheel_py) else {
max_compatibility =
max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Python));
continue;
};
for wheel_abi in wheel_abi_tags {
let Some(platforms) = abis.get(wheel_abi) else {
max_compatibility =
max_compatibility.max(TagCompatibility::Incompatible(IncompatibleTag::Abi));
continue;
};
for wheel_platform in wheel_platform_tags {
let priority = platforms.get(wheel_platform).copied();
if let Some(priority) = priority {
max_compatibility =
max_compatibility.max(TagCompatibility::Compatible(priority));
} else {
max_compatibility = max_compatibility
.max(TagCompatibility::Incompatible(IncompatibleTag::Platform));
}
}
}
}
max_compatibility
}
}

Expand Down
3 changes: 0 additions & 3 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ pub enum ResolveError {
#[error("There are conflicting local versions requested for package `{0}`: {1} vs. {2}")]
ConflictingLocal(PackageName, String, String),

#[error("There are conflicting versions for `{0}`: {1}")]
ConflictingVersions(String, String),

#[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")]
DisallowedUrl(PackageName, String),

Expand Down
5 changes: 3 additions & 2 deletions crates/uv-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct PubGrubDependencies(Vec<(PubGrubPackage, Range<Version>)>);

impl PubGrubDependencies {
/// Generate a set of `PubGrub` dependencies from a set of requirements.
#[allow(clippy::too_many_arguments)]
pub(crate) fn from_requirements(
requirements: &[Requirement],
constraints: &Constraints,
Expand Down Expand Up @@ -145,7 +146,7 @@ fn to_pubgrub(
if let [specifier] = specifiers.as_ref() {
if *specifier.operator() == Operator::Equal {
if let Some(expected) = locals.get(&requirement.name) {
return if locals.is_allowed( expected, specifier.version()) {
return if Locals::is_compatible(expected, specifier.version()) {
debug!(
"using local version {expected} for {name} instead of {specifier}",
expected = expected,
Expand All @@ -162,7 +163,7 @@ fn to_pubgrub(
expected.to_string(),
specifier.version().to_string(),
))
}
};
}
}
}
Expand Down
28 changes: 18 additions & 10 deletions crates/uv-resolver/src/resolver/locals.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
use std::ops::Deref;

use rustc_hash::FxHashMap;

use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, VerbatimUrl, VersionOrUrl};
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::{Manifest, ResolveError};
use crate::Manifest;

#[derive(Debug, Default)]
pub(crate) struct Locals {
/// A map of package names to their associated, required URLs.
/// A map of package names to their associated, required local versions.
required: FxHashMap<PackageName, Version>,
}

impl Locals {
/// Determine the set of permitted local versions in the [`Manifest`].
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: &MarkerEnvironment,
) -> Result<Self, ResolveError> {
) -> Self {
let mut required: FxHashMap<PackageName, Version> = FxHashMap::default();

// Add all direct requirements and constraints. If there are any conflicts, return an error.
Expand Down Expand Up @@ -50,16 +49,25 @@ impl Locals {
}
}

Ok(Self { required })
Self { required }
}

/// Return the [`VerbatimUrl`] associated with the given package name, if any.
pub(crate) fn get(&self, package: &PackageName) -> Option<&Version> {
self.required.get(package)
}

/// Returns `true` if a package is allowed to have a local version.
pub(crate) fn is_allowed(&self, expected: &Version, provided: &Version) -> bool {
/// Returns `true` if a provided version is compatible with the expected local version.
///
/// The versions are compatible if they are the same including their local segment, or the
/// same except for the local segment, which is empty in the provided version.
///
/// For example, if the expected version is `1.0.0+local` and the provided version is `1.0.0+other`,
/// this function will return `false`.
///
/// If the expected version is `1.0.0+local` and the provided version is `1.0.0`, the function will
/// return `true`.
pub(crate) fn is_compatible(expected: &Version, provided: &Version) -> bool {
// The requirements should be the same, ignoring local segments.
if expected.clone().without_local() != provided.clone().without_local() {
return false;
Expand All @@ -80,7 +88,7 @@ fn to_local(version_or_url: Option<&VersionOrUrl>) -> Option<&Version> {
return None;
};

let [specifier] = specifier.deref() else {
let [specifier] = &**specifier else {
return None;
};

Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
selector: CandidateSelector::for_resolution(options, &manifest, markers),
dependency_mode: options.dependency_mode,
urls: Urls::from_manifest(&manifest, markers)?,
locals: Locals::from_manifest(&manifest, markers)?,
locals: Locals::from_manifest(&manifest, markers),
project: manifest.project,
requirements: manifest.requirements,
constraints: Constraints::from_requirements(manifest.constraints),
Expand Down
38 changes: 23 additions & 15 deletions crates/uv/tests/pip_install_scenarios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,7 @@ fn local_simple() {
filters.push((r"local-simple-", "pkg-"));

uv_snapshot!(filters, command(&context)
.arg("local-simple-a==1.2.3+foo")
.arg("local-simple-a==1.2.3")
, @r###"
success: false
exit_code: 1
Expand All @@ -1344,7 +1344,7 @@ fn local_simple() {
╰─▶ Because there is no version of albatross==1.2.3 and you require albatross==1.2.3, we can conclude that the requirements are unsatisfiable.
"###);

// The verison '1.2.3+foo' satisfies the constraint '==1.2.3'.
// The version '1.2.3+foo' satisfies the constraint '==1.2.3'.
assert_not_installed(&context.venv, "local_simple_a", &context.temp_dir);
}

Expand Down Expand Up @@ -1386,7 +1386,7 @@ fn local_not_used_with_sdist() {
+ albatross==1.2.3
"###);

// The verison '1.2.3' with an sdist satisfies the constraint '==1.2.3'.
// The version '1.2.3' with an sdist satisfies the constraint '==1.2.3'.
assert_not_installed(
&context.venv,
"local_not_used_with_sdist_a",
Expand Down Expand Up @@ -1431,7 +1431,7 @@ fn local_used_without_sdist() {
╰─▶ Because albatross==1.2.3 is unusable because no wheels are available with a matching Python ABI and you require albatross==1.2.3, we can conclude that the requirements are unsatisfiable.
"###);

// The verison '1.2.3+foo' satisfies the constraint '==1.2.3'.
// The version '1.2.3+foo' satisfies the constraint '==1.2.3'.
assert_not_installed(
&context.venv,
"local_used_without_sdist_a",
Expand Down Expand Up @@ -1532,9 +1532,19 @@ fn local_transitive() {
+ bluebird==2.0.0+foo
"###);

// The verison '2.0.0+foo' satisfies both ==2.0.0 and ==2.0.0+foo.
assert_not_installed(&context.venv, "local_transitive_a", &context.temp_dir);
assert_not_installed(&context.venv, "local_transitive_b", &context.temp_dir);
// The version '2.0.0+foo' satisfies both ==2.0.0 and ==2.0.0+foo.
assert_installed(
&context.venv,
"local_transitive_a",
"1.0.0",
&context.temp_dir,
);
assert_installed(
&context.venv,
"local_transitive_b",
"2.0.0+foo",
&context.temp_dir,
);
}

/// A transitive dependency has both a non-local and local version published, but
Expand Down Expand Up @@ -1569,19 +1579,17 @@ fn local_transitive_confounding() {
uv_snapshot!(filters, command(&context)
.arg("local-transitive-confounding-a")
, @r###"
success: true
exit_code: 0
success: false
exit_code: 1
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Downloaded 2 packages in [TIME]
Installed 2 packages in [TIME]
+ albatross==1.0.0
+ bluebird==2.0.0
× No solution found when resolving dependencies:
╰─▶ Because bluebird==2.0.0 is unusable because no wheels are available with a matching Python ABI and albatross==1.0.0 depends on bluebird==2.0.0, we can conclude that albatross==1.0.0 cannot be used.
And because only albatross==1.0.0 is available and you require albatross, we can conclude that the requirements are unsatisfiable.
"###);

// The verison '1.2.3+foo' satisfies the constraint '==1.2.3'.
// The version '1.2.3+foo' satisfies the constraint '==1.2.3'.
assert_not_installed(
&context.venv,
"local_transitive_confounding_a",
Expand Down
12 changes: 10 additions & 2 deletions requirements.in
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
torchvision==0.15.1+cu118
torch==2.0.0+cu118
#torchvision==0.15.1+cu118
#torch==2.0.0+cu118

# This should _fail_. It shouldn't allow a local version.
# torch>2.2.1

# This should return 2.2.0, not a local version.
# torch<2.2.1

torch>=2.2.1

0 comments on commit dcc133d

Please sign in to comment.