Skip to content

Normalize platform_system to sys_platform #9949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 101 additions & 117 deletions crates/uv-pep508/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,49 @@ impl InternerGuard<'_> {
key,
operator,
value,
} => (
Variable::String(key.into()),
Edges::from_string(operator, value),
),
} => {
// Normalize `platform_system` markers to `sys_platform` nodes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would feel more confident if we could codify this upstream (packaging.python.org probably), so uv doesn't end up making assumptions that python redistributors don't make.

Copy link
Member Author

@charliermarsh charliermarsh Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a convo about it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do we have any context on why there is platform_system and sys_platform?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe it's obvious, but I think a comment saying why we normalize to sys_platform (instead of the other way around) would be good too.

Copy link
Member

@zanieb zanieb Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//
// The `platform` module is "primarily intended for diagnostic information to be
// read by humans."
//
// We only normalize when we can confidently guarantee that the values are
// exactly equivalent. For example, we normalize `platform_system == 'Windows'`
// to `sys_platform == 'win32'`, but we do not normalize `platform_system == 'FreeBSD'`
// to `sys_platform == 'freebsd'`, since FreeBSD typically includes a major version
// in its `sys.platform` output.
//
// For cases that aren't normalized, we do our best to encode known-incompatible
// values in `exclusions`.
//
// See: https://discuss.python.org/t/clarify-usage-of-platform-system/70900
let (key, value) = match (key, value.as_str()) {
(MarkerValueString::PlatformSystem, "Windows") => {
(CanonicalMarkerValueString::SysPlatform, "win32".to_string())
}
(MarkerValueString::PlatformSystem, "Darwin") => (
CanonicalMarkerValueString::SysPlatform,
"darwin".to_string(),
),
(MarkerValueString::PlatformSystem, "Linux") => {
(CanonicalMarkerValueString::SysPlatform, "linux".to_string())
}
(MarkerValueString::PlatformSystem, "AIX") => {
(CanonicalMarkerValueString::SysPlatform, "aix".to_string())
}
(MarkerValueString::PlatformSystem, "Emscripten") => (
CanonicalMarkerValueString::SysPlatform,
"emscripten".to_string(),
),
// See: https://peps.python.org/pep-0738/#sys
(MarkerValueString::PlatformSystem, "Android") => (
CanonicalMarkerValueString::SysPlatform,
"android".to_string(),
),
_ => (key.into(), value),
};
(Variable::String(key), Edges::from_string(operator, value))
}
// A variable representing the existence or absence of a particular extra.
MarkerExpression::Extra {
name: MarkerValueExtra::Extra(extra),
Expand Down Expand Up @@ -821,86 +860,23 @@ impl InternerGuard<'_> {
return exclusions;
}
let mut tree = NodeId::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'

// Pairs of `os_name` and `sys_platform` that are known to be incompatible.
//
// For example: `os_name == 'nt' and sys_platform == 'darwin'`
let mut pairs = vec![
(
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
},
MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
key: MarkerValueString::OsName,
operator: MarkerOperator::Equal,
value: "Darwin".to_string(),
value: "nt".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,
Expand All @@ -913,7 +889,6 @@ impl InternerGuard<'_> {
value: "darwin".to_string(),
},
),
// os_name == 'nt' and sys_platform == 'linux'
(
MarkerExpression::String {
key: MarkerValueString::OsName,
Expand All @@ -923,10 +898,9 @@ impl InternerGuard<'_> {
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
value: "ios".to_string(),
},
),
// os_name == 'posix' and sys_platform == 'win32'
(
MarkerExpression::String {
key: MarkerValueString::OsName,
Expand All @@ -939,51 +913,61 @@ impl InternerGuard<'_> {
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(),
},
),
] {
];

// Pairs of `platform_system` and `sys_platform` that are known to be incompatible.
//
// For example: `platform_system == 'FreeBSD' and sys_platform == 'darwin'`
//
// Any `platform_system` values that we normalize away (like `Windows` to `win32`) are
// omitted, since we never expect them to be present in the tree.
//
// Unfortunately, we can't include Cygwin here, since Cygwin appears to use a
// `platform_system` value with versions encoded (e.g., `CYGWIN_NT-10.0-22631).
//
for platform_system in ["FreeBSD", "NetBSD", "OpenBSD", "SunOS", "iOS", "iPadOS"] {
// An enumeration of known values, excluding FreeBSD, SunOS, and other Unix systems,
// which use the lowercased `uname -s`, which typically includes a version (e.g.,
// `freebsd8`).
//
// See: https://docs.python.org/3/library/sys.html#sys.platform
for sys_platform in [
"aix",
"android",
"emscripten",
"ios",
"linux",
"darwin",
"win32",
"cygwin",
"wasi",
] {
// Some of the above pairs are actually compatible.
if matches!((platform_system, sys_platform), ("iOS" | "iPadOS", "ios")) {
continue;
}
pairs.push((
MarkerExpression::String {
key: MarkerValueString::PlatformSystem,
operator: MarkerOperator::Equal,
value: platform_system.to_string(),
},
MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: sys_platform.to_string(),
},
));
}
}

for (a, b) in pairs {
let a = self.expression(a);
let b = self.expression(b);
let a_and_b = conjunction(self, a, b);
tree = disjunction(self, tree, a_and_b);
}

self.state.exclusions = Some(tree);
tree
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ RequirementsTxt {
),
),
),
marker: python_full_version >= '3.8' and python_full_version < '4.0' and platform_system == 'Windows',
marker: python_full_version >= '3.8' and python_full_version < '4.0' and sys_platform == 'win32',
origin: Some(
File(
"<REQUIREMENTS_DIR>/poetry-with-hashes.txt",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ RequirementsTxt {
),
),
),
marker: python_full_version >= '3.8' and python_full_version < '4.0' and platform_system == 'Windows',
marker: python_full_version >= '3.8' and python_full_version < '4.0' and sys_platform == 'win32',
origin: Some(
File(
"<REQUIREMENTS_DIR>/poetry-with-hashes.txt",
Expand Down
15 changes: 3 additions & 12 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,15 @@ mod tree;
pub const VERSION: u32 = 1;

static LINUX_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Linux' and os_name == 'posix' and sys_platform == 'linux'",
)
.unwrap();
let pep508 = MarkerTree::from_str("os_name == 'posix' and sys_platform == 'linux'").unwrap();
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});
static WINDOWS_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Windows' and os_name == 'nt' and sys_platform == 'win32'",
)
.unwrap();
let pep508 = MarkerTree::from_str("os_name == 'nt' and sys_platform == 'win32'").unwrap();
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});
static MAC_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Darwin' and os_name == 'posix' and sys_platform == 'darwin'",
)
.unwrap();
let pep508 = MarkerTree::from_str("os_name == 'posix' and sys_platform == 'darwin'").unwrap();
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/it/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn dependency_extra() -> Result<()> {
click==8.1.7 \
--hash=sha256:ae74fb96c20a0277a1d615f1e4d73c8414f5a98db8b799a7931d1582f3390c28 \
--hash=sha256:ca9853ad459e787e2192211578cc907e7594e294c7ccc834310722b41b9ca6de
colorama==0.4.6 ; platform_system == 'Windows' \
colorama==0.4.6 ; sys_platform == 'win32' \
--hash=sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44 \
--hash=sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6
flask==3.0.2 \
Expand Down
Loading
Loading