Skip to content

Commit 7b81730

Browse files
charliermarshcharliermarsh
committed
Sort
1 parent fd3a6b5 commit 7b81730

16 files changed

+191
-158
lines changed

‎crates/uv-pep508/src/marker/simplify.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub(crate) fn to_dnf(tree: MarkerTree) -> Vec<Vec<MarkerExpression>> {
2121
let mut dnf = Vec::new();
2222
collect_dnf(tree, &mut dnf, &mut Vec::new());
2323
simplify(&mut dnf);
24+
sort(&mut dnf);
2425
dnf
2526
}
2627

@@ -278,6 +279,22 @@ fn simplify(dnf: &mut Vec<Vec<MarkerExpression>>) {
278279
}
279280
}
280281

282+
/// Sort the clauses in a DNF expression, for backwards compatibility. The goal is to avoid
283+
/// unnecessary churn in the display output of the marker expressions, e.g., when modifying the
284+
/// internal representations used in the marker algebra.
285+
fn sort(dnf: &mut [Vec<MarkerExpression>]) {
286+
// Sort each clause.
287+
for clause in dnf.iter_mut() {
288+
clause.sort_by_key(MarkerExpression::kind);
289+
}
290+
// Sort the clauses.
291+
dnf.sort_by(|a, b| {
292+
a.iter()
293+
.map(MarkerExpression::kind)
294+
.cmp(b.iter().map(MarkerExpression::kind))
295+
});
296+
}
297+
281298
/// Merge any edges that lead to identical subtrees into a single range.
282299
pub(crate) fn collect_edges<'a, T>(
283300
map: impl ExactSizeIterator<Item = (&'a Ranges<T>, MarkerTree)>,

‎crates/uv-pep508/src/marker/tree.rs

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,19 @@ pub enum MarkerExpression {
494494
},
495495
}
496496

