Skip to content

Commit

Permalink
Encode mutually-incompatible pairs of markers
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Nov 26, 2024
1 parent d187535 commit 7a177a9
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 35 deletions.
4 changes: 2 additions & 2 deletions crates/uv-distribution/src/metadata/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ impl LoweredRequirement {
})
.chain(std::iter::once(Ok(remaining)))
.filter(|requirement| match requirement {
Ok(requirement) => !requirement.0.marker.is_false(),
Ok(requirement) => !(requirement.0.marker.is_false() || requirement.0.marker.is_conflicting()),
Err(_) => true,
}),
)
Expand Down Expand Up @@ -466,7 +466,7 @@ impl LoweredRequirement {
})
.chain(std::iter::once(Ok(remaining)))
.filter(|requirement| match requirement {
Ok(requirement) => !requirement.0.marker.is_false(),
Ok(requirement) => !(requirement.0.marker.is_false() || requirement.0.marker.is_conflicting()),
Err(_) => true,
}),
)
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-pep508/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use crate::marker::lowering::{
LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
};
use crate::marker::MarkerValueExtra;
use crate::ExtraOperator;
use crate::{ExtraOperator, MarkerTree};
use crate::{MarkerExpression, MarkerOperator, MarkerValueVersion};

/// The global node interner.
Expand Down Expand Up @@ -396,6 +396,7 @@ impl InternerGuard<'_> {
}
}


// Restrict the output of a given boolean variable in the tree.
//
// If the provided function `f` returns a `Some` boolean value, the tree will be simplified
Expand Down
151 changes: 147 additions & 4 deletions crates/uv-pep508/src/marker/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::cmp::Ordering;
use std::fmt::{self, Display, Formatter};
use std::ops::{Bound, Deref};
use std::str::FromStr;

use std::sync::LazyLock;
use itertools::Itertools;
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use uv_normalize::ExtraName;
Expand All @@ -16,9 +16,7 @@ use crate::marker::lowering::{
LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
};
use crate::marker::parse;
use crate::{
MarkerEnvironment, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, TracingReporter,
};
use crate::{MarkerEnvironment, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, TracingReporter, VerbatimUrl};

/// Ways in which marker evaluation can fail
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
Expand Down Expand Up @@ -685,6 +683,150 @@ impl MarkerTree {
self.0.is_false()
}

/// Whether the marker is known to be unsatisfiable.
///
/// For example, while the marker specification and grammar do not _forbid_ it, we know that
/// both `sys_platform == 'win32'` and `platform_system == 'Darwin'` will never true at the
/// same time.
///
/// This method thus encodes assumptions about the environment that are not guaranteed by the
/// PEP 508 specification alone.
pub fn is_conflicting(&self) -> bool {
static MUTUAL_EXCLUSIONS: LazyLock<MarkerTree> = LazyLock::new(|| {
let mut tree = MarkerTree::FALSE;
for (a, b) in [
// sys_platform == 'darwin' and platform_system == 'Windows'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "darwin".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Windows".to_string(),
}),
// sys_platform == 'darwin' and platform_system == 'Linux'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "darwin".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Linux".to_string(),
}),
// sys_platform == 'win32' and platform_system == 'Darwin'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "win32".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Darwin".to_string(),
}),
// sys_platform == 'win32' and platform_system == 'Linux'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "win32".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Linux".to_string(),
}),
// sys_platform == 'linux' and platform_system == 'Darwin'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Darwin".to_string(),
}),
// sys_platform == 'linux' and platform_system == 'Windows'
(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Windows".to_string(),
}),
// os_name == 'nt' and sys_platform == 'darwin'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "nt".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "darwin".to_string(),
}),
// os_name == 'nt' and sys_platform == 'linux'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "nt".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
}),
// os_name == 'posix' and sys_platform == 'win32'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "posix".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "win32".to_string(),
}),
// os_name == 'nt' and platform_system == 'Darwin'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "nt".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Darwin".to_string(),
}),
// os_name == 'nt' and platform_system == 'Linux'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "nt".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Linux".to_string(),
}),
// os_name == 'posix' and platform_system == 'Windows'
(MarkerExpression::String {
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "posix".to_string(),
}, MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: "Windows".to_string(),
}),
] {
let mut a = MarkerTree::expression(a);
let b = MarkerTree::expression(b);
a.and(b);
tree.or(a);
}
tree.negate()
});