497+
/// The kind of a [`MarkerExpression`].
498+
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
499+
pub(crate) enum MarkerExpressionKind {
500+
/// A version expression, e.g. `<version key> <version op> <quoted PEP 440 version>`.
501+
Version(MarkerValueVersion),
502+
/// A version "in" expression, e.g. `<version key> in <quoted list of PEP 440 versions>`.
503+
VersionIn(MarkerValueVersion),
504+
/// A string marker comparison, e.g. `sys_platform == '...'`.
505+
String(MarkerValueString),
506+
/// An extra expression, e.g. `extra == '...'`.
507+
Extra,
508+
}
509+
497510
/// The operator for an extra expression, either '==' or '!='.
498511
#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
499512
pub enum ExtraOperator {
@@ -564,6 +577,16 @@ impl MarkerExpression {
564577
pub fn from_str(s: &str) -> Result<Option<Self>, Pep508Error> {
565578
MarkerExpression::parse_reporter(s, &mut TracingReporter)
566579
}
580+
581+
/// Return the kind of this marker expression.
582+
pub(crate) fn kind(&self) -> MarkerExpressionKind {
583+
match self {
584+
MarkerExpression::Version { key, .. } => MarkerExpressionKind::Version(*key),
585+
MarkerExpression::VersionIn { key, .. } => MarkerExpressionKind::VersionIn(*key),
586+
MarkerExpression::String { key, .. } => MarkerExpressionKind::String(*key),
587+
MarkerExpression::Extra { .. } => MarkerExpressionKind::Extra,
588+
}
589+
}
567590
}
568591

569592
impl Display for MarkerExpression {
@@ -692,13 +715,11 @@ impl MarkerTree {
692715
}
693716

694717
/// Combine this marker tree with the one given via a conjunction.
695-
#[allow(clippy::needless_pass_by_value)]
696718
pub fn and(&mut self, tree: MarkerTree) {
697719
self.0 = INTERNER.lock().and(self.0, tree.0);
698720
}
699721

700722
/// Combine this marker tree with the one given via a disjunction.
701-
#[allow(clippy::needless_pass_by_value)]
702723
pub fn or(&mut self, tree: MarkerTree) {
703724
self.0 = INTERNER.lock().or(self.0, tree.0);
704725
}
@@ -708,7 +729,6 @@ impl MarkerTree {
708729
///
709730
/// If the marker set is always `true`, then it can be said that `self`
710731
/// implies `consequent`.
711-
#[allow(clippy::needless_pass_by_value)]
712732
pub fn implies(&mut self, consequent: MarkerTree) {
713733
// This could probably be optimized, but is clearly
714734
// correct, since logical implication is `-P or Q`.
@@ -723,10 +743,8 @@ impl MarkerTree {
723743
/// never both evaluate to `true` in a given environment. However, this method may return
724744
/// false negatives, i.e. it may not be able to detect that two markers are disjoint for
725745
/// complex expressions.
726-
pub fn is_disjoint(&self, other: MarkerTree) -> bool {
727-
let mutex = &*MUTUAL_EXCLUSIONS;
728-
let node = INTERNER.lock().and(self.0, other.0);
729-
node.is_false() || INTERNER.lock().is_disjoint(node, mutex.0)
746+
pub fn is_disjoint(self, other: MarkerTree) -> bool {
747+
INTERNER.lock().is_disjoint(self.0, other.0)
730748
}
731749

732750
/// Returns the contents of this marker tree, if it contains at least one expression.
@@ -1019,7 +1037,6 @@ impl MarkerTree {
10191037
/// results of that simplification. (If `requires-python` changes, then one
10201038
/// should reconstitute all relevant markers from the source data.)
10211039
#[must_use]
1022-
#[allow(clippy::needless_pass_by_value)]
10231040
pub fn simplify_python_versions(
10241041
self,
10251042
lower: Bound<&Version>,
@@ -1040,7 +1057,6 @@ impl MarkerTree {
10401057
/// `python_full_version <= '3.10'`, this would result in a marker of
10411058
/// `python_full_version >= '3.8' and python_full_version <= '3.10'`.
10421059
#[must_use]
1043-
#[allow(clippy::needless_pass_by_value)]
10441060
pub fn complexify_python_versions(
10451061
self,
10461062
lower: Bound<&Version>,
@@ -2247,7 +2263,7 @@ mod test {
22472263

22482264
assert_simplifies(
22492265
"((extra == 'a' or extra == 'b') and extra == 'c') or extra == 'b'",
2250-
"(extra == 'a' and extra == 'c') or extra == 'b'",
2266+
"extra == 'b' or (extra == 'a' and extra == 'c')",
22512267
);
22522268

22532269
// post-normalization filtering
@@ -2469,10 +2485,10 @@ mod test {
24692485
or (implementation_name != 'pypy' and sys_platform == 'win32')
24702486
or (sys_platform == 'win32' and os_name != 'nt')
24712487
or (sys_platform != 'win32' and os_name == 'nt')",
2472-
"(sys_platform != 'win32' and implementation_name == 'pypy') \
2488+
"(implementation_name == 'pypy' and sys_platform != 'win32') \
2489+
or (implementation_name != 'pypy' and sys_platform == 'win32') \
24732490
or (os_name != 'nt' and sys_platform == 'win32') \
2474-
or (os_name == 'nt' and sys_platform != 'win32') \
2475-
or (sys_platform == 'win32' and implementation_name != 'pypy')",
2491+
or (os_name == 'nt' and sys_platform != 'win32')",
24762492
);
24772493

24782494
// This is a case we can simplify fully, but it's dependent on the variable order.
@@ -2500,7 +2516,7 @@ mod test {
25002516
"(os_name == 'Linux' and sys_platform == 'win32') \
25012517
or (os_name != 'Linux' and sys_platform == 'win32' and python_version == '3.7') \
25022518
or (os_name != 'Linux' and sys_platform == 'win32' and python_version == '3.8')",
2503-
"(sys_platform == 'win32' and python_full_version >= '3.7' and python_full_version < '3.9') or (os_name == 'Linux' and sys_platform == 'win32')",
2519+
"(python_full_version >= '3.7' and python_full_version < '3.9' and sys_platform == 'win32') or (os_name == 'Linux' and sys_platform == 'win32')",
25042520
);
25052521

25062522
assert_simplifies(
@@ -2522,9 +2538,9 @@ mod test {
25222538
assert_simplifies(
25232539
"(os_name == 'nt' and sys_platform == 'win32') \
25242540
or (os_name != 'nt' and platform_version == '1' and (sys_platform == 'win32' or sys_platform == 'win64'))",
2525-
"(sys_platform == 'win32' and platform_version == '1') \
2526-
or (os_name != 'nt' and sys_platform == 'win64' and platform_version == '1') \
2527-
or (os_name == 'nt' and sys_platform == 'win32')",
2541+
"(os_name != 'nt' and platform_version == '1' and sys_platform == 'win64') \
2542+
or (os_name == 'nt' and sys_platform == 'win32') \
2543+
or (platform_version == '1' and sys_platform == 'win32')",
25282544
);
25292545

25302546
assert_simplifies(

‎crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__line-endings-poetry-with-hashes.txt.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ RequirementsTxt {
8585
),
8686
),
8787
),
88-
marker: platform_system == 'Windows' and python_full_version >= '3.8' and python_full_version < '4.0',
88+
marker: python_full_version >= '3.8' and python_full_version < '4.0' and platform_system == 'Windows',
8989
origin: Some(
9090
File(
9191
"<REQUIREMENTS_DIR>/poetry-with-hashes.txt",

‎crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-poetry-with-hashes.txt.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ RequirementsTxt {
8585
),
8686
),
8787
),
88-
marker: platform_system == 'Windows' and python_full_version >= '3.8' and python_full_version < '4.0',
88+
marker: python_full_version >= '3.8' and python_full_version < '4.0' and platform_system == 'Windows',
8989
origin: Some(
9090
File(
9191
"<REQUIREMENTS_DIR>/poetry-with-hashes.txt",

‎crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-editable.txt.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ RequirementsTxt {
167167
"dev",
168168
),
169169
],
170-
marker: os_name == 'posix' and python_full_version >= '3.9',
170+
marker: python_full_version >= '3.9' and os_name == 'posix',
171171
origin: Some(
172172
File(
173173
"<REQUIREMENTS_DIR>/editable.txt",
@@ -224,7 +224,7 @@ RequirementsTxt {
224224
"dev",
225225
),
226226
],
227-
marker: os_name == 'posix' and python_full_version >= '3.9',
227+
marker: python_full_version >= '3.9' and os_name == 'posix',
228228
origin: Some(
229229
File(
230230
"<REQUIREMENTS_DIR>/editable.txt",
@@ -274,7 +274,7 @@ RequirementsTxt {
274274
},
275275
},
276276
extras: [],
277-
marker: os_name == 'posix' and python_full_version >= '3.9',
277+
marker: python_full_version >= '3.9' and os_name == 'posix',
278278
origin: Some(
279279
File(
280280
"<REQUIREMENTS_DIR>/editable.txt",

‎crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-windows-editable.txt.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ RequirementsTxt {
167167
"dev",
168168
),
169169
],
170-
marker: os_name == 'posix' and python_full_version >= '3.9',
170+
marker: python_full_version >= '3.9' and os_name == 'posix',
171171
origin: Some(
172172
File(
173173
"<REQUIREMENTS_DIR>/editable.txt",
@@ -224,7 +224,7 @@ RequirementsTxt {
224224
"dev",
225225
),
226226
],
227-
marker: os_name == 'posix' and python_full_version >= '3.9',
227+
marker: python_full_version >= '3.9' and os_name == 'posix',
228228
origin: Some(
229229
File(
230230
"<REQUIREMENTS_DIR>/editable.txt",
@@ -274,7 +274,7 @@ RequirementsTxt {
274274
},
275275
},
276276
extras: [],
277-
marker: os_name == 'posix' and python_full_version >= '3.9',
277+
marker: python_full_version >= '3.9' and os_name == 'posix',
278278
origin: Some(
279279
File(
280280
"<REQUIREMENTS_DIR>/editable.txt",

‎crates/uv/tests/it/edit.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2958,7 +2958,7 @@ fn add_update_marker() -> Result<()> {
29582958
dependencies = [
29592959
"requests>=2.0,<2.20 ; python_full_version < '3.11'",
29602960
"requests>=2.30; python_version >= '3.11'",
2961-
"requests>=2.31 ; sys_platform == 'win32' and python_full_version >= '3.12'",
2961+
"requests>=2.31 ; python_full_version >= '3.12' and sys_platform == 'win32'",
29622962
]
29632963
29642964
[build-system]
@@ -2999,7 +2999,7 @@ fn add_update_marker() -> Result<()> {
29992999
"requests>=2.0,<2.20 ; python_full_version < '3.11'",
30003000
"requests>=2.10 ; sys_platform == 'win32'",
30013001
"requests>=2.30; python_version >= '3.11'",
3002-
"requests>=2.31 ; sys_platform == 'win32' and python_full_version >= '3.12'",
3002+
"requests>=2.31 ; python_full_version >= '3.12' and sys_platform == 'win32'",
30033003
]
30043004
30053005
[build-system]

‎crates/uv/tests/it/export.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,15 +442,15 @@ fn dependency_multiple_markers() -> Result<()> {
442442
attrs==23.2.0 ; python_full_version >= '3.12' or sys_platform == 'win32' \
443443
--hash=sha256:935dc3b529c262f6cf76e50877d35a4bd3c1de194fd41f47a2b7ae8f19971f30 \
444444
--hash=sha256:99b87a485a5820b23b879f04c2305b44b951b502fd64be915879d77a7e8fc6f1
445-
cffi==1.16.0 ; (os_name == 'nt' and implementation_name != 'pypy' and python_full_version >= '3.12') or (os_name == 'nt' and sys_platform == 'win32' and implementation_name != 'pypy') \
445+
cffi==1.16.0 ; (python_full_version >= '3.12' and implementation_name != 'pypy' and os_name == 'nt') or (implementation_name != 'pypy' and os_name == 'nt' and sys_platform == 'win32') \
446446
--hash=sha256:2c56b361916f390cd758a57f2e16233eb4f64bcbeee88a4881ea90fca14dc6ab \
447447
--hash=sha256:68678abf380b42ce21a5f2abde8efee05c114c2fdb2e9eef2efdb0257fba1235 \
448448
--hash=sha256:9f90389693731ff1f659e55c7d1640e2ec43ff725cc61b04b2f9c6d8d017df6a \
449449
--hash=sha256:b2ca4e77f9f47c55c194982e10f058db063937845bb2b7a86c84a6cfe0aefa8b \
450450
--hash=sha256:bcb3ef43e58665bbda2fb198698fcae6776483e0c4a631aa5647806c25e02cc0 \
451451
--hash=sha256:db8e577c19c0fda0beb7e0d4e09e0ba74b1e4c092e0e40bfa12fe05b6f6d75ba \
452452
--hash=sha256:e6024675e67af929088fda399b2094574609396b1decb609c55fa58b028a32a1
453-
exceptiongroup==1.2.0 ; sys_platform == 'win32' and python_full_version < '3.11' \
453+
exceptiongroup==1.2.0 ; python_full_version < '3.11' and sys_platform == 'win32' \
454454
--hash=sha256:4bfd3996ac73b41e9b9628b04e079f193850720ea5945fc96a08633c66912f14 \
455455
--hash=sha256:91f5c769735f051a4290d52edd0858999b57e5876e9f85937691bd4c9fa3ed68
456456
idna==3.6 ; python_full_version >= '3.12' or sys_platform == 'win32' \
@@ -459,7 +459,7 @@ fn dependency_multiple_markers() -> Result<()> {
459459
outcome==1.3.0.post0 ; python_full_version >= '3.12' or sys_platform == 'win32' \
460460
--hash=sha256:9dcf02e65f2971b80047b377468e72a268e15c0af3cf1238e6ff14f7f91143b8 \
461461
--hash=sha256:e771c5ce06d1415e356078d3bdd68523f284b4ce5419828922b6871e65eda82b
462-
pycparser==2.21 ; (os_name == 'nt' and implementation_name != 'pypy' and python_full_version >= '3.12') or (os_name == 'nt' and sys_platform == 'win32' and implementation_name != 'pypy') \
462+
pycparser==2.21 ; (python_full_version >= '3.12' and implementation_name != 'pypy' and os_name == 'nt') or (implementation_name != 'pypy' and os_name == 'nt' and sys_platform == 'win32') \
463463
--hash=sha256:8ee45429555515e1f6b185e78100aea234072576aa43ab53aefcae078162fca9 \
464464
--hash=sha256:e644fdec12f7872f86c58ff790da456218b10f863970249516d60a5eaca77206
465465
sniffio==1.3.1 ; python_full_version >= '3.12' or sys_platform == 'win32' \

‎crates/uv/tests/it/lock.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,7 +1014,7 @@ fn lock_wheel_url() -> Result<()> {
10141014
{ name = "trio", marker = "extra == 'trio'", specifier = ">=0.23" },
10151015
{ name = "trustme", marker = "extra == 'test'" },
10161016
{ name = "typing-extensions", marker = "python_full_version < '3.11'", specifier = ">=4.1" },
1017-
{ name = "uvloop", marker = "platform_system != 'Windows' and platform_python_implementation == 'CPython' and extra == 'test'", specifier = ">=0.17" },
1017+
{ name = "uvloop", marker = "platform_python_implementation == 'CPython' and platform_system != 'Windows' and extra == 'test'", specifier = ">=0.17" },
10181018
]
10191019

10201020
[[package]]
@@ -1159,7 +1159,7 @@ fn lock_sdist_url() -> Result<()> {
11591159
{ name = "trio", marker = "extra == 'trio'", specifier = ">=0.23" },
11601160
{ name = "trustme", marker = "extra == 'test'" },
11611161
{ name = "typing-extensions", marker = "python_full_version < '3.11'", specifier = ">=4.1" },
1162-
{ name = "uvloop", marker = "platform_system != 'Windows' and platform_python_implementation == 'CPython' and extra == 'test'", specifier = ">=0.17" },
1162+
{ name = "uvloop", marker = "platform_python_implementation == 'CPython' and platform_system != 'Windows' and extra == 'test'", specifier = ">=0.17" },
11631163
]
11641164

11651165
[[package]]
@@ -12368,7 +12368,7 @@ fn lock_overlapping_environment() -> Result<()> {
1236812368
----- stderr -----
1236912369
error: Supported environments must be disjoint, but the following markers overlap: `platform_system != 'Windows'` and `python_full_version >= '3.11'`.
1237012370

12371-
hint: replace `python_full_version >= '3.11'` with `platform_system == 'Windows' and python_full_version >= '3.11'`.
12371+
hint: replace `python_full_version >= '3.11'` with `python_full_version >= '3.11' and platform_system == 'Windows'`.
1237212372
"###);
1237312373

1237412374
Ok(())
@@ -16537,9 +16537,9 @@ fn lock_multiple_sources_conflict() -> Result<()> {
1653716537
|
1653816538
9 | iniconfig = [
1653916539
| ^
16540-
Source markers must be disjoint, but the following markers overlap: `sys_platform == 'win32' and python_full_version == '3.12.*'` and `sys_platform == 'win32'`.
16540+
Source markers must be disjoint, but the following markers overlap: `python_full_version == '3.12.*' and sys_platform == 'win32'` and `sys_platform == 'win32'`.
1654116541

16542-
hint: replace `sys_platform == 'win32'` with `sys_platform == 'win32' and python_full_version != '3.12.*'`.
16542+
hint: replace `sys_platform == 'win32'` with `python_full_version != '3.12.*' and sys_platform == 'win32'`.
1654316543
"###);
1654416544

1654516545
Ok(())
@@ -17168,11 +17168,11 @@ fn lock_multiple_sources_respect_marker() -> Result<()> {
1716817168
version = "0.1.0"
1716917169
source = { virtual = "." }
1717017170
dependencies = [
17171-
{ name = "iniconfig", marker = "sys_platform != 'darwin' and platform_system == 'Windows'" },
17171+
{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform != 'darwin'" },
1717217172
]
1717317173

1717417174
[package.metadata]
17175-
requires-dist = [{ name = "iniconfig", marker = "sys_platform != 'darwin' and platform_system == 'Windows'" }]
17175+
requires-dist = [{ name = "iniconfig", marker = "platform_system == 'Windows' and sys_platform != 'darwin'" }]
1717617176
"###
1717717177
);
1717817178
});
@@ -17777,7 +17777,7 @@ fn lock_group_include() -> Result<()> {
1777717777
source = { registry = "https://pypi.org/simple" }
1777817778
dependencies = [
1777917779
{ name = "attrs" },
17780-
{ name = "cffi", marker = "os_name == 'nt' and implementation_name != 'pypy'" },
17780+
{ name = "cffi", marker = "implementation_name != 'pypy' and os_name == 'nt'" },
1778117781
{ name = "idna" },
1778217782
{ name = "outcome" },
1778317783
{ name = "sniffio" },
@@ -18959,7 +18959,7 @@ fn lock_recursive_extra() -> Result<()> {
1895918959
{ name = "iniconfig" },
1896018960
]
1896118961
qux = [
18962-
{ name = "iniconfig", marker = "sys_platform == 'darwin' and python_full_version < '3.13'" },
18962+
{ name = "iniconfig", marker = "python_full_version < '3.13' and sys_platform == 'darwin'" },
1896318963
]
1896418964

1896518965
[package.metadata]

0 commit comments

Comments
 (0)