self.is_disjoint(&MUTUAL_EXCLUSIONS)
}

/// Returns a new marker tree that is the negation of this one.
#[must_use]
pub fn negate(&self) -> MarkerTree {
Expand Down Expand Up @@ -727,6 +869,7 @@ impl MarkerTree {
INTERNER.lock().is_disjoint(self.0, other.0)
}


/// Returns the contents of this marker tree, if it contains at least one expression.
///
/// If the marker is `true`, this method will return `None`.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/resolver/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ impl<'d> Forker<'d> {
let mut envs = vec![];
{
let not_marker = self.marker.negate();
if !env_marker.is_disjoint(&not_marker) {
if !env_marker.is_disjoint(&not_marker) && !env_marker.is_conflicting() {
envs.push(env.narrow_environment(not_marker));
}
}
Expand Down
31 changes: 4 additions & 27 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18829,7 +18829,7 @@ fn lock_multiple_sources_respect_marker() -> Result<()> {
----- stdout -----

----- stderr -----
Resolved 3 packages in [TIME]
Resolved 2 packages in [TIME]
"###);

let lock = context.read("uv.lock");
Expand All @@ -18841,11 +18841,6 @@ fn lock_multiple_sources_respect_marker() -> Result<()> {
lock, @r###"
version = 1
requires-python = ">=3.12"
resolution-markers = [
"platform_system == 'Windows' and sys_platform == 'darwin'",
"platform_system == 'Windows' and sys_platform != 'darwin'",
"platform_system != 'Windows'",
]

[options]
exclude-newer = "2024-03-25T00:00:00Z"
Expand All @@ -18854,39 +18849,21 @@ fn lock_multiple_sources_respect_marker() -> Result<()> {
name = "iniconfig"
version = "2.0.0"
source = { registry = "https://pypi.org/simple" }
resolution-markers = [
"platform_system == 'Windows' and sys_platform != 'darwin'",
]
sdist = { url = "https://files.pythonhosted.org/packages/d7/4b/cbd8e699e64a6f16ca3a8220661b5f83792b3017d0f79807cb8708d33913/iniconfig-2.0.0.tar.gz", hash = "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", size = 4646 }
wheels = [
{ url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374", size = 5892 },
]

[[package]]
name = "iniconfig"
version = "2.0.0"
source = { url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl" }
resolution-markers = [
"platform_system == 'Windows' and sys_platform == 'darwin'",
]
wheels = [
{ url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374" },
]

[[package]]
name = "project"
version = "0.1.0"
source = { virtual = "." }
dependencies = [
{ name = "iniconfig", version = "2.0.0", source = { registry = "https://pypi.org/simple" }, marker = "platform_system == 'Windows' and sys_platform != 'darwin'" },
{ name = "iniconfig", version = "2.0.0", source = { url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl" }, marker = "platform_system == 'Windows' and sys_platform == 'darwin'" },
{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform != 'darwin'" },
]

[package.metadata]
requires-dist = [
{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform != 'darwin'" },
{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform == 'darwin'", url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl" },
]
requires-dist = [{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform != 'darwin'" }]
"###
);
});
Expand All @@ -18898,7 +18875,7 @@ fn lock_multiple_sources_respect_marker() -> Result<()> {
----- stdout -----

----- stderr -----
Resolved 3 packages in [TIME]
Resolved 2 packages in [TIME]
"###);

Ok(())
Expand Down

0 comments on commit 7a177a9

Please sign in to comment.